[PATCH] D36357: Added a better diagnostic when using the delete operator with lambdas

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

Thanks! Some minor nits, please feel free to commit once they're addressed.

In D36357#1177852 <https://reviews.llvm.org/D36357#1177852>, @Rakete1111 wrote:

> Note that clang doesn't support the fourth kind of lambda yet ([]<>), because D36527 <https://reviews.llvm.org/D36527> hasn't been merged yet, so I didn't add a test case for that one.


We support that now, so please add a test for that :)



================
Comment at: clang/include/clang/Basic/DiagnosticParseKinds.td:109
+def err_lambda_after_delete : Error<
+  "'[]' after delete interpreted as 'delete[]'">;
 
----------------
Please add to this: "; add parentheses to treat this as a lambda-expression" or similar.


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:2996
+           GetLookAheadToken(4).is(tok::identifier))))) {
+      SourceLocation RightBracketLock = NextToken().getLocation();
+      // Warn if the non-capturing lambda isn't surrounded by parenthesis
----------------
`RightBracketLock` -> `RSquareLoc`

(Our convention is to use `Loc` for "location" and to avoid the word "bracket" because it means different things in different English dialects -- usually `[]` in US English and usually `()` in UK English.)


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:2997
+      SourceLocation RightBracketLock = NextToken().getLocation();
+      // Warn if the non-capturing lambda isn't surrounded by parenthesis
+      // to disambiguate it from 'delete[]'.
----------------
if -> that
parenthesis -> parentheses


================
Comment at: clang/lib/Parse/ParseExprCXX.cpp:3014
+      // Evaluate any postfix expressions used on the lambda.
+      Lambda = ParsePostfixExpressionSuffix(Lambda);
+      return Actions.ActOnCXXDelete(Start, UseGlobal, /*ArrayForm=*/false,
----------------
Maybe we should do this before producing the diagnostic so that we can suggest inserting the `)` after the complete expression? (But I don't have a strong preference either way.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D36357/new/

https://reviews.llvm.org/D36357