[PATCH] D17149: Consolidate and improve the handling of built-in feature-like macros
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Sun Apr 3 17:12:06 PDT 2016
rsmith added inline comments.
================
Comment at: lib/Lex/PPMacroExpansion.cpp:1430-1432
@@ +1429,5 @@
+ Preprocessor &PP,
+ int(*Op)(Token &Tok,
+ Preprocessor &PP,
+ bool &HasLexedNextTok)) {
+ // Parse the initial '('.
----------------
Use `llvm::function_ref` here instead, and don't pass a `Preprocessor` into the function (instead, capture it in a lambda).
================
Comment at: lib/Lex/PPMacroExpansion.cpp:1456-1457
@@ +1455,4 @@
+
+ // Parse next non-comment, non-annotation token.
+ do PP.LexUnexpandedNonComment(Tok); while (Tok.isAnnotation());
+
----------------
If we get an annotation token here, we should reject it, not silently ignore it. Also, we shouldn't see comment tokens here (we shouldn't be doing macro expansion with comments enabled); you should call `LexUnexpandedToken` rather than `LexUnexpandedNonComment`.
================
Comment at: lib/Lex/PPMacroExpansion.cpp:1481-1484
@@ +1480,6 @@
+ auto Diag = PP.Diag(Tok.getLocation(), diag::err_pp_unexpected_after);
+ if (IdentifierInfo *LastII = LastTok.getIdentifierInfo())
+ Diag << LastII;
+ else
+ Diag << LastTok.getKind();
+ Diag << tok::l_paren << LastTok.getLocation();
----------------
The only way we can get here without already having a value or producing a diagnostic is if this is the first token inside the parens. So this will always say "unexpected '(' after '('".
I think it would be better to always `break` here after incrementing `ParenDepth` (even when `!Result.hasValue()`), and let `Op` produce the relevant diagnostic for this case.
================
Comment at: lib/Lex/PPMacroExpansion.cpp:1519
@@ +1518,3 @@
+
+ // Diagnose expected ')'.
+ if (!SuppressDiagnostic) {
----------------
expected -> missing
http://reviews.llvm.org/D17149
More information about the cfe-commits
mailing list