[m-rev.] for post-commit review: stop using .*opt.tmp files

Julien Fischer jfischer at opturion.com
Sun Feb 18 14:31:39 AEDT 2024


On Sun, 18 Feb 2024, Zoltan Somogyi wrote:

> Use string builders for .*opt files.
> 
> compiler/intermod.m:
> compiler/intermod_analysis.m:
>     Instead of writing stuff out to .*opt files, add them to string builders
>     supplied by the callers.
> 
> compiler/mercury_compile_front_end.m:
> compiler/mercury_compile_middle_passes.m:
>     Use the new code in intermod*.m to handle .*opt files the same way
>     we now handle .int* files: by
>
>     - converting the new contents we want for the file to a string,
>     - reading in the old version of the file as a string,
>     - comparing the two strings,
>     - writing out the new version only if it not the same as the old version.

The diff is fine.

It did occur to me that there is an issue with this change (and the one for .int
file) on systems that use CR-LF line endings. This is because the string containing
the existing version of any .int / .opt files would presumably contain the CR-LF
line endings, while those constructed by string builders presumably wouldn't.

This doesn't seem to be the case (at least for the C grades) for a small test
program I wrote on Windows. Some of our I/O read primitives convert CR-LF to
'\n', (e.g. io.primitives_read.read_char_code/6) but I'm not certain about some
of the others, particularly on some of the non-C backends.

In short, there isn't an obvious correctness argument for this change on
systems using CR-LF endings, even though it appears to work. The real problem
is, arguably, that the user-facing documentation of the io module does not
describe what happens with CR-LF line endings when reading from text file input
streams.

(Also, does anyone have any objection to doing s/CR-LF/CRLF/ in the
comments in the library?; I was searching for the latter, which is more
usual, before realising we used the former.)

Julien.


More information about the reviews mailing list