From 27a761d5bde0255ae6fbe71c5239544db81e2d63 Mon Sep 17 00:00:00 2001 From: Alexis Hunt Date: Wed, 4 May 2011 23:29:54 +0000 Subject: [PATCH] there i fixed it 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. llvm-svn: 130887 --- .../include/clang/Serialization/ASTBitCodes.h | 5 +- clang/include/clang/Serialization/ASTReader.h | 5 + clang/lib/Sema/SemaDeclCXX.cpp | 117 +++++++++++------- clang/lib/Serialization/ASTReader.cpp | 34 +++-- clang/lib/Serialization/ASTWriter.cpp | 21 ++++ clang/test/PCH/cxx0x-delegating-ctors.cpp | 18 ++- clang/test/PCH/cxx0x-delegating-ctors.h | 6 - 7 files changed, 143 insertions(+), 63 deletions(-) delete mode 100644 clang/test/PCH/cxx0x-delegating-ctors.h diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 0905b43f878f..7b9e98d34370 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -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. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index da018ab99e65..fd073e29aa95 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -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 UnusedFileScopedDecls; + /// \brief A list of all the delegating constructors we've seen, to diagnose + /// cycles. + llvm::SmallVector DelegatingCtorDecls; + /// \brief A snapshot of Sema's weak undeclared identifier tracking, for /// generating warnings. llvm::SmallVector WeakUndeclaredIdentifiers; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 7b16d63a4814..bb732e2c9fea 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -7945,52 +7945,85 @@ void Sema::SetIvarInitializers(ObjCImplementationDecl *ObjCImplementation) { } } +static +void DelegatingCycleHelper(CXXConstructorDecl* Ctor, + llvm::SmallSet &Valid, + llvm::SmallSet &Invalid, + llvm::SmallSet &Current, + Sema &S) { + llvm::SmallSet::iterator CI = Current.begin(), + CE = Current.end(); + if (Ctor->isInvalidDecl()) + return; + + 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(cast_or_null(FNTarget)); + + 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) + << 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"); + + C + = const_cast(cast(FNTarget)); + S.Diag(C->getLocation(), diag::note_which_delegates_to); + } + } + + 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 Valid, Invalid, Current; - llvm::SmallSet::iterator ci = Current.begin(), - ce = Current.end(); + llvm::SmallSet::iterator CI = Current.begin(), + CE = Current.end(); for (llvm::SmallVector::iterator - i = DelegatingCtorDecls.begin(), - e = DelegatingCtorDecls.end(); - i != e; ++i) { - const FunctionDecl *FNTarget; - CXXConstructorDecl *Target; - (*i)->getTargetConstructor()->hasBody(FNTarget); - Target - = const_cast(cast(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(cast(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); - } + 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(); } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 3227cfc457fa..45a771a4724d 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -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(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. diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 30b55c2591d3..be415015ee70 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -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::iterator diff --git a/clang/test/PCH/cxx0x-delegating-ctors.cpp b/clang/test/PCH/cxx0x-delegating-ctors.cpp index 132428957f73..15311f852944 100644 --- a/clang/test/PCH/cxx0x-delegating-ctors.cpp +++ b/clang/test/PCH/cxx0x-delegating-ctors.cpp @@ -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/clang/test/PCH/cxx0x-delegating-ctors.h b/clang/test/PCH/cxx0x-delegating-ctors.h deleted file mode 100644 index f4cc63679189..000000000000 --- a/clang/test/PCH/cxx0x-delegating-ctors.h +++ /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(); -};