]> granicus.if.org Git - clang/commitdiff
there i fixed it
authorSean Hunt <scshunt@csclub.uwaterloo.ca>
Wed, 4 May 2011 23:29:54 +0000 (23:29 +0000)
committerSean Hunt <scshunt@csclub.uwaterloo.ca>
Wed, 4 May 2011 23:29:54 +0000 (23:29 +0000)
Increase robustness of the delegating constructor cycle detection
mechanism. No more infinite loops on invalid or logic errors leading to
false results. Ensure that this is maintained correctly accross
serialization.

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

include/clang/Serialization/ASTBitCodes.h
include/clang/Serialization/ASTReader.h
lib/Sema/SemaDeclCXX.cpp
lib/Serialization/ASTReader.cpp
lib/Serialization/ASTWriter.cpp
test/PCH/cxx0x-delegating-ctors.cpp
test/PCH/cxx0x-delegating-ctors.h [deleted file]

index 0905b43f878f9b88bfd56add68d73a4ff138642f..7b9e98d34370de4402bdf4c74f881d4fbc748e57 100644 (file)
@@ -362,7 +362,10 @@ namespace clang {
       FP_PRAGMA_OPTIONS = 42,
 
       /// \brief Record code for enabled OpenCL extensions.
-      OPENCL_EXTENSIONS = 43
+      OPENCL_EXTENSIONS = 43,
+
+      /// \brief The list of delegating constructor declarations.
+      DELEGATING_CTORS = 44
     };
 
     /// \brief Record types used within a source manager block.
index da018ab99e659e3c6320a138ab01ff553899370d..fd073e29aa95439b5b0cdcbb3e7fb0fcbe64b2ab 100644 (file)
@@ -54,6 +54,7 @@ class Decl;
 class DeclContext;
 class NestedNameSpecifier;
 class CXXBaseSpecifier;
+class CXXConstructorDecl;
 class CXXCtorInitializer;
 class GotoStmt;
 class MacroDefinition;
@@ -564,6 +565,10 @@ private:
   /// generating warnings.
   llvm::SmallVector<uint64_t, 16> UnusedFileScopedDecls;
 
+  /// \brief A list of all the delegating constructors we've seen, to diagnose
+  /// cycles.
+  llvm::SmallVector<uint64_t, 4> DelegatingCtorDecls;
+
   /// \brief A snapshot of Sema's weak undeclared identifier tracking, for
   /// generating warnings.
   llvm::SmallVector<uint64_t, 64> WeakUndeclaredIdentifiers;
index 7b16d63a48144a201105e40589741a8018da9f8a..bb732e2c9feab73c1384357c15fa563eaf6b7b4f 100644 (file)
@@ -7945,52 +7945,85 @@ void Sema::SetIvarInitializers(ObjCImplementationDecl *ObjCImplementation) {
   }
 }
 
-void Sema::CheckDelegatingCtorCycles() {
-  llvm::SmallSet<CXXConstructorDecl*, 4> Valid, Invalid, Current;
+static
+void DelegatingCycleHelper(CXXConstructorDecl* Ctor,
+                           llvm::SmallSet<CXXConstructorDecl*, 4> &Valid,
+                           llvm::SmallSet<CXXConstructorDecl*, 4> &Invalid,
+                           llvm::SmallSet<CXXConstructorDecl*, 4> &Current,
+                           Sema &S) {
+  llvm::SmallSet<CXXConstructorDecl*, 4>::iterator CI = Current.begin(),
+                                                   CE = Current.end();
+  if (Ctor->isInvalidDecl())
+    return;
 
-  llvm::SmallSet<CXXConstructorDecl*, 4>::iterator ci = Current.begin(),
-                                                   ce = Current.end();
+  const FunctionDecl *FNTarget = 0;
+  CXXConstructorDecl *Target;
+  
+  // We ignore the result here since if we don't have a body, Target will be
+  // null below.
+  (void)Ctor->getTargetConstructor()->hasBody(FNTarget);
+  Target
+= const_cast<CXXConstructorDecl*>(cast_or_null<CXXConstructorDecl>(FNTarget));
 
-  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(),
+  CXXConstructorDecl *Canonical = Ctor->getCanonicalDecl(),
+                     // Avoid dereferencing a null pointer here.
+                     *TCanonical = Target ? Target->getCanonicalDecl() : 0;
+
+  if (!Current.insert(Canonical))
+    return;
+
+  // We know that beyond here, we aren't chaining into a cycle.
+  if (!Target || !Target->isDelegatingConstructor() ||
+      Target->isInvalidDecl() || Valid.count(TCanonical)) {
+    for (CI = Current.begin(), CE = Current.end(); CI != CE; ++CI)
+      Valid.insert(*CI);
+    Current.clear();
+  // We've hit a cycle.
+  } else if (TCanonical == Canonical || Invalid.count(TCanonical) ||
+             Current.count(TCanonical)) {
+    // If we haven't diagnosed this cycle yet, do so now.
+    if (!Invalid.count(TCanonical)) {
+      S.Diag((*Ctor->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);
-        }
-      }
+        << Ctor;
+
+      // Don't add a note for a function delegating directo to itself.
+      if (TCanonical != Canonical)
+        S.Diag(Target->getLocation(), diag::note_it_delegates_to);
+
+      CXXConstructorDecl *C = Target;
+      while (C->getCanonicalDecl() != Canonical) {
+        (void)C->getTargetConstructor()->hasBody(FNTarget);
+        assert(FNTarget && "Ctor cycle through bodiless function");
 
-      (*i)->setInvalidDecl();
-      Invalid.insert(*i);
-      for (ci = Current.begin(), ce = Current.end(); ci != ce; ++ci) {
-        (*ci)->setInvalidDecl();
-        Invalid.insert(*i);
+        C
+       = const_cast<CXXConstructorDecl*>(cast<CXXConstructorDecl>(FNTarget));
+        S.Diag(C->getLocation(), diag::note_which_delegates_to);
       }
-      Current.clear();
-    } else {
-      Current.insert(*i);
     }
+
+    for (CI = Current.begin(), CE = Current.end(); CI != CE; ++CI)
+      Invalid.insert(*CI);
+    Current.clear();
+  } else {
+    DelegatingCycleHelper(Target, Valid, Invalid, Current, S);
+  }
+}
+   
+
+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) {
+   DelegatingCycleHelper(*I, Valid, Invalid, Current, *this);
   }
