[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 Paolo Carlini paolo.carlini at oracle dot com changed: What|Removed |Added CC||jwakely.gcc at gmail dot com, ||paolo.carlini at oracle dot com --- Comment #4 from Paolo Carlini paolo.carlini at oracle dot com --- This weird issue has to do with atomic including stdbool.h via atomic_base.h. I don't know why it does that. (minor, unrelated, nit I also noticed: when atomic sees __cplusplus 201103L it should *only* include c++0x_warning.h). Jon, any idea about stdbool.h? Out of curiousity, want to see if something breaks if I simply comment it out...
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 Paolo Carlini paolo.carlini at oracle dot com changed: What|Removed |Added CC||dodji at gcc dot gnu.org --- Comment #5 from Paolo Carlini paolo.carlini at oracle dot com --- Well, of course the user can always explicitly include, eg, cstdbool, thus it seems that the real underlying issue is that the system-headers machinery should not be fooled by things like this in a system header... or should it? The define is rather interesting... #define false false I'm adding in CC Dodji too...
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #6 from Jonathan Wakely redi at gcc dot gnu.org --- (In reply to Paolo Carlini from comment #5) #define false false No idea what's going on, but I think I have a patch somewhere to disable that macro for C++, it's very explicitly non-conforming: [support.runtime]/8 The header cstdbool and the header stdbool.h shall not define macros named bool, true, or false. Let me dig out the old patch. I also remember some discussion with Joseph IIRC.
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #7 from Paolo Carlini paolo.carlini at oracle dot com --- Ah, that makes a lot of sense! If testing goes well, I mean to commit the below, which in any case shouldn't hurt: Index: include/bits/atomic_base.h === --- include/bits/atomic_base.h (revision 216624) +++ include/bits/atomic_base.h (working copy) @@ -33,7 +33,6 @@ #pragma GCC system_header #include bits/c++config.h -#include stdbool.h #include stdint.h #include bits/atomic_lockfree_defines.h Index: include/std/atomic === --- include/std/atomic (revision 216624) +++ include/std/atomic (working copy) @@ -36,7 +36,7 @@ #if __cplusplus 201103L # include bits/c++0x_warning.h -#endif +#else #include bits/atomic_base.h @@ -1129,4 +1129,6 @@ _GLIBCXX_END_NAMESPACE_VERSION } // namespace -#endif +#endif // C++11 + +#endif // _GLIBCXX_ATOMIC
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #8 from Jonathan Wakely redi at gcc dot gnu.org --- See https://gcc.gnu.org/ml/gcc-patches/2012-01/msg00375.html Gerald objected to the patch saying the dumb macros should be defined for C++98 mode or it will break code. Not sure I agree, but I'll adjust the patch to only define them when __cplusplus 201103L
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #9 from Manuel López-Ibáñez manu at gcc dot gnu.org --- (In reply to Paolo Carlini from comment #5) Well, of course the user can always explicitly include, eg, cstdbool, thus it seems that the real underlying issue is that the system-headers machinery should not be fooled by things like this in a system header... or should it? The define is rather interesting... #define false false I'm adding in CC Dodji too... In the case of NULL we do: source_location loc = expansion_point_location_if_in_system_header (input_location); however, we do not do it in the case of 'false' (because we do not think it should be a macro, but it actually is). Perhaps we should do it, is there a downside to it?
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #10 from Manuel López-Ibáñez manu at gcc dot gnu.org --- (In reply to Manuel López-Ibáñez from comment #9) (In reply to Paolo Carlini from comment #5) Well, of course the user can always explicitly include, eg, cstdbool, thus it seems that the real underlying issue is that the system-headers machinery should not be fooled by things like this in a system header... or should it? The define is rather interesting... #define false false I'm adding in CC Dodji too... In the case of NULL we do: source_location loc = expansion_point_location_if_in_system_header (input_location); however, we do not do it in the case of 'false' (because we do not think it should be a macro, but it actually is). Perhaps we should do it, is there a downside to it? BTW, this is the guideline I was asking for here: https://gcc.gnu.org/ml/gcc-patches/2014-10/msg01783.html In this case, 'false' is one of those special macros.
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #11 from Paolo Carlini paolo.carlini at oracle dot com --- Ah I see, then Dodji finally has the testcase he was looking for ;) Well, an equivalent one not using stdbool, which Jon is going to patch. Over the next week I will be mostly offline, unfortunately, please make sure Dodji sees it!
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #12 from Jonathan Wakely redi at gcc dot gnu.org --- (In reply to Manuel López-Ibáñez from comment #9) however, we do not do it in the case of 'false' (because we do not think it should be a macro, but it actually is). Perhaps we should do it, is there a downside to it? The C++ standard explicitly forbids false from being a macro, it's a bug in stdbool.h and IMHO the front-end should not be changed to accommodate the bug.
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 Jakub Jelinek jakub at gcc dot gnu.org changed: What|Removed |Added CC||jakub at gcc dot gnu.org --- Comment #13 from Jakub Jelinek jakub at gcc dot gnu.org --- But the C standard requires that it is a macro. So, shouldn't just cstdbool #undef false and #undef true, or does C++ behave a particular behavior also for a header it doesn't own (stdbool.h)?
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #14 from Jakub Jelinek jakub at gcc dot gnu.org --- Ah, as stdbool.h documents, using it in C++ (apparently meant C++98) is a GNU extension, and for that I bet having those macros is desirable. And then there is C++11 [support.runtime]/8 that requires it is not done: The header cstdbool and the header stdbool.h shall not define macros named bool, true, or false. So, perhaps those macros should be conditional on C++ version?
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #15 from Jonathan Wakely redi at gcc dot gnu.org --- Yes, that's what the patch I'm testing does. IMHO we should only define it for -std=gnu++98 and not any other -std mode, but I'll be conservative and leave it defined for -std=c++98 as well.
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304
--- Comment #16 from Manuel López-Ibáñez manu at gcc dot gnu.org ---
(In reply to Jonathan Wakely from comment #15)
Yes, that's what the patch I'm testing does.
IMHO we should only define it for -std=gnu++98 and not any other -std mode,
but I'll be conservative and leave it defined for -std=c++98 as well.
Thus, the warning also needs fixing. Since the same behavior will occur if the
user directly or indirectly includes stdbool.h.
A testcase:
# 1 false.c
# 1 built-in
# 1 command-line
# 1 false.c
# 1 sys.h 1
# 2 sys.h 3
# 2 false.c 2
int * foo() {return
# 2 false.c 3
false
# 2 false.c
;}
# 1 nonsys.h 1
# 4 false.c 2
int * bar() {return false;}
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304
--- Comment #17 from Manuel López-Ibáñez manu at gcc dot gnu.org ---
(In reply to Manuel López-Ibáñez from comment #16)
A testcase:
== sys.h ==
#pragma GCC system_header
#if defined false
#undef false
#endif
#define false false
== nonsys.h ==
#if defined false
#undef false
#endif
#define false false
== false.c ==
#include sys.h
int * foo() {return false;}
#include nonsys.h
int * bar() {return false;}
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #18 from Jonathan Wakely redi at gcc dot gnu.org --- I assume that testcase is meant to be C++ (because it isn't valid C) but it's not valid C++ either: 17.6.4.3.1 [macro.names] A translation unit that includes a header shall not contain any macros that define names declared or defined in that header. Nor shall such a translation unit define macros for names lexically identical to keywords.
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #20 from Manuel López-Ibáñez manu at gcc dot gnu.org --- (In reply to Manuel López-Ibáñez from comment #19) (In reply to Jonathan Wakely from comment #18) 17.6.4.3.1 [macro.names] A translation unit that includes a header shall not contain any macros that define names declared or defined in that header. Nor shall such a translation unit define macros for names lexically identical to keywords. (Also, why is this not an error with -std=c++11 -pedantic-errors?) In fact, if that quote appears in C++98, then it should be an error with -pedantic-errors in any C++ -std= mode.
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #19 from Manuel López-Ibáñez manu at gcc dot gnu.org --- (In reply to Jonathan Wakely from comment #18) I assume that testcase is meant to be C++ (because it isn't valid C) but it's not valid C++ either: 17.6.4.3.1 [macro.names] A translation unit that includes a header shall not contain any macros that define names declared or defined in that header. Nor shall such a translation unit define macros for names lexically identical to keywords. This is exactly what G++'s stdbool.h is doing with -std=gnu++98 and -std=c++98. (Also, why is this not an error with -std=c++11 -pedantic-errors?)
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #21 from Jonathan Wakely redi at gcc dot gnu.org --- (In reply to Manuel López-Ibáñez from comment #19) This is exactly what G++'s stdbool.h is doing with -std=gnu++98 and -std=c++98. stdbool.h is a header which is C++ standardese for a standard library header. The rule only applies to user code, not std::lib headers. In your testcase sys.h is OK but nonsys.h is not. Although you can argue that since stdbool.h is not defined by the C++98 standard it is not a header and must not define the macro in __STRICT_ANSI__ mode. (Also, why is this not an error with -std=c++11 -pedantic-errors?) Because noone has ever implemented the diagnostic. Note that it only applies to user-code that includes a header so it's OK to define macros using the names of keywords as long as you never include a standard header. That makes it a bit more complicated to implement the diagnostic.
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 --- Comment #22 from Manuel López-Ibáñez manu at gcc dot gnu.org --- (In reply to Jonathan Wakely from comment #21) (In reply to Manuel López-Ibáñez from comment #19) This is exactly what G++'s stdbool.h is doing with -std=gnu++98 and -std=c++98. stdbool.h is a header which is C++ standardese for a standard library header. The rule only applies to user code, not std::lib headers. In your testcase sys.h is OK but nonsys.h is not. We can drop the nonsys.h case. It is only testing that the no system_headers case also works (according to your other comment, it will be valid if no system_header is included). My point is that the fix cannot be only limited to tweaking stdbool.h, as long as any C++ mode still defines false as a macro.
[Bug c++/60304] Including atomic disables -Wconversion-null
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=60304 Manuel López-Ibáñez manu at gcc dot gnu.org changed: What|Removed |Added Keywords||diagnostic Status|UNCONFIRMED |NEW Last reconfirmed||2014-10-06 CC||manu at gcc dot gnu.org Component|preprocessor|c++ Ever confirmed|0 |1 --- Comment #3 from Manuel López-Ibáñez manu at gcc dot gnu.org --- For some reason, it thinks that false comes from a system-header. Weird.
