aaron.ballman added inline comments.
================
Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:323
"whitespace recommended after macro name">;
+def warn_include_cycle : Warning<"#include cycle">,
+ InGroup<DiagGroup<"include-cycle">>, DefaultIgnore;
----------------
urnathan wrote:
> aaron.ballman wrote:
> > This diagnostic doesn't really seem like it's going to help the user to
> > know what's wrong with their code or how to fix it. (Also, the notes
> > @erichkeane was hoping were emitted don't seem to be, at least according to
> > the new test cases.) I think we need to give users a bit more guidance here.
> The test infrastructire ignores the 'included from ' chain, which is emitted.
> What do you suggest?
Even posting the "included from" doesn't help clarify what's wrong with the
code though, if I'm understanding the tests properly. My concern is with code
like:
```
// self.h
#ifndef GUARD
#define GUARD
#include "self.h" // expected-warning {{#include cycle}}
#endif
```
(This is effectively the same test as `warn-loop-macro-1.h`.) There is no
include *cycle* ("a series of events that are regularly repeated in the same
order.") in this code -- the header is included for the first time, `GUARD` is
not defined, then `GUARD` is defined and the header is included for the second
time. This time `GUARD` is defined and no further cycles into `self.h` occurs.
But I think I'm seeing now what you're trying to get at (correct me if I'm
wrong): when processing an include stack, opening the same header file twice is
a problem (regardless of the contents of the header or how you got into the
same file twice)? If that's accurate, would it make more sense to diagnose this
as:
`including the same header (%0) more than once in an include stack causes
%select{incorrect behavior of Clang modules|the header to not be a C++ module
header unit}1`
or something along those lines? Basically, I'd like to avoid saying "cycle"
because that starts to sound like "you have potentially infinite recursion in
the include stack" and I'd like to add information about what's actually wrong
instead of pointing out what the code does and hoping the user knows why that's
bad.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D134654/new/
https://reviews.llvm.org/D134654
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
