PR49260: Improve diagnostics for no matching 'operator new'.

Fix duplicate diagnostic for an over-aligned allocation with no matching
function, and add custom diagnostic for the case where the
non-allocating placement new was intended but <new> was not included.

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -7188,6 +7188,9 @@ def err_need_header_before_typeid : Error<
   "you need to include <typeinfo> before using the 'typeid' operator">;
 def err_need_header_before_ms_uuidof : Error<
   "you need to include <guiddef.h> before using the '__uuidof' operator">;
+def err_need_header_before_placement_new : Error<
+  "no matching %0 function for non-allocating placement new expression; "
+  "include <new>">;
 def err_ms___leave_not_in___try : Error<
   "'__leave' statement not in __try block">;
 def err_uuidof_without_guid : Error<

clang/lib/Sema/SemaExprCXX.cpp

@@ -2458,12 +2458,27 @@ static bool resolveAllocationOverload(
     }
 
     if (Diagnose) {
-      PartialDiagnosticAt PD(R.getNameLoc(), S.PDiag(diag::err_ovl_no_viable_function_in_call)
+      // If this is an allocation of the form 'new (p) X' for some object
-          << R.getLookupName() << Range);
+      // pointer p (or an expression that will decay to such a pointer),
+      // diagnose the missing inclusion of <new>.
+      if (!R.isClassLookup() && Args.size() == 2 &&
+          (Args[1]->getType()->isObjectPointerType() ||
+           Args[1]->getType()->isArrayType())) {
+        S.Diag(R.getNameLoc(), diag::err_need_header_before_placement_new)
+            << R.getLookupName() << Range;
+        // Listing the candidates is unlikely to be useful; skip it.
+        return true;
+      }
 
-      // If we have aligned candidates, only note the align_val_t candidates
+      // Finish checking all candidates before we note any. This checking can
-      // from AlignedCandidates and the non-align_val_t candidates from
+      // produce additional diagnostics so can't be interleaved with our
-      // Candidates.
+      // emission of notes.
+      //
+      // For an aligned allocation, separately check the aligned and unaligned
+      // candidates with their respective argument lists.
+      SmallVector<OverloadCandidate*, 32> Cands;
+      SmallVector<OverloadCandidate*, 32> AlignedCands;
+      llvm::SmallVector<Expr*, 4> AlignedArgs;
       if (AlignedCandidates) {
         auto IsAligned = [](OverloadCandidate &C) {
           return C.Function->getNumParams() > 1 &&
@@ -2471,17 +2486,26 @@ static bool resolveAllocationOverload(
         };
         auto IsUnaligned = [&](OverloadCandidate &C) { return !IsAligned(C); };
 
-        // This was an overaligned allocation, so list the aligned candidates
+        AlignedArgs.reserve(Args.size() + 1);
-        // first.
+        AlignedArgs.push_back(Args[0]);
-        Args.insert(Args.begin() + 1, AlignArg);
+        AlignedArgs.push_back(AlignArg);
-        AlignedCandidates->NoteCandidates(PD, S, OCD_AllCandidates, Args, "",
+        AlignedArgs.append(Args.begin() + 1, Args.end());
-                                          R.getNameLoc(), IsAligned);
+        AlignedCands = AlignedCandidates->CompleteCandidates(
-        Args.erase(Args.begin() + 1);
+            S, OCD_AllCandidates, AlignedArgs, R.getNameLoc(), IsAligned);
-        Candidates.NoteCandidates(PD, S, OCD_AllCandidates, Args, "", R.getNameLoc(),
+
-                                  IsUnaligned);
+        Cands = Candidates.CompleteCandidates(S, OCD_AllCandidates, Args,
+                                              R.getNameLoc(), IsUnaligned);
       } else {
-        Candidates.NoteCandidates(PD, S, OCD_AllCandidates, Args);
+        Cands = Candidates.CompleteCandidates(S, OCD_AllCandidates, Args,
+                                              R.getNameLoc());
       }
+
+      S.Diag(R.getNameLoc(), diag::err_ovl_no_viable_function_in_call)
+          << R.getLookupName() << Range;
+      if (AlignedCandidates)
+        AlignedCandidates->NoteCandidates(S, AlignedArgs, AlignedCands, "",
+                                          R.getNameLoc());
+      Candidates.NoteCandidates(S, Args, Cands, "", R.getNameLoc());
     }
     return true;
 

clang/test/SemaCXX/new-delete.cpp

@@ -32,10 +32,10 @@ inline void *operator new(size_t) { // no warning, due to __attribute__((used))
 }
 
 // PR5823
-void* operator new(const size_t); // expected-note 2 {{candidate}}
+void* operator new(const size_t); // expected-note {{candidate}}
-void* operator new(size_t, int*); // expected-note 3 {{candidate}}
+void* operator new(size_t, int*); // expected-note 2{{candidate}}
-void* operator new(size_t, float*); // expected-note 3 {{candidate}}
+void* operator new(size_t, float*); // expected-note 2{{candidate}}
-void* operator new(size_t, S); // expected-note 2 {{candidate}}
+void* operator new(size_t, S); // expected-note {{candidate}}
 
 struct foo { };
 
@@ -130,7 +130,7 @@ void bad_news(int *ip)
   (void)new (0, 0) int; // expected-error {{no matching function for call to 'operator new'}}
   (void)new (0L) int; // expected-error {{call to 'operator new' is ambiguous}}
   // This must fail, because the member version shouldn't be found.
-  (void)::new ((S*)0) U; // expected-error {{no matching function for call to 'operator new'}}
+  (void)::new ((S*)0) U; // expected-error {{no matching 'operator new' function for non-allocating placement new expression; include <new>}}
   // This must fail, because any member version hides all global versions.
   (void)new U; // expected-error {{no matching function for call to 'operator new'}}
   (void)new (int[]); // expected-error {{array size must be specified in new expression with no initializer}}
@@ -143,6 +143,14 @@ void bad_news(int *ip)
 #endif
 }
 
