Hello Nicholas, First of all, thank you for taking the time to dive into this code and provide such a detailed analysis along with a patch. This is appreciated.
Please find below my comments to some parts of your message. Nicholas Ormrod <nicholas.orm...@hotmail.com> a écrit: > PR preprocessor/60723 > > Description: > > When line directives are inserted into the expansion of a macro, the line > directive may erroneously specify the file as being a system file. This > causes certain warnings to be suppressed in the rest of the file. Agreed. > The fact that line directives are ever inserted into a macro is itself a > half-bug. Please see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60723 for > full details. We could discuss that, but I might slightly disagree. I would rather say that it's the way the directives are inserted that is a bug. But let's put this topic aside for now. To ease the discussion at hand, let me paste here the test case that you submitted to the bugzilla: cat inc.h #define FOO() static const char * F = __FILE__ ; $ cat src.cpp #include <inc.h> FOO( ) int main() { int z = 1 / 0; return z; } $ $ g++ -E -isystem . src.cpp # 1 "src.cpp" # 1 "<interne>" # 1 "<command-line>" # 1 "/usr/include/stdc-predef.h" 1 3 4 # 1 "<command-line>" 2 # 1 "src.cpp" # 1 "./inc.h" 1 3 4 # 2 "src.cpp" 2 static const char * F = "src.cpp" # 2 "src.cpp" 3 4 ; int main() { int z = 1 / 0; return z; } $ So when compiling the resulting pre-processed file, the warning concerning the division by zero is not emitted, and that is a bug, I agree with you. > Patch: > > Information for locations is, for similar pieces of data, read from a > LOCATION_* macro. The sysp read which was causing the error was using an > inconsistent method to read the data. Resolving this is a two-line > fix. Maybe I am missing something, but my understanding seems to differ here, sorry. I think that we really want to be able to say if the macro FOO that got expanded in the file src.cpp was actually *defined* in a system header or not. That is, if the macro is a system macro[1] or not. Because the expansion of a system macro should (generally) not emit a warning, even when that expansion occurs in a file (src.cpp in your example from bugzilla) that is not a system file. So in that case we want to suppress the potential warnings that arise from the macro expansion. But the issue here is, I think, that (in src.cpp) we consider the tokens resulting from the expansion of the macro FOO as being system tokens[2] (and rightly so) and *also* that all the subsequent tokens of the src.cpp file are being system tokens; and this is wrong. So, I would tend to think that a potential proper fix would emit a subsequent line directive after the lines: # 2 "src.cpp" 3 4 ; So that the pre-processed file looks like: $ g++ -E -isystem . src.cpp # 1 "src.cpp" # 1 "<interne>" # 1 "<command-line>" # 1 "/usr/include/stdc-predef.h" 1 3 4 # 1 "<command-line>" 2 # 1 "src.cpp" # 1 "./inc.h" 1 3 4 # 2 "src.cpp" 2 static const char * F = "src.cpp" # 2 "src.cpp" 3 4 ; # 3 "src.cpp" 4 int main() { int z = 1 / 0; return z; } $ Note the additional line directive "# 3 "src.cpp" 4" that doesn't mention the '3' flags and thus says that the rest of the tokens are *not* system tokens. What do you think? [1]: A system macro is a macro defined in a system header. [2]: A system token is a token coming from the expansion of a system macro Cheers, -- Dodji
