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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
