diff --git a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp --- a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp +++ b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp @@ -27,13 +27,14 @@ class Action : public clang::ASTFrontendAction { public: explicit Action(SymbolIndexManager &SymbolIndexMgr, bool MinimizeIncludePaths) - : SemaSource(SymbolIndexMgr, MinimizeIncludePaths, - /*GenerateDiagnostics=*/false) {} + : SemaSource(new IncludeFixerSemaSource(SymbolIndexMgr, + MinimizeIncludePaths, + /*GenerateDiagnostics=*/false)) {} std::unique_ptr CreateASTConsumer(clang::CompilerInstance &Compiler, StringRef InFile) override { - SemaSource.setFilePath(InFile); + SemaSource->setFilePath(InFile); return std::make_unique(); } @@ -51,8 +52,8 @@ CompletionConsumer = &Compiler->getCodeCompletionConsumer(); Compiler->createSema(getTranslationUnitKind(), CompletionConsumer); - SemaSource.setCompilerInstance(Compiler); - Compiler->getSema().addExternalSource(&SemaSource); + SemaSource->setCompilerInstance(Compiler); + Compiler->getSema().addExternalSource(SemaSource.get()); clang::ParseAST(Compiler->getSema(), Compiler->getFrontendOpts().ShowStats, Compiler->getFrontendOpts().SkipFunctionBodies); @@ -61,12 +62,12 @@ IncludeFixerContext getIncludeFixerContext(const clang::SourceManager &SourceManager, clang::HeaderSearch &HeaderSearch) const { - return SemaSource.getIncludeFixerContext(SourceManager, HeaderSearch, - SemaSource.getMatchedSymbols()); + return SemaSource->getIncludeFixerContext(SourceManager, HeaderSearch, + SemaSource->getMatchedSymbols()); } private: - IncludeFixerSemaSource SemaSource; + IntrusiveRefCntPtr SemaSource; }; } // namespace 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,25 +40,24 @@ static char ID; private: - SmallVector Sources; // doesn't own them. + SmallVector Sources; public: - - ///Constructs a new multiplexing external sema source and appends the + /// 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; - ///Appends new source to the source list. + /// Appends new source to the source list. /// ///\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 @@ -16,24 +16,30 @@ char MultiplexExternalSemaSource::ID; -///Constructs a new multiplexing external sema source and appends the +/// 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. +/// 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,8 +1284,8 @@ // translation unit, with an initializer equal to 0. llvm::SmallSet Seen; for (TentativeDefinitionsType::iterator - T = TentativeDefinitions.begin(ExternalSource), - TEnd = TentativeDefinitions.end(); + T = TentativeDefinitions.begin(ExternalSource.get()), + TEnd = TentativeDefinitions.end(); T != TEnd; ++T) { VarDecl *VD = (*T)->getActingDefinition(); @@ -1336,8 +1329,9 @@ if (!Diags.hasErrorOccurred() && TUKind != TU_Module) { // Output warning for unused file scoped decls. for (UnusedFileScopedDeclsType::iterator - I = UnusedFileScopedDecls.begin(ExternalSource), - E = UnusedFileScopedDecls.end(); I != E; ++I) { + 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,8 +18183,8 @@ llvm::SmallPtrSet Valid, Invalid, Current; for (DelegatingCtorDeclsType::iterator - I = DelegatingCtorDecls.begin(ExternalSource), - E = DelegatingCtorDecls.end(); + 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