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