[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas
Richard Smith - zygoloid via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jul 10 14:48:12 PDT 2018
rsmith added a comment.
Thanks!
================
Comment at: lib/Parse/ParseExprCXX.cpp:2956
+ const Token Next = GetLookAheadToken(2);
+ const auto GetAfter = [this] { return GetLookAheadToken(3); };
+
----------------
I don't think this lambda is useful: since you're not caching the result of `GetLookAheadToken(3)` between calls, you may as well call it directly below.
================
Comment at: lib/Parse/ParseExprCXX.cpp:2961-2962
+ (Next.is(tok::l_paren) &&
+ (GetAfter().is(tok::r_paren) ||
+ (GetAfter().is(tok::identifier) &&
+ GetLookAheadToken(4).is(tok::identifier))))) {
----------------
Use `GetLookAheadToken(3).isOneOf(tok::r_paren, tok::identifier)` here.
================
Comment at: lib/Parse/ParseExprCXX.cpp:2971-2980
+ Diag(Start, diag::err_lambda_after_delete)
+ << SourceRange(Start, StartLoc.getLocWithOffset(1));
+
+ Diag(StartLoc, diag::note_lambda_after_delete)
+ << FixItHint::CreateInsertion(StartLoc, "(")
+ << FixItHint::CreateInsertion(
+ Lexer::getLocForEndOfToken(Lambda.get()->getLocEnd(), 1,
----------------
Because we can be very confident the fix is correct, and we're recovering as if the fix were applied, you can combine the `err_` diagnostic and the `note_` diagnostic into one. That way, tools like `clang -fixit` will apply your fixit.
================
Comment at: lib/Parse/ParseExprCXX.cpp:2972
+ Diag(Start, diag::err_lambda_after_delete)
+ << SourceRange(Start, StartLoc.getLocWithOffset(1));
+
----------------
If you want the location of the `]` token for the end of the range here, it'd be best to save `NextToken().getLocation()` prior to calling `ParseLambdaExpression()`. Adding 1 to the location of the `[` token isn't necessarily the right location (for instance, if there's whitespace between the `[` and `]`, or if the left square bracket is spelled as `<:`).
https://reviews.llvm.org/D36357
More information about the cfe-commits
mailing list