aaron.ballman accepted this revision as: aaron.ballman.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM with a request for a comment, but please give @tahonermann a chance to 
look at the review before landing.



================
Comment at: clang/lib/Lex/Lexer.cpp:3379
 
-  if (LooseMatch)
+  if (LooseMatch && Diagnose)
     Res = LooseMatch->CodePoint;
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > Why do we only want to do this if we're diagnosing?
> The scenario we want to avoid:
> 
> There is a tentative parse of a token,  no diagnostic is emitted.
> The token gets cached and contains the loose match.
> Then that cached token gets reused, and because it is well formed, we never 
> get to emit a diagnostic for a loose match.
> Only returning a Loose Match when a diagnostic ensure we always emit the diag 
> once
> Note that previously we would always emit a diag, but doing so caused the 
> diag to be emitted twice for macros (once when parsing the directive and once 
> more when substituting.
Ahhh okay, that makes sense. I think it'd help to add a comment here to explain 
that because it's a bit subtle.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138861/new/

https://reviews.llvm.org/D138861

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to