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

[Nicolas Lesser via Phabricator via cfe-commits]

Rakete1111 updated this revision to Diff 199380.
Rakete1111 added a comment.

- Addressed review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D36357

Files:
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/lib/Parse/ParseExprCXX.cpp
  clang/test/FixIt/fixit-cxx0x.cpp
  clang/test/Parser/cxx0x-lambda-expressions.cpp
  clang/test/SemaCXX/new-delete-0x.cpp


Index: clang/test/SemaCXX/new-delete-0x.cpp
===================================================================
--- clang/test/SemaCXX/new-delete-0x.cpp
+++ clang/test/SemaCXX/new-delete-0x.cpp
@@ -34,6 +34,6 @@
 void bad_deletes()
 {
   // 'delete []' is always array delete, per [expr.delete]p1.
-  // FIXME: Give a better diagnostic.
-  delete []{ return (int*)0; }(); // expected-error {{expected expression}}
+  delete []{ return (int*)0; }(); // expected-error {{'[]' after delete interpreted as 'delete[]'}}
 }
+
Index: clang/test/Parser/cxx0x-lambda-expressions.cpp
===================================================================
--- clang/test/Parser/cxx0x-lambda-expressions.cpp
+++ clang/test/Parser/cxx0x-lambda-expressions.cpp
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wno-unused-value -verify -std=c++2a %s
 
 enum E { e };
 
@@ -53,8 +54,14 @@
   void delete_lambda(int *p) {
     delete [] p;
     delete [] (int*) { new int }; // ok, compound-literal, not lambda
-    delete [] { return new int; } (); // expected-error{{expected expression}}
+    delete [] { return new int; } (); // expected-error {{'[]' after delete interpreted as 'delete[]'}}
     delete [&] { return new int; } (); // ok, lambda
+
+    delete []() { return new int; }(); // expected-error{{'[]' after delete interpreted as 'delete[]'}}
+    delete [](E Enum) { return new int((int)Enum); }(e); // expected-error{{'[]' after delete interpreted as 'delete[]'}}
+#if __cplusplus > 201703L
+    delete []<int = 0> { return new int; }(); // expected-error{{'[]' after delete interpreted as 'delete[]'}}
+#endif
   }
 
   // We support init-captures in C++11 as an extension.
Index: clang/test/FixIt/fixit-cxx0x.cpp
===================================================================
--- clang/test/FixIt/fixit-cxx0x.cpp
+++ clang/test/FixIt/fixit-cxx0x.cpp
@@ -58,6 +58,9 @@
   (void)[&, i, i]{ }; // expected-error{{'i' can appear only once in a capture list}}
   (void)[] mutable { }; // expected-error{{lambda requires '()' before 'mutable'}}
   (void)[] -> int { }; // expected-error{{lambda requires '()' before return type}}
+
+  delete []() { return new int; }(); // expected-error{{'[]' after delete interpreted as 'delete[]'}}
+  delete [] { return new int; }(); // expected-error{{'[]' after delete interpreted as 'delete[]'}}
 }
 
 #define bar "bar"
Index: clang/lib/Parse/ParseExprCXX.cpp
===================================================================
--- clang/lib/Parse/ParseExprCXX.cpp
+++ clang/lib/Parse/ParseExprCXX.cpp
@@ -2984,8 +2984,38 @@
     //   [Footnote: A lambda expression with a lambda-introducer that consists
     //              of empty square brackets can follow the delete keyword if
     //              the lambda expression is enclosed in parentheses.]
-    // FIXME: Produce a better diagnostic if the '[]' is unambiguously a
-    //        lambda-introducer.
+
+    const Token Next = GetLookAheadToken(2);
+
+    // Basic lookahead to check if we have a lambda expression.
+    if (Next.isOneOf(tok::l_brace, tok::less) ||
+        (Next.is(tok::l_paren) &&
+         (GetLookAheadToken(3).is(tok::r_paren) ||
+          (GetLookAheadToken(3).is(tok::identifier) &&
+           GetLookAheadToken(4).is(tok::identifier))))) {
+      SourceLocation RSquareLock = NextToken().getLocation();
+      // Warn that the non-capturing lambda isn't surrounded by parentheses
+      // to disambiguate it from 'delete[]'.
+      ExprResult Lambda = ParseLambdaExpression();
+      if (Lambda.isInvalid())
+        return ExprError();
+
+      SourceLocation StartLoc = Lambda.get()->getBeginLoc();
+      Diag(Start, diag::err_lambda_after_delete)
+          << SourceRange(Start, RSquareLock)
+          << FixItHint::CreateInsertion(StartLoc, "(")
+          << FixItHint::CreateInsertion(
+                 Lexer::getLocForEndOfToken(Lambda.get()->getEndLoc(), 0,
+                                            Actions.getSourceManager(),
+                                            getLangOpts()),
+                 ")");
+
+      // Evaluate any postfix expressions used on the lambda.
+      Lambda = ParsePostfixExpressionSuffix(Lambda);
+      return Actions.ActOnCXXDelete(Start, UseGlobal, /*ArrayForm=*/false,
+                                    Lambda.get());
+    }
+
     ArrayDelete = true;
     BalancedDelimiterTracker T(*this, tok::l_square);
 
Index: clang/include/clang/Basic/DiagnosticParseKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticParseKinds.td
+++ clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -105,6 +105,8 @@
   InGroup<CXX98Compat>, DefaultIgnore;
 def ext_alignof_expr : ExtWarn<
   "%0 applied to an expression is a GNU extension">, InGroup<GNUAlignofExpression>;
+def err_lambda_after_delete : Error<
+  "'[]' after delete interpreted as 'delete[]'; add parentheses to treat this as a lambda-expression">;
 
 def warn_microsoft_dependent_exists : Warning<
   "dependent %select{__if_not_exists|__if_exists}0 declarations are ignored">,

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits