Skip to content
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

Clang with -pedantic issues -Wgnu-line-marker that it wrote itself in -frewrite-includes output #63284

Open
boris-kolpackov opened this issue Jun 13, 2023 · 8 comments
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@boris-kolpackov
Copy link
Contributor

boris-kolpackov commented Jun 13, 2023

Clang issues the "this style of line directive is a GNU extension" warning for line directives that it writes itself in the output produced with -frewrite-includes. For example:

$ cat <<EOF >test.c
#include <stdlib.h>
EOF

$ clang -pedantic -E -frewrite-includes -o test.i test.c
$ clang -pedantic -c test.i
test.i:1:5: warning: this style of line directive is a GNU extension [-Wgnu-line-marker]
# 1 "<built-in>"
    ^
test.c:4:5: warning: this style of line directive is a GNU extension [-Wgnu-line-marker]
# 1 "test.c"
    ^
test.c:1:5: warning: this style of line directive is a GNU extension [-Wgnu-line-marker]
# 1 "/usr/include/stdlib.h" 1 3 4
    ^

This appears to be new behavior starting from Clang 15.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed new issue labels Jun 13, 2023
@llvmbot
Copy link
Member

llvmbot commented Jun 13, 2023

@llvm/issue-subscribers-clang-frontend

@shafik
Copy link
Collaborator

shafik commented Jun 13, 2023

CC @ken-matsui @AaronBallman

@AaronBallman
Copy link
Collaborator

We previously failed to issue an extension warning for GNU line markers, but that was fixed in Clang 15.

I'm not certain this is a bug so much as a feature request. You're preprocessing to a file (no diagnostics from that step) and then running the preprocessed code back through the compiler (that's when you get the diagnostic). Clang does not support -fpreprocessed to tell the compiler that the code has already been preprocessed, which is how you would silence this normally: https://godbolt.org/z/YP76bbfdd

@boris-kolpackov
Copy link
Contributor Author

[...] and then running the preprocessed code back through the compiler (that's when you get the diagnostic). Clang does not support -fpreprocessed to tell the compiler that the code has already been preprocessed [...]

But -frewrite-includes doesn't fully preprocess the file, so even if -fpreprocessed were supported, it would be wrong to pass it (we would need -fpartially-preprocessed or some such).

I am not sure what I am requesting here, if anything at all. I think it's surprising that Clang warns about things it wrote itself so I wanted to report it and also have a place to refer to from our code which hacks around it (for now we just forcefully disable this warning when compiling partially preprocessed sources).

From my POV the correct fix would be to write some marker to the output of -frewrite-includes to indicate that the following GNU line markers are "blessed". But this would be quite involved, admittedly.

@AaronBallman
Copy link
Collaborator

But -frewrite-includes doesn't fully preprocess the file, so even if -fpreprocessed were supported, it would be wrong to pass it (we would need -fpartially-preprocessed or some such).

That's a really good point! Oofda...

I am not sure what I am requesting here, if anything at all. I think it's surprising that Clang warns about things it wrote itself so I wanted to report it and also have a place to refer to from our code which hacks around it (for now we just forcefully disable this warning when compiling partially preprocessed sources).

I think it's good that you brought the topic up, there's some interesting nuance here, so thank you!

From my POV the correct fix would be to write some marker to the output of -frewrite-includes to indicate that the following GNU line markers are "blessed". But this would be quite involved, admittedly.

That runs into the problem of being an extension as well, and -pedantic-errors is something that should work to prevent use of any extensions (ideally, we've got bugs still to fix there though). But at the same time, I agree with you that it'd be nice to avoid the pedantic warnings somehow...

Ooh, here's a thought. #pragma clang diagnostic is quite possibly a case where we would not want to ever issue a pedantic warning (otherwise there's no way to disable a pedantic warning without producing more pedantic warnings). So perhaps rewrite includes could do:

#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wpedantic"

# 1 "<built-in>"
... other stuff ...

#pragma clang diagnostic pop

to disable pedantic diagnostics for the line markers for rewritten includes. This seems to already work today: https://godbolt.org/z/78MGG656z

That said, if there's a ton of line markers, it might cause sufficient compile time overhead to be a problem.

@boris-kolpackov
Copy link
Contributor Author

Haven't thought of #pragma clang diagnostic. I think if we wrap the -fdirectives-only output with ignore "-Wgnu-line-marker" we will get exactly what we want since the user's line directives, if any, will be diagnosed during the -fdirective-only preprocessing. Here is a prototype:

$ cat <<EOF >test.c
#include <stdlib.h>
# 1 "test.y"
EOF

$ clang -pedantic -E -frewrite-includes -o test.i test.c
test.c:2:5: warning: this style of line directive is a GNU extension [-Wgnu-line-marker]
# 1 "test.y"
    ^

$ cat <<EOF >test1.i
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wgnu-line-marker"
EOF

$ cat test.i >>test1.i

$ cat <<EOF >>test1.i
#pragma clang diagnostic pop
EOF

$ clang -pedantic -c test1.i

Note that I believe we want to ignore -Wgnu-line-marker and not -Wpedantic since the latter will also ignore any language (as opposed to preprocessor) issues that might still be present in the -fdirectives-only output.

@AaronBallman
Copy link
Collaborator

We do need to think about what diagnostic to disable, but my thinking is that disabling -Wpedantic is correct for any code we're generating that the user has no control over (in general). As an implementation detail, we want to be able to use whatever extension mechanisms we need and that may include other extensions than just line markers. But I suppose this also depends on the scope of the pragmas. If we're wrapping the line markers themselves with the pragmas, then we only need to disable -Wgnu-line-marker. I've not looked into the -frewrite-includes implementation to know if we are going to want a broader scope than that or not, hence my thinking about -Wpedantic.

@maartenarnst
Copy link

Just noting that we're also seeing this issue in AMD's fork for the ROCm stack:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

No branches or pull requests

6 participants