sammccall added inline comments.
================
Comment at: clang/lib/Lex/PPLexerChange.cpp:436
+ if (PragmaAssumeNonNullLoc.isValid() && !this->PPOpts->GeneratePreamble &&
+ !(CurLexer && CurLexer->getFileID() == PredefinesFileID) &&
!isEndOfMacro && !(CurLexer && CurLexer->Is_PragmaLexer)) {
----------------
dgoldman wrote:
> sammccall wrote:
> > dgoldman wrote:
> > > sammccall wrote:
> > > > what's the PredefinesFileID special case about?
> > > See ExitedFromPredefinesFile below, this is how I check if we're falling
> > > off the end of the preamble region into the main file (same as done for
> > > the conditional stack), LMK if there's a better way.
> > I don't understand how this checks if we're falling off the preamble
> > region, and the code around ExitedFromPredefinesFile doesn't clarify this
> > for me. Can you explain?
> >
> > The `if (ExitedFromPredefinesFile)` appears to be handling the logical
> > *insertion* of preamble PP events when *consuming* a preamble, which is not
> > what you're trying to do here.
> >
> > The condition here is of the form "if we have an open pragma, and we're not
> > generating a preamble, and some other stuff, then diagnose". So if the
> > baseline case is "diagnose" and the preamble case is an exception, the
> > "other stuff" clauses don't limit the preamble exception, they add extra
> > exceptions!
> `!(CurLexer && CurLexer->getFileID() == PredefinesFileID)` makes sure that if
> we're consuming the preamble, we don't emit the warning. AFAICT this is the
> way to tell that the preamble is terminated, since the current file ID being
> processed is the predefines file and the file is now terminated as of this
> method call (since !isEndOfMacro && !(CurLexer && CurLexer->Is_PragmaLexer)).
> `!this->PPOpts->GeneratePreamble` makes sure that if we're generating the
> preamble, we don't emit the warning. We need to special case both cases
> otherwise we get an error when generating the preamble or when we load the
> preamble before even processing the rest of the main file.
>
> Does that makes sense?
> !(CurLexer && CurLexer->getFileID() == PredefinesFileID) makes sure that if
> we're consuming the preamble, we don't emit the warning
1) if we're consuming the preamble, what exactly is the sequence of events that
would otherwise lead us to emit the warning?
2) what if we're in the predefines file for some other reason instead?
I'm hoping you'll explain to me what you think the predefines file is, and what
its relationship to the preamble is, and so why this condition is correct :-)
My understanding (which is pretty shaky!) is that the predefines file is a kind
of catchall used to inject text into the preprocessor that doesn't appear in
the source file - that it contains definitions of builtin integer types, macros
defined by `-D` on the command line, and so on. If it has any relationship to
the preamble, it's something subtle that's to do with the relative order in
which the preprocessor sees entities like the predefines, the preprocesor, and
the main file.
So according to my understanding, interpreting "reaching EOF in the predefines
file" as "consuming a preamble" is either wrong or something very subtle
requiring a significant comment.
The code you refer to below is doing something very different: it's saying that
if we have a preamble, then reaching the end of the predefines file is the
*trigger* to inject state from it!
> !this->PPOpts->GeneratePreamble makes sure that if we're generating the
> preamble, we don't emit the warning
Sure, but that doesn't sound correct at all! A preamble mostly consists of
parsing a lot of headers, and if any of those headers have an unpaired pragma,
we should be warning on that at EOF. It's only if we hit the "pretend" EOF from
the truncated main file that we want to suppress the warning.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122179/new/
https://reviews.llvm.org/D122179
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
