-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add a newline at the end of test files #13685
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of See #13684
please use Fixes #13684
or we will end up with stale issues that are still open.
(Note that meson doesn't care if you open a corresponding issue for each PR -- if you expect a change to not be controversial you don't have to bother.)
Note regarding the commit message:
- C compilers don't demand anything, POSIX demands it. C source code is defined as text stored in text files, and POSIX says a text file is a file that has a trailing newline: https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_403
mesonbuild/compilers/compilers.py
Outdated
@@ -805,6 +805,7 @@ def compile(self, code: 'mesonlib.FileOrString', | |||
'testfile.' + self.default_suffix) | |||
with open(srcname, 'w', encoding='utf-8') as ofile: | |||
ofile.write(code) | |||
ofile.write('\n') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dcbaker this will technically have a visible effect on user code in meson.build I think -- depending on whether user code passed to cc.compiles()
is well formed. Do we care? I think we don't care. The alternative is indeed adding a newline to 20 different locations across 5 different files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sure hope we don't care. But now that I think about it, is there a case where some language is going to get mad in the other way? like does (for example) D have a --pedantic-errors
option that will yell if there's two trailing newlines?, ala:
has_int = dc.compiles('int add(int l, int r) { return l + r; }\n')
since we'd now insert a second newline?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To which I'm wondering, would it be safer to do something like:
with open(...) as f:
f.write(code)
if not code.endswith('\n'):
f.write('\n')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if it is harmless (from a POSIX and compiler perspective) to add an extra newline when there is already one there, what you suggest @dcbaker may be preferable anyway. To me it makes it more obvious to the reader why we might be appending a newline. I'm happy to update this PR if that's the consensus.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I sure hope we don't care. But now that I think about it, is there a case where some language is going to get mad in the other way? like does (for example) D have a
--pedantic-errors
option that will yell if there's two trailing newlines?, ala:has_int = dc.compiles('int add(int l, int r) { return l + r; }\n')since we'd now insert a second newline?
D is pretty lax when it comes to warnings, it will only complain on stuff that it knows is 99-100% wrong from a running code standpoint. The only stylistic diagnostic that I'm aware of is using an empty statement with for
or if
and that's an error, not a warning:
void main () {
if (true);
int oops;
}
a.d(2): Error: use `{ }` for an empty statement, not `;`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know about D :) I just I don't want to end up in a position where we start inserting an extra newline in the check case to fix warnings/errors on *nix, then start having user's code fail in other cases because their compiler/platform is very strict that there must be exactly one newline at the end of the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eli-schwartz, what do you think?
I've updated this to only add the newline if needed, as suggested by @dcbaker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In general, please have one logical change per commit and avoid making changes then partially rolling them back in a followup commit.
When running in some settings, a C compiler may demand newlines at the end of each file. Instead of modifying everywhere that writes out test files to incorporate newlines in each indivudual string, simply add a newline when writing it out. Only add a newline to the end of the file if there isn't one already there. An examples of when this is a problem is running with `CC=clang` and `CFLAGS="--std=c99 -pedantic-errors"` and meson.build contains (for example) the following: ``` project('myproject', 'c') executable('myexecutable', 'main.c') cc = meson.get_compiler('c') sizeof_int = cc.sizeof('int') ``` sizeof_int will be -1, because the compile failed. The meson logs contain the error `testfile.c:7:10: error: no newline at end of file`
48b6429
to
5102f43
Compare
When running in some settings, a C compiler may demand newlines at the end of each file. Instead of modifying everywhere that writes out test files to incorporate newlines in each indivudual string, simply add a newline when writing it out.
An examples of when this is a problem is running with
CC=clang
andCFLAGS="--std=c99 -pedantic-errors"
and meson.build contains (for example) the following:sizeof_int will be -1, because the compile failed. The meson logs contain the error
testfile.c:7:10: error: no newline at end of file
Fixes #13684