[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