diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h --- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h +++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h @@ -40,17 +40,17 @@ static char ID; private: - SmallVector Sources; // doesn't own them. + SmallVector Sources; public: ///Constructs a new multiplexing external sema source and appends the /// given element to it. /// - ///\param[in] s1 - A non-null (old) ExternalSemaSource. - ///\param[in] s2 - A non-null (new) ExternalSemaSource. + ///\param[in] S1 - A non-null (old) ExternalSemaSource. + ///\param[in] S2 - A non-null (new) ExternalSemaSource. /// - MultiplexExternalSemaSource(ExternalSemaSource& s1, ExternalSemaSource& s2); + MultiplexExternalSemaSource(ExternalSemaSource* S1, ExternalSemaSource* S2); ~MultiplexExternalSemaSource() override; @@ -58,7 +58,7 @@ /// ///\param[in] source - An ExternalSemaSource. /// - void addSource(ExternalSemaSource &source); + void AddSource(ExternalSemaSource *Source); //===--------------------------------------------------------------------===// // ExternalASTSource. diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -360,10 +360,7 @@ void operator=(const Sema &) = delete; ///Source of additional semantic information. - ExternalSemaSource *ExternalSource; - - ///Whether Sema has generated a multiplexer and has to delete it. - bool isMultiplexExternalSource; + IntrusiveRefCntPtr ExternalSource; static bool mightHaveNonExternalLinkage(const DeclaratorDecl *FD); @@ -1637,7 +1634,7 @@ ASTContext &getASTContext() const { return Context; } ASTConsumer &getASTConsumer() const { return Consumer; } ASTMutationListener *getASTMutationListener() const; - ExternalSemaSource* getExternalSource() const { return ExternalSource; } + ExternalSemaSource* getExternalSource() const { return ExternalSource.get(); } DarwinSDKInfo *getDarwinSDKInfoForAvailabilityChecking(SourceLocation Loc, StringRef Platform); diff --git a/clang/lib/Frontend/ChainedIncludesSource.cpp b/clang/lib/Frontend/ChainedIncludesSource.cpp --- a/clang/lib/Frontend/ChainedIncludesSource.cpp +++ b/clang/lib/Frontend/ChainedIncludesSource.cpp @@ -27,15 +27,15 @@ using namespace clang; namespace { -class ChainedIncludesSourceImpl : public ExternalSemaSource { +class ChainedIncludesSource : public ExternalSemaSource { public: - ChainedIncludesSourceImpl(std::vector> CIs) + ChainedIncludesSource(std::vector> CIs) : CIs(std::move(CIs)) {} protected: - //===----------------------------------------------------------------------===// + //===--------------------------------------------------------------------===// // ExternalASTSource interface. - //===----------------------------------------------------------------------===// + //===--------------------------------------------------------------------===// /// Return the amount of memory used by memory buffers, breaking down /// by heap-backed versus mmap'ed memory. @@ -51,30 +51,7 @@ private: std::vector> CIs; }; - -/// Members of ChainedIncludesSource, factored out so we can initialize -/// them before we initialize the ExternalSemaSource base class. -struct ChainedIncludesSourceMembers { - ChainedIncludesSourceMembers( - std::vector> CIs, - IntrusiveRefCntPtr FinalReader) - : Impl(std::move(CIs)), FinalReader(std::move(FinalReader)) {} - ChainedIncludesSourceImpl Impl; - IntrusiveRefCntPtr FinalReader; -}; - -/// Use MultiplexExternalSemaSource to dispatch all ExternalSemaSource -/// calls to the final reader. -class ChainedIncludesSource - : private ChainedIncludesSourceMembers, - public MultiplexExternalSemaSource { -public: - ChainedIncludesSource(std::vector> CIs, - IntrusiveRefCntPtr FinalReader) - : ChainedIncludesSourceMembers(std::move(CIs), std::move(FinalReader)), - MultiplexExternalSemaSource(Impl, *this->FinalReader) {} -}; -} +} // end anonymous namespace static ASTReader * createASTReader(CompilerInstance &CI, StringRef pchFile, @@ -214,6 +191,8 @@ if (!Reader) return nullptr; - return IntrusiveRefCntPtr( - new ChainedIncludesSource(std::move(CIs), Reader)); + auto ChainedSrc = + llvm::makeIntrusiveRefCnt(std::move(CIs)); + return llvm::makeIntrusiveRefCnt( + ChainedSrc.get(), Reader.get()); } diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp --- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp +++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp @@ -19,21 +19,27 @@ ///Constructs a new multiplexing external sema source and appends the /// given element to it. /// -MultiplexExternalSemaSource::MultiplexExternalSemaSource(ExternalSemaSource &s1, - ExternalSemaSource &s2){ - Sources.push_back(&s1); - Sources.push_back(&s2); +MultiplexExternalSemaSource::MultiplexExternalSemaSource(ExternalSemaSource *S1, + ExternalSemaSource *S2){ + S1->Retain(); + S2->Retain(); + Sources.push_back(S1); + Sources.push_back(S2); } // pin the vtable here. -MultiplexExternalSemaSource::~MultiplexExternalSemaSource() {} +MultiplexExternalSemaSource::~MultiplexExternalSemaSource() { + for (auto &S : Sources) + S->Release(); +} ///Appends new source to the source list. /// ///\param[in] source - An ExternalSemaSource. /// -void MultiplexExternalSemaSource::addSource(ExternalSemaSource &source) { - Sources.push_back(&source); +void MultiplexExternalSemaSource::AddSource(ExternalSemaSource *Source) { + Source->Retain(); + Sources.push_back(Source); } //===----------------------------------------------------------------------===// diff --git a/clang/lib/Sema/Sema.cpp b/clang/lib/Sema/Sema.cpp --- a/clang/lib/Sema/Sema.cpp +++ b/clang/lib/Sema/Sema.cpp @@ -185,11 +185,10 @@ Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, TranslationUnitKind TUKind, CodeCompleteConsumer *CodeCompleter) - : ExternalSource(nullptr), isMultiplexExternalSource(false), - CurFPFeatures(pp.getLangOpts()), LangOpts(pp.getLangOpts()), PP(pp), - Context(ctxt), Consumer(consumer), Diags(PP.getDiagnostics()), - SourceMgr(PP.getSourceManager()), CollectStats(false), - CodeCompleter(CodeCompleter), CurContext(nullptr), + : ExternalSource(nullptr), CurFPFeatures(pp.getLangOpts()), + LangOpts(pp.getLangOpts()), PP(pp), Context(ctxt), Consumer(consumer), + Diags(PP.getDiagnostics()), SourceMgr(PP.getSourceManager()), + CollectStats(false), CodeCompleter(CodeCompleter), CurContext(nullptr), OriginalLexicalContext(nullptr), MSStructPragmaOn(false), MSPointerToMemberRepresentationMethod( LangOpts.getMSPointerToMemberRepresentationMethod()), @@ -473,10 +472,6 @@ = dyn_cast_or_null(Context.getExternalSource())) ExternalSema->ForgetSema(); - // If Sema's ExternalSource is the multiplexer - we own it. - if (isMultiplexExternalSource) - delete ExternalSource; - // Delete cached satisfactions. std::vector Satisfactions; Satisfactions.reserve(Satisfactions.size()); @@ -550,12 +545,10 @@ return; } - if (isMultiplexExternalSource) - static_cast(ExternalSource)->addSource(*E); - else { - ExternalSource = new MultiplexExternalSemaSource(*ExternalSource, *E); - isMultiplexExternalSource = true; - } + if (auto Ex = dyn_cast(ExternalSource)) + Ex->AddSource(E); + else + ExternalSource = new MultiplexExternalSemaSource(ExternalSource.get(), E); } /// Print out statistics about the semantic analysis. @@ -1291,7 +1284,7 @@ // translation unit, with an initializer equal to 0. llvm::SmallSet Seen; for (TentativeDefinitionsType::iterator - T = TentativeDefinitions.begin(ExternalSource), + T = TentativeDefinitions.begin(ExternalSource.get()), TEnd = TentativeDefinitions.end(); T != TEnd; ++T) { VarDecl *VD = (*T)->getActingDefinition(); @@ -1336,7 +1329,7 @@ if (!Diags.hasErrorOccurred() && TUKind != TU_Module) { // Output warning for unused file scoped decls. for (UnusedFileScopedDeclsType::iterator - I = UnusedFileScopedDecls.begin(ExternalSource), + I = UnusedFileScopedDecls.begin(ExternalSource.get()), E = UnusedFileScopedDecls.end(); I != E; ++I) { if (ShouldRemoveFromUnused(this, *I)) continue; diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -18183,7 +18183,7 @@ llvm::SmallPtrSet Valid, Invalid, Current; for (DelegatingCtorDeclsType::iterator - I = DelegatingCtorDecls.begin(ExternalSource), + I = DelegatingCtorDecls.begin(ExternalSource.get()), E = DelegatingCtorDecls.end(); I != E; ++I) DelegatingCycleHelper(*I, Valid, Invalid, Current, *this); diff --git a/clang/unittests/Sema/ExternalSemaSourceTest.cpp b/clang/unittests/Sema/ExternalSemaSourceTest.cpp --- a/clang/unittests/Sema/ExternalSemaSourceTest.cpp +++ b/clang/unittests/Sema/ExternalSemaSourceTest.cpp @@ -219,6 +219,8 @@ void PushWatcher(DiagnosticWatcher *Watcher) { Watchers.push_back(Watcher); } }; +using llvm::makeIntrusiveRefCnt; + // Make sure that the DiagnosticWatcher is not miscounting. TEST(ExternalSemaSource, DiagCheck) { auto Installer = std::make_unique(); @@ -234,14 +236,14 @@ // instead of the usual suggestion we would use above. TEST(ExternalSemaSource, ExternalTypoCorrectionPrioritized) { auto Installer = std::make_unique(); - NamespaceTypoProvider Provider("AAB", "BBB"); + auto Provider = makeIntrusiveRefCnt("AAB", "BBB"); DiagnosticWatcher Watcher("AAB", "BBB"); - Installer->PushSource(&Provider); + Installer->PushSource(Provider.get()); Installer->PushWatcher(&Watcher); std::vector Args(1, "-std=c++11"); ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs( std::move(Installer), "namespace AAA { } using namespace AAB;", Args)); - ASSERT_LE(0, Provider.CallCount); + ASSERT_LE(0, Provider->CallCount); ASSERT_EQ(1, Watcher.SeenCount); } @@ -249,34 +251,34 @@ // ExternalSemaSource. TEST(ExternalSemaSource, ExternalTypoCorrectionOrdering) { auto Installer = std::make_unique(); - NamespaceTypoProvider First("XXX", "BBB"); - NamespaceTypoProvider Second("AAB", "CCC"); - NamespaceTypoProvider Third("AAB", "DDD"); + auto First = makeIntrusiveRefCnt("XXX", "BBB"); + auto Second = makeIntrusiveRefCnt("AAB", "CCC"); + auto Third = makeIntrusiveRefCnt("AAB", "DDD"); DiagnosticWatcher Watcher("AAB", "CCC"); - Installer->PushSource(&First); - Installer->PushSource(&Second); - Installer->PushSource(&Third); + Installer->PushSource(First.get()); + Installer->PushSource(Second.get()); + Installer->PushSource(Third.get()); Installer->PushWatcher(&Watcher); std::vector Args(1, "-std=c++11"); ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs( std::move(Installer), "namespace AAA { } using namespace AAB;", Args)); - ASSERT_LE(1, First.CallCount); - ASSERT_LE(1, Second.CallCount); - ASSERT_EQ(0, Third.CallCount); + ASSERT_LE(1, First->CallCount); + ASSERT_LE(1, Second->CallCount); + ASSERT_EQ(0, Third->CallCount); ASSERT_EQ(1, Watcher.SeenCount); } TEST(ExternalSemaSource, ExternalDelayedTypoCorrection) { auto Installer = std::make_unique(); - FunctionTypoProvider Provider("aaa", "bbb"); + auto Provider = makeIntrusiveRefCnt("aaa", "bbb"); DiagnosticWatcher Watcher("aaa", "bbb"); - Installer->PushSource(&Provider); + Installer->PushSource(Provider.get()); Installer->PushWatcher(&Watcher); std::vector Args(1, "-std=c++11"); ASSERT_TRUE(clang::tooling::runToolOnCodeWithArgs( std::move(Installer), "namespace AAA { } void foo() { AAA::aaa(); }", Args)); - ASSERT_LE(0, Provider.CallCount); + ASSERT_LE(0, Provider->CallCount); ASSERT_EQ(1, Watcher.SeenCount); } @@ -284,8 +286,8 @@ // solve the problem. TEST(ExternalSemaSource, TryOtherTacticsBeforeDiagnosing) { auto Installer = std::make_unique(); - CompleteTypeDiagnoser Diagnoser(false); - Installer->PushSource(&Diagnoser); + auto Diagnoser = makeIntrusiveRefCnt(false); + Installer->PushSource(Diagnoser.get()); std::vector Args(1, "-std=c++11"); // This code hits the class template specialization/class member of a class // template specialization checks in Sema::RequireCompleteTypeImpl. @@ -293,26 +295,26 @@ std::move(Installer), "template struct S { class C { }; }; S::C SCInst;", Args)); - ASSERT_EQ(0, Diagnoser.CallCount); + ASSERT_EQ(0, Diagnoser->CallCount); } // The first ExternalSemaSource where MaybeDiagnoseMissingCompleteType returns // true should be the last one called. TEST(ExternalSemaSource, FirstDiagnoserTaken) { auto Installer = std::make_unique(); - CompleteTypeDiagnoser First(false); - CompleteTypeDiagnoser Second(true); - CompleteTypeDiagnoser Third(true); - Installer->PushSource(&First); - Installer->PushSource(&Second); - Installer->PushSource(&Third); + auto First = makeIntrusiveRefCnt(false); + auto Second = makeIntrusiveRefCnt(true); + auto Third = makeIntrusiveRefCnt(true); + Installer->PushSource(First.get()); + Installer->PushSource(Second.get()); + Installer->PushSource(Third.get()); std::vector Args(1, "-std=c++11"); ASSERT_FALSE(clang::tooling::runToolOnCodeWithArgs( std::move(Installer), "class Incomplete; Incomplete IncompleteInstance;", Args)); - ASSERT_EQ(1, First.CallCount); - ASSERT_EQ(1, Second.CallCount); - ASSERT_EQ(0, Third.CallCount); + ASSERT_EQ(1, First->CallCount); + ASSERT_EQ(1, Second->CallCount); + ASSERT_EQ(0, Third->CallCount); } } // anonymous namespace