+
+  for (CI = Invalid.begin(), CE = Invalid.end(); CI != CE; ++CI)
+    (*CI)->setInvalidDecl();
 }
index 3227cfc457fa44ebefaa17b10c3ce47a65a4e332..45a771a4724d130d394acf054bc62d25957badeb 100644 (file)
@@ -2114,15 +2114,6 @@ ASTReader::ReadASTBlock(PerFileData &F) {
       TotalVisibleDeclContexts += Record[3];
       break;
 
-    case TENTATIVE_DEFINITIONS:
-      // Optimization for the first block.
-      if (TentativeDefinitions.empty())
-        TentativeDefinitions.swap(Record);
-      else
-        TentativeDefinitions.insert(TentativeDefinitions.end(),
-                                    Record.begin(), Record.end());
-      break;
-
     case UNUSED_FILESCOPED_DECLS:
       // Optimization for the first block.
       if (UnusedFileScopedDecls.empty())
@@ -2132,6 +2123,15 @@ ASTReader::ReadASTBlock(PerFileData &F) {
                                      Record.begin(), Record.end());
       break;
 
+    case DELEGATING_CTORS:
+      // Optimization for the first block.
+      if (DelegatingCtorDecls.empty())
+        DelegatingCtorDecls.swap(Record);
+      else
+        DelegatingCtorDecls.insert(DelegatingCtorDecls.end(),
+                                   Record.begin(), Record.end());
+      break;
+
     case WEAK_UNDECLARED_IDENTIFIERS:
       // Later blocks overwrite earlier ones.
       WeakUndeclaredIdentifiers.swap(Record);
@@ -2331,6 +2331,15 @@ ASTReader::ReadASTBlock(PerFileData &F) {
       // Later tables overwrite earlier ones.
       OpenCLExtensions.swap(Record);
       break;
+
+    case TENTATIVE_DEFINITIONS:
+      // Optimization for the first block.
+      if (TentativeDefinitions.empty())
+        TentativeDefinitions.swap(Record);
+      else
+        TentativeDefinitions.insert(TentativeDefinitions.end(),
+                                    Record.begin(), Record.end());
+      break;
     }
     First = false;
   }
@@ -4100,6 +4109,13 @@ void ASTReader::InitializeSema(Sema &S) {
     SemaObj->UnusedFileScopedDecls.push_back(D);
   }
 
+  // If there were any delegating constructors, add them to Sema's list
+  for (unsigned I = 0, N = DelegatingCtorDecls.size(); I != N; ++I) {
+    CXXConstructorDecl *D
+     = cast<CXXConstructorDecl>(GetDecl(DelegatingCtorDecls[I]));
+    SemaObj->DelegatingCtorDecls.push_back(D);
+  }
+
   // If there were any locally-scoped external declarations,
   // deserialize them and add them to Sema's table of locally-scoped
   // external declarations.
