rsmith added inline comments.
================ Comment at: lib/Parse/ParseExprCXX.cpp:2907-2909 + const Token Next = GetLookAheadToken(2); + const Token After = GetLookAheadToken(3); + const Token Last = GetLookAheadToken(4); ---------------- Please don't request the additional lookahead tokens until you've checked `Next`. (Maybe factor out some parts of the check into a lambda so you can return early?) ================ Comment at: lib/Parse/ParseExprCXX.cpp:2919 + // to disambiguate it from 'delete[]'. + ExprResult Lambda = TryParseLambdaExpression(); + if (!Lambda.isInvalid()) { ---------------- `TryParseLambdaExpression` will always commit to parsing as a //lambda-expression//, because the first two tokens are `[]`. Since you've already determined that we definitely have a *lambda-expression* here, you may as well just call `ParseLambdaExpression` directly. ================ Comment at: lib/Parse/ParseExprCXX.cpp:2925 + + SourceLocation BeforeBracket = StartLoc.getLocWithOffset(-1); + Diag(BeforeBracket, diag::note_lambda_after_delete) ---------------- This offset of -1 isn't right: when we have a fix-it pointing at a character, insertions are inserted before that character. Given `delete[]{}`, this would insert the `(` between `delet` and `e[]`, rather than between `delete` and `[]`. ================ Comment at: lib/Parse/ParseExprCXX.cpp:2929 + << FixItHint::CreateInsertion( + Lambda.get()->getLocEnd().getLocWithOffset(1), ")"); + ---------------- Instead of `getLocWithOffset` here, use `Lexer::getLocForEndOfToken`; `getLocWIthOffset(1)` will return the wrong location if the closing brace token's length is greater than 1 (which can happen in the presence of escaped newlines). ================ Comment at: lib/Parse/ParseExprCXX.cpp:2935-2936 + Lambda.get()); + } + else + return ExprError(); ---------------- Our coding style puts the `else` on the same line as the `}`. However, we also generally don't like using `else` when the `if` block returns. And in this case I'd reverse the sense of the earlier `if`: ``` if (Lambda.isInvalid()) return ExprError(); ``` https://reviews.llvm.org/D36357 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits