Skip to content

Commit 27a761d

Browse files
alercahalercah
committed
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
1 parent 2159b8d commit 27a761d

File tree

7 files changed

+142
-62
lines changed

7 files changed

+142
-62
lines changed

‎clang/include/clang/Serialization/ASTBitCodes.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -362,7 +362,10 @@ namespace clang {
362362
FP_PRAGMA_OPTIONS = 42,
363363

364364
/// \brief Record code for enabled OpenCL extensions.
365-
OPENCL_EXTENSIONS = 43
365+
OPENCL_EXTENSIONS = 43,
366+
367+
/// \brief The list of delegating constructor declarations.
368+
DELEGATING_CTORS = 44
366369
};
367370

368371
/// \brief Record types used within a source manager block.

‎clang/include/clang/Serialization/ASTReader.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ class Decl;
5454
class DeclContext;
5555
class NestedNameSpecifier;
5656
class CXXBaseSpecifier;
57+
class CXXConstructorDecl;
5758
class CXXCtorInitializer;
5859
class GotoStmt;
5960
class MacroDefinition;
@@ -564,6 +565,10 @@ class ASTReader
564565
/// generating warnings.
565566
llvm::SmallVector<uint64_t, 16> UnusedFileScopedDecls;
566567

568+
/// \brief A list of all the delegating constructors we've seen, to diagnose
569+
/// cycles.
570+
llvm::SmallVector<uint64_t, 4> DelegatingCtorDecls;
571+
567572
/// \brief A snapshot of Sema's weak undeclared identifier tracking, for
568573
/// generating warnings.
569574
llvm::SmallVector<uint64_t, 64> WeakUndeclaredIdentifiers;

‎clang/lib/Sema/SemaDeclCXX.cpp

Lines changed: 74 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -7945,52 +7945,85 @@ void Sema::SetIvarInitializers(ObjCImplementationDecl *ObjCImplementation) {
79457945
}
79467946
}
79477947