index 30b55c2591d348b1ccf03649c2b8987e6939e2a3..be415015ee70d95019e65ab412583a4098f0f046 100644 (file)
@@ -764,6 +764,7 @@ void ASTWriter::WriteBlockInfoBlock() {
   RECORD(HEADER_SEARCH_TABLE);
   RECORD(FP_PRAGMA_OPTIONS);
   RECORD(OPENCL_EXTENSIONS);
+  RECORD(DELEGATING_CTORS);
   
   // SourceManager Block.
   BLOCK(SOURCE_MANAGER_BLOCK);
@@ -2723,6 +2724,10 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls,
   for (unsigned i=0, e = SemaRef.UnusedFileScopedDecls.size(); i !=e; ++i)
     AddDeclRef(SemaRef.UnusedFileScopedDecls[i], UnusedFileScopedDecls);
 
+  RecordData DelegatingCtorDecls;
+  for (unsigned i=0, e = SemaRef.DelegatingCtorDecls.size(); i != e; ++i)
+    AddDeclRef(SemaRef.DelegatingCtorDecls[i], DelegatingCtorDecls);
+
   RecordData WeakUndeclaredIdentifiers;
   if (!SemaRef.WeakUndeclaredIdentifiers.empty()) {
     WeakUndeclaredIdentifiers.push_back(
@@ -2897,6 +2902,10 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls,
   // Write the record containing CUDA-specific declaration references.
   if (!CUDASpecialDeclRefs.empty())
     Stream.EmitRecord(CUDA_SPECIAL_DECL_REFS, CUDASpecialDeclRefs);
+  
+  // Write the delegating constructors.
+  if (!DelegatingCtorDecls.empty())
+    Stream.EmitRecord(DELEGATING_CTORS, DelegatingCtorDecls);
 
   // Some simple statistics
   Record.clear();
@@ -2971,6 +2980,14 @@ void ASTWriter::WriteASTChain(Sema &SemaRef, MemorizeStatCalls *StatCalls,
       AddDeclRef(SemaRef.UnusedFileScopedDecls[i], UnusedFileScopedDecls);
   }
 
+  // Build a record containing all of the delegating constructor decls in this
+  // file.
+  RecordData DelegatingCtorDecls;
+  for (unsigned i=0, e = SemaRef.DelegatingCtorDecls.size(); i != e; ++i) {
+    if (SemaRef.DelegatingCtorDecls[i]->getPCHLevel() == 0)
+      AddDeclRef(SemaRef.DelegatingCtorDecls[i], DelegatingCtorDecls);
+  }
+
   // We write the entire table, overwriting the tables from the chain.
   RecordData WeakUndeclaredIdentifiers;
   if (!SemaRef.WeakUndeclaredIdentifiers.empty()) {
@@ -3128,6 +3145,10 @@ void ASTWriter::WriteASTChain(Sema &SemaRef, MemorizeStatCalls *StatCalls,
   // Write the record containing declaration references of Sema.
   if (!SemaDeclRefs.empty())
     Stream.EmitRecord(SEMA_DECL_REFS, SemaDeclRefs);
+  
+  // Write the delegating constructors.
+  if (!DelegatingCtorDecls.empty())
+    Stream.EmitRecord(DELEGATING_CTORS, DelegatingCtorDecls);
 
   // Write the updates to DeclContexts.
   for (llvm::SmallPtrSet<const DeclContext *, 16>::iterator
index 132428957f73fd8a8c485a8e08fe876dfdfc646f..15311f8529446750c74529f4523dc9fe5cbf25e8 100644 (file)
@@ -1,12 +1,20 @@
 // Test this without pch.
-// RUN: %clang_cc1 -include %S/cxx0x-delegating-ctors.h -std=c++0x -fsyntax-only -verify %s
+// RUN: %clang_cc1 -include %s -std=c++0x -fsyntax-only -verify %s
 
 // Test with pch.
-// RUN: %clang_cc1 -x c++-header -std=c++0x -emit-pch -o %t %S/cxx0x-delegating-ctors.h
+// RUN: %clang_cc1 -x c++-header -std=c++0x -emit-pch -o %t %s
 // RUN: %clang_cc1 -std=c++0x -include-pch %t -fsyntax-only -verify %s 
 
-// Currently we can't deal with a note in the header
-// XFAIL: *
-
+#ifndef PASS1
+#define PASS1
+struct foo {
+  foo(int) : foo() { } // expected-note{{it delegates to}}
+  foo();
+  foo(bool) : foo('c') { } // expected-note{{it delegates to}}
+  foo(char) : foo(true) { } // expected-error{{creates a delegation cycle}} \
+                            // expected-note{{which delegates to}}
+};
+#else
 foo::foo() : foo(1) { } // expected-error{{creates a delegation cycle}} \
                         // expected-note{{which delegates to}}
+#endif
diff --git a/test/PCH/cxx0x-delegating-ctors.h b/test/PCH/cxx0x-delegating-ctors.h
deleted file mode 100644 (file)
index f4cc636..0000000
+++ /dev/null
@@ -1,6 +0,0 @@
-// Header for PCH test cxx0x-delegating-ctors.cpp
-
-struct foo {
-  foo(int) : foo() { } // expected-note{{it delegates to}}
-  foo();
-};