PR19729: Delete a bunch of bogus code in Sema::FindAllocationOverload. This

caused us to perform copy-initialization for the parameters of an allocation
function called by a new-expression multiple times, resulting in us rejecting
allocations that passed non-copyable parameters (and much worse things in
MSVC compat mode, where we potentially called this function multiple times).

llvm-svn: 208724

clang/lib/Sema/SemaExpr.cpp

@@ -4164,18 +4164,14 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
                                   VariadicCallType CallType, bool AllowExplicit,
                                   bool IsListInitialization) {
   unsigned NumParams = Proto->getNumParams();
-  unsigned NumArgsToCheck = Args.size();
   bool Invalid = false;
-  if (Args.size() != NumParams)
-    // Use default arguments for missing arguments
-    NumArgsToCheck = NumParams;
   unsigned ArgIx = 0;
   // Continue to check argument types (even if we have too few/many args).
-  for (unsigned i = FirstParam; i != NumArgsToCheck; i++) {
+  for (unsigned i = FirstParam; i < NumParams; i++) {
     QualType ProtoArgType = Proto->getParamType(i);
 
     Expr *Arg;
-    ParmVarDecl *Param;
+    ParmVarDecl *Param = FDecl ? FDecl->getParamDecl(i) : nullptr;
     if (ArgIx < Args.size()) {
       Arg = Args[ArgIx++];
 
@@ -4184,11 +4180,6 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
                               diag::err_call_incomplete_argument, Arg))
         return true;
 
-      // Pass the argument
-      Param = 0;
-      if (FDecl && i < FDecl->getNumParams())
-        Param = FDecl->getParamDecl(i);
-
       // Strip the unbridged-cast placeholder expression off, if applicable.
       bool CFAudited = false;
       if (Arg->getType() == Context.ARCUnbridgedCastTy &&
@@ -4209,7 +4200,7 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
       // Remember that parameter belongs to a CF audited API.
       if (CFAudited)
         Entity.setParameterCFAudited();
-      
+
       ExprResult ArgE = PerformCopyInitialization(Entity,
                                                   SourceLocation(),
                                                   Owned(Arg),
@@ -4220,8 +4211,7 @@ bool Sema::GatherArgumentsForCall(SourceLocation CallLoc, FunctionDecl *FDecl,
 
       Arg = ArgE.takeAs<Expr>();
     } else {
-      assert(FDecl && "can't use default arguments without a known callee");
-      Param = FDecl->getParamDecl(i);
+      assert(Param && "can't use default arguments without a known callee");
 
       ExprResult ArgExpr =
         BuildCXXDefaultArgExpr(CallLoc, FDecl, Param);

clang/lib/Sema/SemaExprCXX.cpp

@@ -1414,12 +1414,14 @@ Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
 
   SmallVector<Expr *, 8> AllPlaceArgs;
   if (OperatorNew) {
-    // Add default arguments, if any.
     const FunctionProtoType *Proto =
-      OperatorNew->getType()->getAs<FunctionProtoType>();
-    VariadicCallType CallType =
-      Proto->isVariadic() ? VariadicFunction : VariadicDoesNotApply;
+        OperatorNew->getType()->getAs<FunctionProtoType>();
+    VariadicCallType CallType = Proto->isVariadic() ? VariadicFunction
+                                                    : VariadicDoesNotApply;
 
+    // We've already converted the placement args, just fill in any default
+    // arguments. Skip the first parameter because we don't have a corresponding
+    // argument.
     if (GatherArgumentsForCall(PlacementLParen, OperatorNew, Proto, 1,
                                PlacementArgs, AllPlaceArgs, CallType))
       return ExprError();
@@ -1427,6 +1429,7 @@ Sema::BuildCXXNew(SourceRange Range, bool UseGlobal,
     if (!AllPlaceArgs.empty())
       PlacementArgs = AllPlaceArgs;
 
+    // FIXME: This is wrong: PlacementArgs misses out the first (size) argument.
     DiagnoseSentinelCalls(OperatorNew, PlacementLParen, PlacementArgs);
 
     // FIXME: Missing call to CheckFunctionCall or equivalent
@@ -1684,11 +1687,6 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
     return false;
   }
 
-  // FindAllocationOverload can change the passed in arguments, so we need to
-  // copy them back.
-  if (!PlaceArgs.empty())
-    std::copy(AllocArgs.begin() + 1, AllocArgs.end(), PlaceArgs.data());
-
   // C++ [expr.new]p19:
   //
   //   If the new-expression begins with a unary :: operator, the
@@ -1832,8 +1830,22 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range,
   return false;
 }
 
-/// FindAllocationOverload - Find an fitting overload for the allocation
-/// function in the specified scope.
+/// \brief Find an fitting overload for the allocation function
+/// in the specified scope.
+///
+/// \param StartLoc The location of the 'new' token.
+/// \param SourceRange The range of the placement arguments.
+/// \param Name The name of the function ('operator new' or 'operator new[]').
+/// \param Args The placement arguments specified.
+/// \param Ctx The scope in which we should search; either a class scope or the
+///        translation unit.
+/// \param AllowMissing If \c true, report an error if we can't find any
+///        allocation functions. Otherwise, succeed but don't fill in \p
+///        Operator.
+/// \param Operator Filled in with the found allocation function. Unchanged if
+///        no allocation function was found.
+/// \param Diagnose If \c true, issue errors if the allocation function is not
+///        usable.
 bool Sema::FindAllocationOverload(SourceLocation StartLoc, SourceRange Range,
                                   DeclarationName Name, MultiExprArg Args,
                                   DeclContext *Ctx,
@@ -1879,33 +1891,11 @@ bool Sema::FindAllocationOverload(SourceLocation StartLoc, SourceRange Range,
   case OR_Success: {
     // Got one!
     FunctionDecl *FnDecl = Best->Function;
-    MarkFunctionReferenced(StartLoc, FnDecl);
-    // The first argument is size_t, and the first parameter must be size_t,
-    // too. This is checked on declaration and can be assumed. (It can't be
-    // asserted on, though, since invalid decls are left in there.)
-    // Watch out for variadic allocator function.
-    unsigned NumArgsInFnDecl = FnDecl->getNumParams();
-    for (unsigned i = 0; (i < Args.size() && i < NumArgsInFnDecl); ++i) {
-      InitializedEntity Entity = InitializedEntity::InitializeParameter(Context,
-                                                       FnDecl->getParamDecl(i));
-
-      if (!Diagnose && !CanPerformCopyInitialization(Entity, Owned(Args[i])))
-        return true;
-
-      ExprResult Result
-        = PerformCopyInitialization(Entity, SourceLocation(), Owned(Args[i]));
-      if (Result.isInvalid())
-        return true;
-
-      Args[i] = Result.takeAs<Expr>();
-    }
-
-    Operator = FnDecl;
-
     if (CheckAllocationAccess(StartLoc, Range, R.getNamingClass(),
                               Best->FoundDecl, Diagnose) == AR_inaccessible)
       return true;
 
+    Operator = FnDecl;
     return false;
   }
 

clang/test/SemaCXX/cxx0x-initializer-constructor.cpp

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++0x -fsyntax-only -verify %s
+// RUN: %clang_cc1 -std=c++0x -fsyntax-only -fexceptions -verify %s
 
 struct one { char c[1]; };
 struct two { char c[2]; };
@@ -304,7 +304,6 @@ namespace init_list_default {
   B b {}; // calls default constructor
 }
 
-
 // PR13470, <rdar://problem/11974632>
 namespace PR13470 {
   struct W {
@@ -365,3 +364,14 @@ namespace PR13470 {
     yi.h(); // ok, all diagnostics produced in template definition
   }
 }
+
+namespace PR19729 {
+  struct A {
+    A(int);
+    A(const A&) = delete;
+  };
+  struct B {
+    void *operator new(std::size_t, A);
+  };
+  B *p = new ({123}) B;
+}

clang/test/SemaCXX/microsoft-new-delete.cpp

@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fms-compatibility -fsyntax-only -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fms-compatibility -fsyntax-only -verify -std=c++11 %s
 
 #include <stddef.h>
 
@@ -10,3 +9,26 @@ void f() {
   // Expect no error in MSVC compatibility mode
   int *p = new(arbitrary) int[4];
 }
+
+class noncopyable { noncopyable(const noncopyable&); } extern nc; // expected-note {{here}}
+void *operator new[](size_t, noncopyable);
+void *operator new(size_t, const noncopyable&);
+void *q = new (nc) int[4]; // expected-error {{calling a private constructor}}
+
+struct bitfield { int n : 3; } bf; // expected-note {{here}}
+void *operator new[](size_t, int &);
+void *operator new(size_t, const int &);
+void *r = new (bf.n) int[4]; // expected-error {{non-const reference cannot bind to bit-field}}
+
+struct base {};
+struct derived : private base {} der; // expected-note {{here}}
+void *operator new[](size_t, base &);
+void *operator new(size_t, derived &);
+void *s = new (der) int[4]; // expected-error {{private}}
+
+struct explicit_ctor { explicit explicit_ctor(int); };
+struct explicit_ctor_tag {} ect;
+void *operator new[](size_t, explicit_ctor_tag, explicit_ctor);
+void *operator new(size_t, explicit_ctor_tag, int);
+void *t = new (ect, 0) int[4];
+void *u = new (ect, {0}) int[4];