Implement a better version of delegating constructor cycle detection.

This is more efficient as it's all done at once at the end of the TU.
This could still get expensive, so a flag is provided to disable it. As
an added bonus, the diagnostics will now print out a cycle.

The PCH test is XFAILed because we currently can't deal with a note
emitted in the header and I, being tired, see no other way to verify the
serialization of delegating constructors. We should probably address
this problem /somehow/ but no good solution comes to mind.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@130836 91177308-0d34-0410-b5e6-96231b3b80d8

include/clang/AST/DeclCXX.h

@@ -1619,10 +1619,9 @@ public:
   /// getTargetConstructor - When this constructor delegates to
   /// another, retrieve the target
   CXXConstructorDecl *getTargetConstructor() const {
-    if (isDelegatingConstructor())
+    assert(isDelegatingConstructor() &&
+           "A non-delegating constructor has no target");
     return CtorInitializers[0]->getTargetConstructor();
-    else
-      return 0;
   }
 
   /// isDefaultConstructor - Whether this constructor is a default

include/clang/Basic/DiagnosticSemaKinds.td

@@ -1013,8 +1013,12 @@ def err_delegation_unimplemented : Error<
   "delegating constructors are not fully implemented">;
 def err_delegating_initializer_alone : Error<
   "an initializer for a delegating constructor must appear alone">;
-def err_delegating_ctor_loop : Error<
-  "constructor %0 delegates to itself (possibly indirectly)">;
+def err_delegating_ctor_cycle : Error<
+  "constructor for %0 creates a delegation cycle">;
+def note_it_delegates_to : Note<
+  "it delegates to">;
+def note_which_delegates_to : Note<
+  "which delegates to">;
 def err_delegating_codegen_not_implemented : Error<
   "code generation for delegating constructors not implemented">;
 

include/clang/Basic/LangOptions.h

@@ -134,6 +134,9 @@ public:
   unsigned MRTD : 1;            // -mrtd calling convention
   unsigned DelayedTemplateParsing : 1;  // Delayed template parsing
 
+  // Do we try to detect cycles in delegating constructors?
+  unsigned CheckDelegatingCtorCycles : 1; 
+
 private:
   // We declare multibit enums as unsigned because MSVC insists on making enums
   // signed.  Set/Query these values using accessors.
@@ -232,6 +235,8 @@ public:
     MRTD = 0;
     DelayedTemplateParsing = 0;
     ParseUnknownAnytype = 0;
+
+    CheckDelegatingCtorCycles = 1;
   }
 
   GCMode getGCMode() const { return (GCMode) GC; }

include/clang/Driver/CC1Options.td

@@ -547,6 +547,8 @@ def fdeprecated_macro : Flag<"-fdeprecated-macro">,
   HelpText<"Defines the __DEPRECATED macro">;
 def fno_deprecated_macro : Flag<"-fno-deprecated-macro">,
   HelpText<"Undefines the __DEPRECATED macro">;
+def fno_check_delegating_ctor_cycles : Flag<"-fno-check-delegating-ctor-cycles">,
+  HelpText<"Turns off delegating constructor cycle detection">;
 
 //===----------------------------------------------------------------------===//
 // Header Search Options

include/clang/Sema/Sema.h

@@ -312,6 +312,10 @@ public:
   /// and must warn if not used. Only contains the first declaration.
   llvm::SmallVector<const DeclaratorDecl*, 4> UnusedFileScopedDecls;
 
+  /// \brief All the delegating constructors seen so far in the file, used for
+  /// cycle detection at the end of the TU.
+  llvm::SmallVector<CXXConstructorDecl*, 4> DelegatingCtorDecls;
+
   /// \brief Callback to the parser to parse templated functions when needed.
   typedef void LateTemplateParserCB(void *P, const FunctionDecl *FD);
   LateTemplateParserCB *LateTemplateParser;
@@ -704,6 +708,8 @@ public:
 
   void ActOnEndOfTranslationUnit();
 
+  void CheckDelegatingCtorCycles();
+
   Scope *getScopeForContext(DeclContext *Ctx);
 
   void PushFunctionScope();

lib/Frontend/CompilerInvocation.cpp

