AndyG added inline comments.

================
Comment at: lib/Lex/PPMacroExpansion.cpp:1456-1457
@@ +1455,4 @@
+
+    // Parse next non-comment, non-annotation token.
+    do PP.LexUnexpandedNonComment(Tok); while (Tok.isAnnotation());
+
----------------
rsmith wrote:
> 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`.
Ok, annotation tokens are not ignored, but drop down into `Op` and can be 
handled there.

================
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();
----------------
rsmith wrote:
> 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.
I've handled this via a specific diagnostic message: "nested parentheses not 
permitted in %0" where %0 is the macro name.  I would prefer this to bringing 
the error checking into `Op` since this would complicate the implementation of 
`Op` needlessly IMHO.  `Op` should be as slimline as possible.


http://reviews.llvm.org/D17149



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

Reply via email to