diff --git a/include/clang/AST/DeclCXX.h b/include/clang/AST/DeclCXX.h index 0cb24767e6..88f473eb48 100644 --- a/include/clang/AST/DeclCXX.h +++ b/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()) - 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 7c4583f7dc..771a68177a 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/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">; diff --git a/include/clang/Basic/LangOptions.h b/include/clang/Basic/LangOptions.h index a5f6789b7d..92af3c19f5 100644 --- a/include/clang/Basic/LangOptions.h +++ b/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; } diff --git a/include/clang/Driver/CC1Options.td b/include/clang/Driver/CC1Options.td index dd0984634d..b4c0c7f7c0 100644 --- a/include/clang/Driver/CC1Options.td +++ b/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 diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h index d70a67f9fe..faa0ac487c 100644 --- a/include/clang/Sema/Sema.h +++ b/include/clang/Sema/Sema.h @@ -312,6 +312,10 @@ public: /// and must warn if not used. Only contains the first declaration. llvm::SmallVector UnusedFileScopedDecls; + /// \brief All the delegating constructors seen so far in the file, used for + /// cycle detection at the end of the TU. + llvm::SmallVector 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(); diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index b84e4c82b3..eadf1b39dc 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/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, diff --git a/lib/Sema/Sema.cpp b/lib/Sema/Sema.cpp index 7707fb1104..7153939151 100644 --- a/lib/Sema/Sema.cpp +++ b/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()) { diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp index 641bcd831b..7b16d63a48 100644 --- a/lib/Sema/SemaDeclCXX.cpp +++ b/lib/Sema/SemaDeclCXX.cpp @@ -1547,7 +1547,8 @@ Sema::BuildDelegatingInitializer(TypeSourceInfo *TInfo, return true; CXXConstructExpr *ConExpr = cast(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,9 +2085,11 @@ Sema::SetDelegatingInitializer(CXXConstructorDecl *Constructor, DiagnoseUseOfDecl(Dtor, Initializer->getSourceLocation()); } + if (LangOpts.CheckDelegatingCtorCycles) + DelegatingCtorDecls.push_back(Constructor); + return false; } - bool Sema::SetCtorInitializers(CXXConstructorDecl *Constructor, @@ -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 Valid, Invalid, Current; + + 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); + } + } +} diff --git a/test/PCH/cxx0x-delegating-ctors.cpp b/test/PCH/cxx0x-delegating-ctors.cpp index 97f2f684fc..132428957f 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 598982fdd2..f4cc636791 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 df88b6bc31..a3e6ff3b4f 100644 --- a/test/SemaCXX/cxx0x-delegating-ctors.cpp +++ b/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}}