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.
