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
diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h
index 0cb2476..88f473e 100644
--- a/include/clang/AST/DeclCXX.h
+++ b/include/clang/AST/DeclCXX.h
@@ -1619,10 +1619,9 @@
/// getTargetConstructor - When this constructor delegates to
/// another, retrieve the target
CXXConstructorDecl *getTargetConstructor() const {
- if (isDelegatingConstructor())
- return CtorInitializers[0]->getTargetConstructor();
- else
- return 0;
+ assert(isDelegatingConstructor() &&
+ "A non-delegating constructor has no target");
+ return CtorInitializers[0]->getTargetConstructor();
}
/// isDefaultConstructor - Whether this constructor is a default
diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 7c4583f..771a681 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1013,8 +1013,12 @@
"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">;
diff --git a/include/clang/Basic/LangOptions.h b/include/clang/Basic/LangOptions.h
index a5f6789..92af3c1 100644
--- a/include/clang/Basic/LangOptions.h
+++ b/include/clang/Basic/LangOptions.h
@@ -134,6 +134,9 @@
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 @@
MRTD = 0;
DelayedTemplateParsing = 0;
ParseUnknownAnytype = 0;
+
+ CheckDelegatingCtorCycles = 1;
}
GCMode getGCMode() const { return (GCMode) GC; }
diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td
index dd09846..b4c0c7f 100644
--- a/include/clang/Driver/CC1Options.td
+++ b/include/clang/Driver/CC1Options.td
@@ -547,6 +547,8 @@
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
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index d70a67f..faa0ac4 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -312,6 +312,10 @@
/// 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 @@
void ActOnEndOfTranslationUnit();
+ void CheckDelegatingCtorCycles();
+
Scope *getScopeForContext(DeclContext *Ctx);
void PushFunctionScope();
diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp
index b84e4c8..eadf1b3 100644
--- a/lib/Frontend/CompilerInvocation.cpp
+++ b/lib/Frontend/CompilerInvocation.cpp
@@ -698,6 +698,9 @@
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 @@
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,
diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp
index 7707fb1..7153939 100644
--- a/lib/Sema/Sema.cpp
+++ b/lib/Sema/Sema.cpp
@@ -473,6 +473,9 @@
}
+ if (LangOpts.CPlusPlus0x && LangOpts.CheckDelegatingCtorCycles)
+ CheckDelegatingCtorCycles();
+
// If there were errors, disable 'unused' warnings since they will mostly be
// noise.
if (!Diags.hasErrorOccurred()) {
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index 641bcd8..7b16d63 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -1547,7 +1547,8 @@
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 @@
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,9 +2085,11 @@
DiagnoseUseOfDecl(Dtor, Initializer->getSourceLocation());
}
+ if (LangOpts.CheckDelegatingCtorCycles)
+ DelegatingCtorDecls.push_back(Constructor);
+
return false;
}
-
bool
Sema::SetCtorInitializers(CXXConstructorDecl *Constructor,
@@ -2479,7 +2466,7 @@
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 @@
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);
+ }
+ }
+}
diff --git a/test/PCH/cxx0x-delegating-ctors.cpp b/test/PCH/cxx0x-delegating-ctors.cpp
index 97f2f68..1324289 100644
--- a/test/PCH/cxx0x-delegating-ctors.cpp
+++ b/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}}
diff --git a/test/PCH/cxx0x-delegating-ctors.h b/test/PCH/cxx0x-delegating-ctors.h
index 598982f..f4cc636 100644
--- a/test/PCH/cxx0x-delegating-ctors.h
+++ b/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();
};
diff --git a/test/SemaCXX/cxx0x-delegating-ctors.cpp b/test/SemaCXX/cxx0x-delegating-ctors.cpp
index df88b6b..a3e6ff3 100644
--- a/test/SemaCXX/cxx0x-delegating-ctors.cpp
+++ b/test/SemaCXX/cxx0x-delegating-ctors.cpp
@@ -22,14 +22,15 @@
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}}