+void no_matching_placement_new() {
+  struct X { int n; };
+  __attribute__((aligned(__alignof(X)))) unsigned char buffer[sizeof(X)];
+  (void)new(buffer) X; // expected-error {{no matching 'operator new' function for non-allocating placement new expression; include <new>}}
+  (void)new(+buffer) X; // expected-error {{no matching 'operator new' function for non-allocating placement new expression; include <new>}}
+  (void)new(&buffer) X; // expected-error {{no matching 'operator new' function for non-allocating placement new expression; include <new>}}
+}
+
 void good_deletes()
 {
   delete (int*)0;

clang/test/SemaCXX/std-align-val-t-in-operator-new.cpp

@@ -21,7 +21,7 @@ enum align_val_t {
 #endif
 } // namespace std
 
-void *operator new(std::size_t count, std::align_val_t al) __attribute__((alloc_align(2)));
+void *operator new(std::size_t count, std::align_val_t al) __attribute__((alloc_align(2))); // #1
 
 #define OVERALIGNED alignas(__STDCPP_DEFAULT_NEW_ALIGNMENT__ * 2)
 
@@ -55,3 +55,29 @@ void *alloc_overaligned_struct_with_extra_255_alignment(int align) {
 std::align_val_t align_variable(int align) { return std::align_val_t(align); }
 std::align_val_t align_align16() { return std::align_val_t(16); }
 std::align_val_t align_align15() { return std::align_val_t(15); }
+
+struct X {};
+void *operator new(std::size_t, X); // #2
+void *operator new(std::size_t, std::align_val_t, X); // #3
+// FIXME: Consider improving notes 1 and 3 here to say that these are aligned
+// allocation functions and the type is not over-aligned.
+X *p = new (123) X; // expected-error {{no matching function}}
+// expected-note@#1 {{no known conversion from 'int' to 'std::align_val_t' for 2nd argument}}
+// expected-note@#2 {{no known conversion from 'int' to 'X' for 2nd argument}}
+// expected-note@#3 {{requires 3 arguments}}
+// expected-note@* {{requires 1 argument, but 2 were provided}} (builtin)
+
+#ifdef __cpp_aligned_new
+struct alignas(__STDCPP_DEFAULT_NEW_ALIGNMENT__ * 2) Y {};
+Y *q = new (123) Y; // expected-error {{no matching function}}
+// expected-note@#1 {{requires 2 arguments, but 3 were provided}}
+// expected-note@#2 {{no known conversion from 'int' to 'X' for 2nd argument}}
+// expected-note@#3 {{no known conversion from 'int' to 'X' for 3rd argument}}
+// expected-note@* {{requires 1 argument, but 2 were provided}} (builtin)
+#endif
+
+X *r = new (std::align_val_t(32), 123) X; // expected-error {{no matching function}}
+// expected-note@#1 {{requires 2 arguments, but 3 were provided}}
+// expected-note@#2 {{requires 2 arguments, but 3 were provided}}
+// expected-note@#3 {{no known conversion from 'int' to 'X' for 3rd argument}}
+// expected-note@* {{requires 1 argument, but 3 were provided}} (builtin)