[m-rev.] for post-commit review: infrastructure for testing new filename translation methods

Zoltan Somogyi zoltan.somogyi at runbox.com
Mon Jun 5 00:51:28 AEST 2023


On 2023-06-04 16:16 +02:00 CEST, "Julien Fischer" <jfischer at opturion.com> wrote:

>> +:- func string_extensions = list(string).
>> +
>> +% We don't test the .m extension, since that is handled separately
>> +% by module_name_to_source_file_name.
>> +string_extensions = > +    ["",
>> +    ".$O",
> 
> $O will expand to .o or .obj depending on the underlying object file extension
> used by the platform ...

Yes, but file_names.m is also asked to figure out the name of a "file"
for a given module whose extension is ".$O". Of course, this cannot be done,
since mmc does not know what the value of the make variable "O" is,
that being set later. Nevertheless, this works, because the code in the compiler
that asks file_names.m to do this does not treat the return value as a file name,
but as the name of a make target.

Separating this kind of extension used to construct not-really-file-names
from those used to create actual file names is one objective of this
series of changes.

>> +    ".mode_constraints",
>> +    ".module_dep",
>> +    ".o",
> 
> ... so why is .o listed separately here?

Because the code of file_names.m gets requests for file name translations
both for ".$O" and ".o", and it is NOT immediately obvious that it does
the same thing for both. It *should* be, but it isn't. Fixing this is another
objective.

> (Or why is .obj not also listed?)

Because that extension appears nowhere in the compiler: "egrep -w obj *.m"
returns nothing.

> ".dylib" (the shared libray extension on macOS) is missing.

egrep -w dylib *.m returns two hits in the compiler: one is the .dylib extension
in a list of "current dir extensions", while the other is in a comment. There is
no code in the compiler that *generates* references to .dylib extensions
as such. If the compiler ever generates references to files with that extension,
it does so as a result being ".dylib"being set as the value of one of the
options, such as "shared_library_extension".

This means that the existing reference to .dylib is wrong: if we ever generate it
because it is the value of an option, we should *test* for it using that same option.
However, I won't delete that reference until that is done.

I did see an "extension" that should have been listed but wasn't: ".$A".

Thanks for the review.

Zoltan.


More information about the reviews mailing list