On Wed Jan 29 2014 at 1:45:22 PM, Alp Toker <[email protected]> wrote: > > On 09/01/2014 23:26, Richard Smith wrote: > > On Thu, Jan 9, 2014 at 3:18 PM, Marshall Clow <[email protected] > > <mailto:[email protected]>> wrote: > > > > > > On Jan 9, 2014, at 2:51 PM, Richard Smith <[email protected] > > <mailto:[email protected]>> wrote: > > > >> On Thu, Jan 9, 2014 at 2:39 PM, Marshall Clow > >> <[email protected] <mailto:[email protected]>> wrote: > >> > >> On Jan 9, 2014, at 2:20 PM, Richard Smith > >> <[email protected] <mailto:[email protected]>> wrote: > >> > >>> On Thu, Jan 9, 2014 at 7:25 AM, Alp Toker <[email protected] > >>> <mailto:[email protected]>> wrote: > >>> > >>> It's not fair on the user or the system header > >>> maintainers to bring up unexpected errors with this flag. > >>> > >>> > >>> I don't think this is clear. If the code in the system > >>> header is ill-formed, I would expect the system header > >>> maintainers to be *grateful* that we flag it as an error and > >>> not just as a warning. But I'll wait to hear what they have > >>> to say about that. > >> > >> A few points, in no particular order: > >> > >> * If we have ill-formed code in system headers, I would > >> expect it to fail to compile whether the user specifies > >> -Wsystem-headers or not. > >> > >> > >> And yet in some cases we don't, because we deliberately support > >> some flavors of broken system headers. > >> Given that, would you want -Wsystem-headers to make such code > >> produce an error or only a warning? > > > > Eww. > > > > First - is any of that code in libc++? > > Second, there's two audiences that I think are affected here, and > > they have different needs: > > * People trying to use the system headers. These people are, in > > general, unable/unwilling to change them. Giving them > > warnings/errors is just noise. > > * People who are working on the system headers. They need to see > > the warnings. > > > > > >> > >> * If a user has a clean build, and then rebuilds with > >> -Wsystem-headers, I would expect to get warnings - not > >> errors. [ Isn't that what PR18327 is all about? ] > >> > >> > >> Do you expect this because the flag starts with -W? > > > > Yes. > > > >> (Maybe that's the problem here -- it's *not* a warning flag in > >> the sense that the other -W flags are. But that's not > >> unprecedented -- nor is -Werror.) > > > > > >> * There are some "interesting" language features which are > >> only enabled for system headers, and cause warnings if used > >> in user code. > >> [ User-defined suffixes that do not start with an underscore, > >> for example. ] > >> > >> > >> This is a really great example, thanks. > > > > Note that putting something like this in your source file gives > > you a warning; > > > > std::complex<long double> operator"" ilx(long double __im) > > { > > return { 0.0l, __im }; > > } > > > > $ clang11 junk.cpp > > junk.cpp:5:31: warning: user-defined literal suffixes not starting > > with '_' are > > reserved; no literal will invoke this operator > > [-Wuser-defined-literals] > > std::complex<long double> operator"" ilx(long double __im) > > ^ > > 1 warning generated. > > > > > >> So it seems there are at least three different classes of errors > >> that we might think about producing in system headers: > >> > >> 1) warnings that the user has turned into errors with -Werror or > >> -Werror=foo > >> 2) errors for ill-formed code that we suppress by default in > >> system headers as a workaround for a system header bug > >> 3) errors for code that is ill-formed outside system headers but > >> valid within system headers (using reserved names, adding names > >> to namespace std, that sort of thing) > >> > >> With -Wsystem-headers enabled, I think (1) should be an error, > >> (3) should remain suppressed (not even a warning), and (2) should > >> be either a warning or an error (and your first two bullets don't > >> give me a clear idea of which way these cases should go). > > > > Agreed for #1 and #3. > > And for #2, I'd like to see a warning; "This is code we wouldn't > > allow outside a system header, but we do so here" > > > > I don't see the point of making #2 an error. > > For most people (who can't fix it), it's just a hassle. > > For system developers, they can read warning output just as well > > as error output. > > > > > > OK, so I think this is Alp's original patch plus a change to treat #1 > > as an error. As you observe, -Wuser-defined-literals is currently not > > an error outside of a system header, so perhaps we have no diagnostics > > in case #3 at the moment, but it's something to keep in mind for the > > future. > > Status update: I did implement those semantics and it doesn't make as > much sense in practice as we'd hoped. > > The problem is that many conventional flags such as a -Werror build or > specific -Werror upgrades will now consistently cause system headers to > break the build, so it's no better than where we started, and arguably > worse. > > As a result I'm back with proposing the original patch for PR18327 as-is > with more confidence. It's the only mode I've found through several > iterations that adds informational/educational value in a way that users > and system header developers can safely consume. > > (There is good news though, I've got some good cleanups and fixes out of > the investigation that I'll post separately, especially in the area of > "explaining" the source of diagnostics which is currently a hack in ToT.) > > Richard, does that work for you?
I don't think I'm following. Without your patch, -Werror -Wsystem-headers was an error; why is it bad for it to stay that way? -Werror is the way users say "I want a broken build if you find a warning". This change seems to break that.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
