[clang] [clang] Simplify the overload resolution logic for operator new and new[] (PR #211482)

[Oliver Hunt via cfe-commits]

================
@@ -2918,55 +2880,184 @@ static void LookupGlobalDeallocationFunctions(Sema &S, SourceLocation Loc,
   }
 }
 
-static bool resolveAllocationOverload(
-    Sema &S, LookupResult &R, SourceRange Range, SmallVectorImpl<Expr *> &Args,
-    ImplicitAllocationParameters &IAP, FunctionDecl *&Operator,
-    OverloadCandidateSet *AlignedCandidates, Expr *AlignArg, bool Diagnose) {
-  Operator = nullptr;
-  if (isTypeAwareAllocation(IAP.PassTypeIdentity)) {
-    assert(S.isStdTypeIdentity(Args[0]->getType(), nullptr));
-    // The internal overload resolution work mutates the argument list
-    // in accordance with the spec. We may want to change that in future,
-    // but for now we deal with this by making a copy of the non-type-identity
-    // arguments.
-    SmallVector<Expr *> UntypedParameters;
-    UntypedParameters.reserve(Args.size() - 1);
-    UntypedParameters.push_back(Args[1]);
-    // Type aware allocation implicitly includes the alignment parameter so
-    // only include it in the untyped parameter list if alignment was explicitly
-    // requested
-    if (isAlignedAllocation(IAP.PassAlignment))
-      UntypedParameters.push_back(Args[2]);
-    UntypedParameters.append(Args.begin() + 3, Args.end());
-
-    AlignedAllocationMode InitialAlignmentMode = IAP.PassAlignment;
-    IAP.PassAlignment = AlignedAllocationMode::Yes;
-    if (resolveAllocationOverloadInterior(
-            S, R, Range, ResolveMode::Typed, Args, IAP.PassAlignment, Operator,
-            AlignedCandidates, AlignArg, Diagnose))
+static void DiagnoseAllocationLookupFailure(
+    Sema &SemaRef, LookupResult &R, SourceRange Range,
+    std::optional<AllocationArgumentSet> &ArgumentCandidates,
+    ArrayRef<Expr *> PlacementArguments) {
+  ImplicitAllocationArguments *UnalignedArgumentList = nullptr;
+  ImplicitAllocationArguments *AlignedArgumentList = nullptr;
+  for (ImplicitAllocationArguments &AllocationArguments :
+       ArgumentCandidates->Candidates) {
+    if (AllocationArguments.PassTypeIdentity == TypeAwareAllocationMode::Yes)
+      continue;
+    if (AllocationArguments.PassAlignment == AlignedAllocationMode::Yes)
+      AlignedArgumentList = &AllocationArguments;
+    else
+      UnalignedArgumentList = &AllocationArguments;
+  }
+  if (!UnalignedArgumentList)
+    return;
+
+  // We re-resolve the rejected candidates for diagnostics rather than requiring
+  // them to be tracked during the initial resolution path. This both simplifies
+  // the resolution logic, and helps with performance.
+  auto Rerun = [&](ImplicitAllocationArguments &ArgumentList,
+                   OverloadCandidateSet &Candidates,
+                   SmallVectorImpl<Expr *> &Args) {
+    llvm::append_range(Args, ArgumentList.getImplicitArguments());
+    llvm::append_range(Args, PlacementArguments);
+    FunctionDecl *Unused = nullptr;
+    resolveAllocationOverload(SemaRef, R, Range, ArgumentList, Args, Unused,
+                              Candidates, /*Diagnose=*/false);
+  };
+  std::optional<OverloadCandidateSet> AlignedCandidates;
+  Expr *AlignArg = nullptr;
+  if (AlignedArgumentList) {
+    AlignedCandidates.emplace(R.getNameLoc(), OverloadCandidateSet::CSK_Normal);
+    SmallVector<Expr *, 4> AlignedArgs;
+    Rerun(*AlignedArgumentList, *AlignedCandidates, AlignedArgs);
+    AlignArg = AlignedArgumentList->getAlignmentArgument();
+  }
+  OverloadCandidateSet UnalignedCandidates(R.getNameLoc(),
+                                           OverloadCandidateSet::CSK_Normal);
+  SmallVector<Expr *, 4> UnalignedArgs;
+  Rerun(*UnalignedArgumentList, UnalignedCandidates, UnalignedArgs);
+  diagnoseNoViableFunctionForAllocationOverloadResolution(
+      SemaRef, R, Range, UnalignedArgs, UnalignedCandidates,
+      AlignedCandidates ? &*AlignedCandidates : nullptr, AlignArg);
+}
+
+bool Sema::getTypeIdentityArgument(QualType Type, SourceLocation Loc,
+                                   Expr **FoundExpr) {
+  auto [Slot, Inserted] =
+      AllocationTypeIdentityArguments.insert({Type, nullptr});
+  if (!Inserted) {
+    if (!Slot->second)
       return true;
-    if (Operator)
-      return false;
+    *FoundExpr = Slot->second;
+    return false;
+  }
+  QualType TypeIdentity = tryBuildStdTypeIdentity(Type, SourceLocation());
+  if (TypeIdentity.isNull())
+    return false;
----------------
ojhunt wrote:

Yeah. I was on the fence here, it means you only get one error on the instantiation failing, which is less noisy, on the other hand the only way to get here is an incorrectly defined std::type_identity.

I'm really ok with either, this was the behavior I settled on, but only because I had to choose one approach (no `if (random()%2) { ... } else {. ..}` :D ). The case where this triggers is extremely dumb.

https://github.com/llvm/llvm-project/pull/211482