@@ -698,6 +698,9 @@ static void LangOptsToArgs(const LangOptions &Opts,
     Res.push_back("-fdelayed-template-parsing");
   if (Opts.Deprecated)
     Res.push_back("-fdeprecated-macro");
+
+  if (Opts.CheckDelegatingCtorCycles)
+    Res.push_back("-fcheck-delegating-ctor-cycles");
 }
 
 static void PreprocessorOptsToArgs(const PreprocessorOptions &Opts,
@@ -1565,6 +1568,8 @@ static void ParseLangArgs(LangOptions &Opts, ArgList &Args, InputKind IK,
   Opts.MRTD = Args.hasArg(OPT_mrtd);
   Opts.FakeAddressSpaceMap = Args.hasArg(OPT_ffake_address_space_map);
   Opts.ParseUnknownAnytype = Args.hasArg(OPT_funknown_anytype);
+  Opts.CheckDelegatingCtorCycles
+    = !Args.hasArg(OPT_fno_check_delegating_ctor_cycles);
 
   // Record whether the __DEPRECATED define was requested.
   Opts.Deprecated = Args.hasFlag(OPT_fdeprecated_macro,

lib/Sema/Sema.cpp

@@ -473,6 +473,9 @@ void Sema::ActOnEndOfTranslationUnit() {
 
   }
 
+  if (LangOpts.CPlusPlus0x && LangOpts.CheckDelegatingCtorCycles)
+    CheckDelegatingCtorCycles();
+
   // If there were errors, disable 'unused' warnings since they will mostly be
   // noise.
   if (!Diags.hasErrorOccurred()) {

lib/Sema/SemaDeclCXX.cpp

@@ -1547,7 +1547,8 @@ Sema::BuildDelegatingInitializer(TypeSourceInfo *TInfo,
     return true;
 
   CXXConstructExpr *ConExpr = cast<CXXConstructExpr>(DelegationInit.get());
-  CXXConstructorDecl *Constructor = ConExpr->getConstructor();
+  CXXConstructorDecl *Constructor
+    = ConExpr->getConstructor();
   assert(Constructor && "Delegating constructor with no target?");
 
   CheckImplicitConversions(DelegationInit.get(), LParenLoc);
@@ -2072,23 +2073,7 @@ static bool CollectFieldInitializer(BaseAndFieldInfo &Info,
 bool
 Sema::SetDelegatingInitializer(CXXConstructorDecl *Constructor,
                                CXXCtorInitializer *Initializer) {
-  CXXConstructorDecl *Target = Initializer->getTargetConstructor();
-  CXXConstructorDecl *Canonical = Constructor->getCanonicalDecl();
-  while (Target) {
-    if (Target->getCanonicalDecl() == Canonical) {
-      Diag(Initializer->getSourceLocation(), diag::err_delegating_ctor_loop)
-        << Constructor;
-      return true;
-    }
-    Target = Target->getTargetConstructor();
-  }
-
-  // We do the cycle detection first so that we know that we're not
-  // going to create a cycle by inserting this link. This ensures that
-  // the AST is cycle-free and we don't get a scenario where we have
-  // a B -> C -> B cycle and then add an A -> B link and get stuck in
-  // an infinite loop as we check for cycles with A and never get there
-  // because we get stuck in a cycle not including A.
+  assert(Initializer->isDelegatingInitializer());
   Constructor->setNumCtorInitializers(1);
   CXXCtorInitializer **initializer =
     new (Context) CXXCtorInitializer*[1];
@@ -2100,10 +2085,12 @@ Sema::SetDelegatingInitializer(CXXConstructorDecl *Constructor,
     DiagnoseUseOfDecl(Dtor, Initializer->getSourceLocation());
   }
 
+  if (LangOpts.CheckDelegatingCtorCycles)
+    DelegatingCtorDecls.push_back(Constructor);
+
   return false;
 }
                                
-                               
 bool
 Sema::SetCtorInitializers(CXXConstructorDecl *Constructor,
                                   CXXCtorInitializer **Initializers,
@@ -2479,7 +2466,7 @@ void Sema::ActOnMemInitializers(Decl *ConstructorDecl,
         HadError = true;
         // We will treat this as being the only initializer.
       }
-      SetDelegatingInitializer(Constructor, *MemInits);
+      SetDelegatingInitializer(Constructor, MemInits[i]);
       // Return immediately as the initializer is set.
       return;
     }
@@ -7957,3 +7944,53 @@ void Sema::SetIvarInitializers(ObjCImplementationDecl *ObjCImplementation) {
                                             AllToInit.data(), AllToInit.size());
   }
 }
+
+void Sema::CheckDelegatingCtorCycles() {
+  llvm::SmallSet<CXXConstructorDecl*, 4> Valid, Invalid, Current;
+
+  llvm::SmallSet<CXXConstructorDecl*, 4>::iterator ci = Current.begin(),
+                                                   ce = Current.end();
+
+  for (llvm::SmallVector<CXXConstructorDecl*, 4>::iterator
+         i = DelegatingCtorDecls.begin(),
+         e = DelegatingCtorDecls.end();
+       i != e; ++i) {
+    const FunctionDecl *FNTarget;
+    CXXConstructorDecl *Target;
+    (*i)->getTargetConstructor()->hasBody(FNTarget);
+    Target
+      = const_cast<CXXConstructorDecl*>(cast<CXXConstructorDecl>(FNTarget));
+
+    if (!Target || !Target->isDelegatingConstructor() || Valid.count(Target)) {
+      Valid.insert(*i);
+      for (ci = Current.begin(), ce = Current.end(); ci != ce; ++ci)
+        Valid.insert(*ci);
+      Current.clear();
+    } else if (Target == *i || Invalid.count(Target) || Current.count(Target)) {
+      if (!Invalid.count(Target)) {
+        Diag((*(*i)->init_begin())->getSourceLocation(),
+             diag::err_delegating_ctor_cycle)
+          << *i;
+        if (Target != *i)
+          Diag(Target->getLocation(), diag::note_it_delegates_to);
+        CXXConstructorDecl *Current = Target;
+        while (Current != *i) {
+          Current->getTargetConstructor()->hasBody(FNTarget);
+          Current
+         = const_cast<CXXConstructorDecl*>(cast<CXXConstructorDecl>(FNTarget));
+          Diag(Current->getLocation(), diag::note_which_delegates_to);
+        }
+      }
+
+      (*i)->setInvalidDecl();
+      Invalid.insert(*i);
+      for (ci = Current.begin(), ce = Current.end(); ci != ce; ++ci) {
+        (*ci)->setInvalidDecl();
+        Invalid.insert(*i);
+      }
+      Current.clear();
+    } else {
+      Current.insert(*i);
+    }
+  }
+}

test/PCH/cxx0x-delegating-ctors.cpp

@@ -5,4 +5,8 @@
 // RUN: %clang_cc1 -x c++-header -std=c++0x -emit-pch -o %t %S/cxx0x-delegating-ctors.h
 // RUN: %clang_cc1 -std=c++0x -include-pch %t -fsyntax-only -verify %s 
 
-foo::foo() : foo(1) { } // expected-error{{delegates to itself}}
+// Currently we can't deal with a note in the header
+// XFAIL: *
+
+foo::foo() : foo(1) { } // expected-error{{creates a delegation cycle}} \
+                        // expected-note{{which delegates to}}

test/PCH/cxx0x-delegating-ctors.h

@@ -1,6 +1,6 @@
 // Header for PCH test cxx0x-delegating-ctors.cpp
 
 struct foo {
-  foo(int) : foo() { }
+  foo(int) : foo() { } // expected-note{{it delegates to}}
   foo();
 };

test/SemaCXX/cxx0x-delegating-ctors.cpp

@@ -22,14 +22,15 @@ foo::foo () : foo(-1) {
 foo::foo (int, int) : foo() {
 }
 
-foo::foo (bool) : foo(true) { // expected-error{{delegates to itself}}
+foo::foo (bool) : foo(true) { // expected-error{{creates a delegation cycle}}
 }
 
 // Good
-foo::foo (const float* f) : foo(*f) {
+foo::foo (const float* f) : foo(*f) { // expected-note{{it delegates to}}
 }
 
-foo::foo (const float &f) : foo(&f) { //expected-error{{delegates to itself}}
+foo::foo (const float &f) : foo(&f) { //expected-error{{creates a delegation cycle}} \
+                                      //expected-note{{which delegates to}}
 }
 
 foo::foo (char) : i(3), foo(3) { // expected-error{{must appear alone}}