On Thu, Jan 9, 2014 at 3:18 PM, Marshall Clow <[email protected]> wrote:
> > On Jan 9, 2014, at 2:51 PM, Richard Smith <[email protected]> wrote: > > On Thu, Jan 9, 2014 at 2:39 PM, Marshall Clow <[email protected]>wrote: > >> On Jan 9, 2014, at 2:20 PM, Richard Smith <[email protected]> wrote: >> >> On Thu, Jan 9, 2014 at 7:25 AM, Alp Toker <[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.
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
