Try to fix PR16239

[wwwwpan]

Summary

• In this bug, Sema is performing a constructor initialization, and it's associated argument conversions invoke this constructor initialization again. This causes an infinite recursion.

[dblaikie]

Richard - this seems like a very direct approach (which might be right, or it's just a hack without knowing the depth to implement other solutions). Did/do you have other ideas/approaches in mind?

[rsmith]

This is the wrong approach for this issue. See

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1673

for the latest wording of the approach that should be used to address this issue.

include/clang/Sema/Sema.h

public:
   void *VisContext; // Really a "PragmaVisStack*"
 
   /// \brief Flag indicating if Sema is building a recovery call expression.
   ///
   /// This flag is used to avoid building recovery call expressions
   /// if Sema is already doing so, which would cause infinite recursions.
   bool IsBuildingRecoveryCallExpr;
 
+  /// \brief All constructors in construction. This is to detect if 
+  /// converting the arguments calls any constructor being constructed,
+  /// which leads to an infinite recursion.
+  llvm::SmallVector<CXXConstructorDecl *, 2> CtorsInConstruction;
+
   /// ExprNeedsCleanups - True if the current evaluation context
   /// requires cleanups to be run at its conclusion.
   bool ExprNeedsCleanups;
 
   /// ExprCleanupObjects - This is the stack of objects requiring
   /// cleanup that are created by the current full expression.  The
   /// element type here is ExprWithCleanups::Object.
   SmallVector<BlockDecl*, 8> ExprCleanupObjects;

lib/Sema/SemaDeclCXX.cpp

Sema::CompleteConstructorCall(CXXConstructorDecl *Constructor,
   unsigned NumArgsInProto = Proto->getNumArgs();
   
   // If too few arguments are available, we'll fill in the rest with defaults.
   if (NumArgs < NumArgsInProto)
     ConvertedArgs.reserve(NumArgsInProto);
   else
     ConvertedArgs.reserve(NumArgs);
 
+  // Mark this constructor being constructed so that its argument construction
+  // should not call itself.
+  CXXConstructorDecl *Ctor = Constructor;
+  if (FunctionTemplateDecl *FuncTmpl = Ctor->getPrimaryTemplate())
+    Ctor = cast<CXXConstructorDecl>(FuncTmpl->getTemplatedDecl());
+  CtorsInConstruction.push_back(Ctor);
+
   VariadicCallType CallType = 
     Proto->isVariadic() ? VariadicConstructor : VariadicDoesNotApply;
   SmallVector<Expr *, 8> AllArgs;
   bool Invalid = GatherArgumentsForCall(Loc, Constructor,
                                         Proto, 0,
                                         llvm::makeArrayRef(Args, NumArgs),
                                         AllArgs,
                                         CallType, AllowExplicit,
                                         IsListInitialization);
   ConvertedArgs.append(AllArgs.begin(), AllArgs.end());
 
   DiagnoseSentinelCalls(Constructor, Loc, AllArgs);
 
   CheckConstructorCall(Constructor,
                        llvm::makeArrayRef<const Expr *>(AllArgs.data(),
                                                         AllArgs.size()),
                        Proto, Loc);
 
+  // Cleanup once finished the constructor call.
+  CtorsInConstruction.pop_back();
+
   return Invalid;
 }
 
 static inline bool
 CheckOperatorNewDeleteDeclarationScope(Sema &SemaRef, 
                                        const FunctionDecl *FnDecl) {
   const DeclContext *DC = FnDecl->getDeclContext()->getRedeclContext();
   if (isa<NamespaceDecl>(DC)) {

lib/Sema/SemaInit.cpp

else {
       // If we're performing copy initialization using a copy constructor, we
       // suppress user-defined conversions on the arguments. We do the same for
       // move constructors.
       if ((CopyInitializing || (InitListSyntax && Args.size() == 1)) &&
           Constructor->isCopyOrMoveConstructor())
         SuppressUserConversions = true;
     }
 
-    if (!Constructor->isInvalidDecl() &&
+    // If Sema is already building this constructor call, then this constructor
+    // cannot be a candidate for initializing its arguments. Otherwise, this
+    // leads to an infinite recursion.
+    bool InConstruction =
+      S.CtorsInConstruction.end() != std::find(S.CtorsInConstruction.begin(),
+                                               S.CtorsInConstruction.end(),
+                                               Constructor);
+
+    if (!Constructor->isInvalidDecl() && !InConstruction &&
         (AllowExplicit || !Constructor->isExplicit()) &&
         (!OnlyListConstructors || S.isInitListConstructor(Constructor))) {
       if (ConstructorTmpl)
         S.AddTemplateOverloadCandidate(ConstructorTmpl, FoundDecl,
                                        /*ExplicitArgs*/ 0, Args,
                                        CandidateSet, SuppressUserConversions);
       else {
         // C++ [over.match.copy]p1:

test/SemaCXX/constructor.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s 
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s 
 typedef int INT;
 
 class Foo {
   Foo();
   (Foo)(float) { }
   explicit Foo(int); // expected-note {{previous declaration is here}}
   Foo(const Foo&);
 

 A::S::S(int) {}
 
 void A::S::f2() {}
 
 A::S::operator int() { return 1; }
 
 A::S::~S() {}
 
+namespace PR16239 {
+struct Foo;
+struct Bar;
+
+struct Move2 {
+  Move2(Foo f); // expected-note {{passing argument to parameter 'f' here}}
+  Move2(Bar b); // expected-note {{passing argument to parameter 'b' here}}
+};
+
+struct Foo {
+  Foo(const Move2&);
+
+  Foo(Foo&); // expected-note {{candidate constructor not viable}}
+};
+
+struct Bar {
+  template <typename T = int>
+  Bar(const Move2&, T t = T());
+
+  Bar(Bar&); // expected-note {{candidate constructor not viable}}
+};
+
+Foo func_foo();
+Bar func_bar();
+
+void Baz() {
+  Foo f = func_foo(); // expected-error{{no matching constructor for initialization}}
+  Bar b = func_bar(); // expected-error{{no matching constructor for initialization}}
+}
+
+} // namespace