7948-
void Sema::CheckDelegatingCtorCycles() {
7949-
llvm::SmallSet<CXXConstructorDecl*, 4> Valid, Invalid, Current;
7948+
static
7949+
void DelegatingCycleHelper(CXXConstructorDecl* Ctor,
7950+
llvm::SmallSet<CXXConstructorDecl*, 4> &Valid,
7951+
llvm::SmallSet<CXXConstructorDecl*, 4> &Invalid,
7952+
llvm::SmallSet<CXXConstructorDecl*, 4> &Current,
7953+
Sema &S) {
7954+
llvm::SmallSet<CXXConstructorDecl*, 4>::iterator CI = Current.begin(),
7955+
CE = Current.end();
7956+
if (Ctor->isInvalidDecl())
7957+
return;
79507958

7951-
llvm::SmallSet<CXXConstructorDecl*, 4>::iterator ci = Current.begin(),
7952-
ce = Current.end();
7959+
const FunctionDecl *FNTarget = 0;
7960+
CXXConstructorDecl *Target;
7961+
7962+
// We ignore the result here since if we don't have a body, Target will be
7963+
// null below.
7964+
(void)Ctor->getTargetConstructor()->hasBody(FNTarget);
7965+
Target
7966+
= const_cast<CXXConstructorDecl*>(cast_or_null<CXXConstructorDecl>(FNTarget));
79537967

7954-
for (llvm::SmallVector<CXXConstructorDecl*, 4>::iterator
7955-
i = DelegatingCtorDecls.begin(),
7956-
e = DelegatingCtorDecls.end();
7957-
i != e; ++i) {
7958-
const FunctionDecl *FNTarget;
7959-
CXXConstructorDecl *Target;
7960-
(*i)->getTargetConstructor()->hasBody(FNTarget);
7961-
Target
7962-
= const_cast<CXXConstructorDecl*>(cast<CXXConstructorDecl>(FNTarget));
7963-
7964-
if (!Target || !Target->isDelegatingConstructor() || Valid.count(Target)) {
7965-
Valid.insert(*i);
7966-
for (ci = Current.begin(), ce = Current.end(); ci != ce; ++ci)
7967-
Valid.insert(*ci);
7968-
Current.clear();
7969-
} else if (Target == *i || Invalid.count(Target) || Current.count(Target)) {
7970-
if (!Invalid.count(Target)) {
7971-
Diag((*(*i)->init_begin())->getSourceLocation(),
7968+
CXXConstructorDecl *Canonical = Ctor->getCanonicalDecl(),
7969+
// Avoid dereferencing a null pointer here.
7970+
*TCanonical = Target ? Target->getCanonicalDecl() : 0;
7971+
7972+
if (!Current.insert(Canonical))
7973+
return;
7974+
7975+
// We know that beyond here, we aren't chaining into a cycle.
7976+
if (!Target || !Target->isDelegatingConstructor() ||
7977+
Target->isInvalidDecl() || Valid.count(TCanonical)) {
7978+
for (CI = Current.begin(), CE = Current.end(); CI != CE; ++CI)
7979+
Valid.insert(*CI);
7980+
Current.clear();
7981+
// We've hit a cycle.
7982+
} else if (TCanonical == Canonical || Invalid.count(TCanonical) ||
7983+
Current.count(TCanonical)) {
7984+
// If we haven't diagnosed this cycle yet, do so now.
7985+
if (!Invalid.count(TCanonical)) {
7986+
S.Diag((*Ctor->init_begin())->getSourceLocation(),
79727987
diag::err_delegating_ctor_cycle)
7973-
<< *i;
7974-
if (Target != *i)
7975-
Diag(Target->getLocation(), diag::note_it_delegates_to);
7976-
CXXConstructorDecl *Current = Target;
7977-
while (Current != *i) {
7978-
Current->getTargetConstructor()->hasBody(FNTarget);
7979-
Current
7980-
= const_cast<CXXConstructorDecl*>(cast<CXXConstructorDecl>(FNTarget));
7981-
Diag(Current->getLocation(), diag::note_which_delegates_to);
7982-
}
7983-
}
7988+
<< Ctor;
7989+
7990+
// Don't add a note for a function delegating directo to itself.
7991+
if (TCanonical != Canonical)
7992+
S.Diag(Target->getLocation(), diag::note_it_delegates_to);
7993+
7994+
CXXConstructorDecl *C = Target;
7995+
while (C->getCanonicalDecl() != Canonical) {
7996+
(void)C->getTargetConstructor()->hasBody(FNTarget);
7997+
assert(FNTarget && "Ctor cycle through bodiless function");
79847998

7985-
(*i)->setInvalidDecl();
7986-
Invalid.insert(*i);
7987-
for (ci = Current.begin(), ce = Current.end(); ci != ce; ++ci) {
7988-
(*ci)->setInvalidDecl();
7989-
Invalid.insert(*i);
7999+
C
8000+
= const_cast<CXXConstructorDecl*>(cast<CXXConstructorDecl>(FNTarget));
8001+
S.Diag(C->getLocation(), diag::note_which_delegates_to);
79908002
}
7991-
Current.clear();
7992-
} else {
7993-
Current.insert(*i);
79948003
}
8004+
8005+
for (CI = Current.begin(), CE = Current.end(); CI != CE; ++CI)
8006+
Invalid.insert(*CI);
8007+
Current.clear();
8008+
} else {
8009+
DelegatingCycleHelper(Target, Valid, Invalid, Current, S);
8010+
}
8011+
}
8012+
8013+
8014+
void Sema::CheckDelegatingCtorCycles() {
8015+
llvm::SmallSet<CXXConstructorDecl*, 4> Valid, Invalid, Current;
8016+
8017+
llvm::SmallSet<CXXConstructorDecl*, 4>::iterator CI = Current.begin(),
8018+
CE = Current.end();
8019+
8020+
for (llvm::SmallVector<CXXConstructorDecl*, 4>::iterator
8021+
I = DelegatingCtorDecls.begin(),
8022+
E = DelegatingCtorDecls.end();
8023+
I != E; ++I) {
8024+
DelegatingCycleHelper(*I, Valid, Invalid, Current, *this);
79958025
}
8026+
8027+
for (CI = Invalid.begin(), CE = Invalid.end(); CI != CE; ++CI)
8028+
(*CI)->setInvalidDecl();
79968029
}

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2114,15 +2114,6 @@ ASTReader::ReadASTBlock(PerFileData &F) {
21142114
TotalVisibleDeclContexts += Record[3];
21152115
break;
21162116

2117-
case TENTATIVE_DEFINITIONS:
2118-
// Optimization for the first block.
2119-
if (TentativeDefinitions.empty())
2120-
TentativeDefinitions.swap(Record);
2121-
else
2122-
TentativeDefinitions.insert(TentativeDefinitions.end(),
2123-
Record.begin(), Record.end());
2124-
break;
2125-
21262117
case UNUSED_FILESCOPED_DECLS:
21272118
// Optimization for the first block.
21282119
if (UnusedFileScopedDecls.empty())
@@ -2132,6 +2123,15 @@ ASTReader::ReadASTBlock(PerFileData &F) {
21322123
Record.begin(), Record.end());
21332124
break;
21342125

2126+
case DELEGATING_CTORS:
2127+
// Optimization for the first block.
2128+
if (DelegatingCtorDecls.empty())
2129+
DelegatingCtorDecls.swap(Record);
2130+
else
2131+
DelegatingCtorDecls.insert(DelegatingCtorDecls.end(),
2132+
Record.begin(), Record.end());
2133+
break;
2134+
21352135
case WEAK_UNDECLARED_IDENTIFIERS:
21362136
// Later blocks overwrite earlier ones.
21372137
WeakUndeclaredIdentifiers.swap(Record);
@@ -2331,6 +2331,15 @@ ASTReader::ReadASTBlock(PerFileData &F) {
23312331
// Later tables overwrite earlier ones.
23322332
OpenCLExtensions.swap(Record);
23332333
break;
2334+
2335+
case TENTATIVE_DEFINITIONS:
2336+
// Optimization for the first block.
2337+
if (TentativeDefinitions.empty())
2338+
TentativeDefinitions.swap(Record);
2339+
else
2340+
TentativeDefinitions.insert(TentativeDefinitions.end(),
2341+
Record.begin(), Record.end());
2342+
break;
23342343
}
23352344
First = false;
23362345
}
@@ -4100,6 +4109,13 @@ void ASTReader::InitializeSema(Sema &S) {
41004109
SemaObj->UnusedFileScopedDecls.push_back(D);
41014110
}
41024111

4112+
// If there were any delegating constructors, add them to Sema's list
4113+
for (unsigned I = 0, N = DelegatingCtorDecls.size(); I != N; ++I) {
4114+
CXXConstructorDecl *D
4115+
= cast<CXXConstructorDecl>(GetDecl(DelegatingCtorDecls[I]));
4116+
SemaObj->DelegatingCtorDecls.push_back(D);
4117+
}
4118+
41034119
// If there were any locally-scoped external declarations,
41044120
// deserialize them and add them to Sema's table of locally-scoped
41054121
// external declarations.

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -764,6 +764,7 @@ void ASTWriter::WriteBlockInfoBlock() {
764764
RECORD(HEADER_SEARCH_TABLE);
765765
RECORD(FP_PRAGMA_OPTIONS);
766766
RECORD(OPENCL_EXTENSIONS);
767+
RECORD(DELEGATING_CTORS);
767768

768769
// SourceManager Block.
769770
BLOCK(SOURCE_MANAGER_BLOCK);
@@ -2723,6 +2724,10 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls,
27232724
for (unsigned i=0, e = SemaRef.UnusedFileScopedDecls.size(); i !=e; ++i)
27242725
AddDeclRef(SemaRef.UnusedFileScopedDecls[i], UnusedFileScopedDecls);
27252726

2727+
RecordData DelegatingCtorDecls;
2728+
for (unsigned i=0, e = SemaRef.DelegatingCtorDecls.size(); i != e; ++i)
2729+
AddDeclRef(SemaRef.DelegatingCtorDecls[i], DelegatingCtorDecls);
2730+
27262731
RecordData WeakUndeclaredIdentifiers;
27272732
if (!SemaRef.WeakUndeclaredIdentifiers.empty()) {
27282733
WeakUndeclaredIdentifiers.push_back(
@@ -2897,6 +2902,10 @@ void ASTWriter::WriteASTCore(Sema &SemaRef, MemorizeStatCalls *StatCalls,
28972902
// Write the record containing CUDA-specific declaration references.
28982903
if (!CUDASpecialDeclRefs.empty())
28992904
Stream.EmitRecord(CUDA_SPECIAL_DECL_REFS, CUDASpecialDeclRefs);
2905+
2906+
// Write the delegating constructors.
2907+
if (!DelegatingCtorDecls.empty())
2908+
Stream.EmitRecord(DELEGATING_CTORS, DelegatingCtorDecls);
29002909

29012910
// Some simple statistics
29022911
Record.clear();
@@ -2971,6 +2980,14 @@ void ASTWriter::WriteASTChain(Sema &SemaRef, MemorizeStatCalls *StatCalls,
29712980
AddDeclRef(SemaRef.UnusedFileScopedDecls[i], UnusedFileScopedDecls);
29722981
}
29732982

2983+
// Build a record containing all of the delegating constructor decls in this
2984+
// file.
2985+
RecordData DelegatingCtorDecls;
2986+
for (unsigned i=0, e = SemaRef.DelegatingCtorDecls.size(); i != e; ++i) {
2987+
if (SemaRef.DelegatingCtorDecls[i]->getPCHLevel() == 0)
2988+
AddDeclRef(SemaRef.DelegatingCtorDecls[i], DelegatingCtorDecls);
2989+
}
2990+
29742991
// We write the entire table, overwriting the tables from the chain.
29752992
RecordData WeakUndeclaredIdentifiers;
29762993
if (!SemaRef.WeakUndeclaredIdentifiers.empty()) {
@@ -3128,6 +3145,10 @@ void ASTWriter::WriteASTChain(Sema &SemaRef, MemorizeStatCalls *StatCalls,
31283145
// Write the record containing declaration references of Sema.
31293146
if (!SemaDeclRefs.empty())
31303147
Stream.EmitRecord(SEMA_DECL_REFS, SemaDeclRefs);
3148+
3149+
// Write the delegating constructors.
3150+
if (!DelegatingCtorDecls.empty())
3151+
Stream.EmitRecord(DELEGATING_CTORS, DelegatingCtorDecls);
31313152

31323153
// Write the updates to DeclContexts.
31333154
for (llvm::SmallPtrSet<const DeclContext *, 16>::iterator

‎clang/test/PCH/cxx0x-delegating-ctors.cpp

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
11
// Test this without pch.
2-
// RUN: %clang_cc1 -include %S/cxx0x-delegating-ctors.h -std=c++0x -fsyntax-only -verify %s
2+
// RUN: %clang_cc1 -include %s -std=c++0x -fsyntax-only -verify %s
33

44
// Test with pch.
5-
// RUN: %clang_cc1 -x c++-header -std=c++0x -emit-pch -o %t %S/cxx0x-delegating-ctors.h
5+
// RUN: %clang_cc1 -x c++-header -std=c++0x -emit-pch -o %t %s
66
// RUN: %clang_cc1 -std=c++0x -include-pch %t -fsyntax-only -verify %s
77

8-
// Currently we can't deal with a note in the header
9-
// XFAIL: *
10-
8+
#ifndef PASS1
9+
#define PASS1
10+
struct foo {
11+
foo(int) : foo() { } // expected-note{{it delegates to}}
12+
foo();
13+
foo(bool) : foo('c') { } // expected-note{{it delegates to}}
14+
foo(char) : foo(true) { } // expected-error{{creates a delegation cycle}} \
15+
// expected-note{{which delegates to}}
16+
};
17+
#else
1118
foo::foo() : foo(1) { } // expected-error{{creates a delegation cycle}} \
1219
// expected-note{{which delegates to}}
20+
#endif

‎clang/test/PCH/cxx0x-delegating-ctors.h

Lines changed: 0 additions & 6 deletions
This file was deleted.

0 commit comments

Comments
 (0)