[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #13 from Jonathan Wakely --- (In reply to Jonathan Wakely from comment #12) > Improving the warning in comment 4 is irrelevant to this bug. I've created Bug 89800 for improving that warning, please move that discussion there.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #12 from Jonathan Wakely --- And as I've already said, the quality of the particular -Waggressive-loop-optimizations warning is a separate issue, and should be dealt with in a separate PR. PR 58876 (mentioned in comment 0 as the reason for creating this bug) shows a proper instantiation trace when -Wsystem-headers is used: In file included from /home/jwakely/gcc/9/include/c++/9.0.1/memory:80, from up.cc:1: /home/jwakely/gcc/9/include/c++/9.0.1/bits/unique_ptr.h: In instantiation of 'void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = A]': /home/jwakely/gcc/9/include/c++/9.0.1/bits/unique_ptr.h:289:17: required from 'std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = A; _Dp = std::default_delete]' up.cc:12:30: required from here /home/jwakely/gcc/9/include/c++/9.0.1/bits/unique_ptr.h:81:2: warning: deleting object of abstract class type 'A' which has non-virtual destructor will cause undefined behavior [-Wdelete-non-virtual-dtor] 81 | delete __ptr; | ^~ That shows the up.cc:12 location from the user code that triggers the warning. But I can't make libstdc++ show that warning because of this bug with the diagnostic pragmas. Improving the warning in comment 4 is irrelevant to this bug.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #11 from Manuel López-Ibáñez --- I'm not being pedantic for the sake of being pedantic. It is trivial to fix the #pragma as I explained above. However, that won't give the user any idea about which user code is triggering the warning. For that, we need to have at the point of warning a token that comes from user code or we need to propagate a user location to the tokens that made up this particular instantation of the template, so that the diagnostic code can windup the location stack and find the user code that triggered the warning. On Fri, 22 Mar 2019, 22:18 redi at gcc dot gnu.org, < gcc-bugzi...@gcc.gnu.org> wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 > > --- Comment #10 from Jonathan Wakely --- > (In reply to Manuel López-Ibáñez from comment #8) > > There is no negative n__ in user code. > > If you want to be pedantic, there's no __n at all in user code. Because > it's a > function parameter of std::advance. But clearly if the compiler says > "undefined > behaviour detected at this line" and the line has a comment saying > "undefined > if n is negative" then the user can figure out that the function got called > with negative n. > > Is that perfect? No. Is it better than printing nothing when UB is > detected? To > me the answer is obviously yes, so it seems like you're just objecting to > making improvements. > > -- > You are receiving this mail because: > You are on the CC list for the bug.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #10 from Jonathan Wakely --- (In reply to Manuel López-Ibáñez from comment #8) > There is no negative n__ in user code. If you want to be pedantic, there's no __n at all in user code. Because it's a function parameter of std::advance. But clearly if the compiler says "undefined behaviour detected at this line" and the line has a comment saying "undefined if n is negative" then the user can figure out that the function got called with negative n. Is that perfect? No. Is it better than printing nothing when UB is detected? To me the answer is obviously yes, so it seems like you're just objecting to making improvements.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #9 from Jonathan Wakely --- There is though. std::prev(it, n) is specified as std::advance(it, -n). Calling prev means advancing a negative amount. But I'm not sure what your point is. Currently there's no warning by default even though the compiler has detected UB. Are you saying it is better to not warn at all, and just have silent UB? If you want the warning to be improved, please open a separate bug report. This one is about the inability to enable warnings in system headers, not the quality of individual warnings.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #8 from Manuel López-Ibáñez --- There is no negative n__ in user code. On Fri, 22 Mar 2019, 21:21 redi at gcc dot gnu.org, < gcc-bugzi...@gcc.gnu.org> wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 > > --- Comment #7 from Jonathan Wakely --- > A comment added to the code would make the caret diagnostic > self-explanatory: > > --- a/libstdc++-v3/include/bits/stl_iterator_base_funcs.h > +++ b/libstdc++-v3/include/bits/stl_iterator_base_funcs.h > @@ -149,7 +149,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER >// concept requirements >__glibcxx_function_requires(_InputIteratorConcept<_InputIterator>) >__glibcxx_assert(__n >= 0); > - while (__n--) > + while (__n--) // if n is negative this has undefined behaviour > ++__i; > } > > That would make the diagnostic look like: > > /home/jwakely/gcc/9/include/c++/9.0.1/bits/stl_iterator_base_funcs.h:152:7: > warning: iteration 9223372036854775807 invokes undefined behavior > [-Waggressive-loop-optimizations] > 152 | while (__n--) // if n is negative this has undefined > behaviour > | > > > It's still a bug that there's no "In instantiation of ... required from ... > required from here" context shown though. > > -- > You are receiving this mail because: > You are on the CC list for the bug.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #7 from Jonathan Wakely --- A comment added to the code would make the caret diagnostic self-explanatory: --- a/libstdc++-v3/include/bits/stl_iterator_base_funcs.h +++ b/libstdc++-v3/include/bits/stl_iterator_base_funcs.h @@ -149,7 +149,7 @@ _GLIBCXX_END_NAMESPACE_CONTAINER // concept requirements __glibcxx_function_requires(_InputIteratorConcept<_InputIterator>) __glibcxx_assert(__n >= 0); - while (__n--) + while (__n--) // if n is negative this has undefined behaviour ++__i; } That would make the diagnostic look like: /home/jwakely/gcc/9/include/c++/9.0.1/bits/stl_iterator_base_funcs.h:152:7: warning: iteration 9223372036854775807 invokes undefined behavior [-Waggressive-loop-optimizations] 152 | while (__n--) // if n is negative this has undefined behaviour | It's still a bug that there's no "In instantiation of ... required from ... required from here" context shown though.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #6 from Jonathan Wakely --- Is it better to silently generate code with undefined behaviour, or issue a flawed warning about that undefined behaviour? If the warning doesn't show the template instantiation context that's a separate issue.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 --- Comment #5 from Manuel López-Ibáñez --- This warning will be incomprehensible to users because the warning never mentions any code that the user can modify. What should the user do according to the warning?
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472
--- Comment #4 from Jonathan Wakely ---
Another place where I'd like to selectively enable warnings is for this code
(from PR 78830):
#include
#include
int main()
{
std::forward_list il = {1, 2, 3, 4, 5, 6, 7};
auto iter = std::prev(il.end());
}
With -Wall -O1 -Wsystem-headers GCC detects the undefined behaviour:
In file included from
/home/jwakely/gcc/9/include/c++/9.0.1/bits/stl_algobase.h:66,
from
/home/jwakely/gcc/9/include/c++/9.0.1/bits/forward_list.h:38,
from /home/jwakely/gcc/9/include/c++/9.0.1/forward_list:38,
from prev.cc:1:
/home/jwakely/gcc/9/include/c++/9.0.1/bits/stl_iterator_base_funcs.h:153:7:
warning: iteration 9223372036854775807 invokes undefined behavior
[-Waggressive-loop-optimizations]
153 | while (__n--)
| ^
/home/jwakely/gcc/9/include/c++/9.0.1/bits/stl_iterator_base_funcs.h:153:7:
note: within this loop
but that very useful warning is suppressed by default. This bug prevents me
from locally enabling -Wsystem-headers around the relevant functions in
.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 Manuel López-Ibáñez changed: What|Removed |Added Keywords||easyhack --- Comment #3 from Manuel López-Ibáñez --- It seems pretty straight-forward: 1. Abstract out the check for classification_history. 2. Create a function diagnostic_report_warnings_p that checks the classification history of Wsystem-headers in addition to the option value. Not that -Wsystem-headers is not a typical warning option, so more work would be needed to have #pragma GCC diagnostic error "-Wsystem-headers" give errors instead of warnings or -Werror #pragma GCC diagnostic warning "-Wsystem-headers" give warnings instead of errors. But that seems less important to fix.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 Manuel López-Ibáñez changed: What|Removed |Added CC||manu at gcc dot gnu.org --- Comment #2 from Manuel López-Ibáñez --- Currently, the macro controlling this only looks at the state of the option: /* Returns nonzero if warnings should be emitted. */ #define diagnostic_report_warnings_p(DC, LOC) \ (!(DC)->dc_inhibit_warnings \ && !(in_system_header_at (LOC) && !(DC)->dc_warn_system_headers)) It needs to look at the classification_history like diagnostic_report_diagnostic() does.
[Bug c++/80472] cannot use push/pop with #pragma GCC diagnostic warning "-Wsystem-headers"
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80472 Marek Polacek changed: What|Removed |Added Status|UNCONFIRMED |NEW Last reconfirmed||2017-04-20 CC||mpolacek at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #1 from Marek Polacek --- Confirmed :(.
