[clang] 5b5329b - [NFC] Make MultiplexExternalSemaSource own sources
Chris Bieneman via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 2 11:58:29 PDT 2022
Author: Chris Bieneman
Date: 2022-09-02T13:57:39-05:00
New Revision: 5b5329bd41ba977459fcd7abb7cf438fd98c98e0
URL: https://github.com/llvm/llvm-project/commit/5b5329bd41ba977459fcd7abb7cf438fd98c98e0
DIFF: https://github.com/llvm/llvm-project/commit/5b5329bd41ba977459fcd7abb7cf438fd98c98e0.diff
LOG: [NFC] Make MultiplexExternalSemaSource own sources
This change refactors the MuiltiplexExternalSemaSource to take ownership
of the underlying sources. As a result it makes a larger cleanup of
external source ownership in Sema and the ChainedIncludesSource.
Reviewed By: aaron.ballman, aprantl
Differential Revision: https://reviews.llvm.org/D133158
Added:
Modified:
clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
clang/include/clang/Sema/MultiplexExternalSemaSource.h
clang/include/clang/Sema/Sema.h
clang/lib/Frontend/ChainedIncludesSource.cpp
clang/lib/Sema/MultiplexExternalSemaSource.cpp
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/unittests/Sema/ExternalSemaSourceTest.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
index 2af1f622aa92f..45fc15619ecab 100644
--- a/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
+++ b/clang-tools-extra/clang-include-fixer/IncludeFixer.cpp
@@ -27,13 +27,14 @@ namespace {
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<clang::ASTConsumer>
CreateASTConsumer(clang::CompilerInstance &Compiler,
StringRef InFile) override {
- SemaSource.setFilePath(InFile);
+ SemaSource->setFilePath(InFile);
return std::make_unique<clang::ASTConsumer>();
}
@@ -51,8 +52,8 @@ class Action : public clang::ASTFrontendAction {
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 @@ class Action : public clang::ASTFrontendAction {
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<IncludeFixerSemaSource> SemaSource;
};
} // namespace
diff --git a/clang/include/clang/Sema/MultiplexExternalSemaSource.h b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
index 78658dcf990c4..704925577d269 100644
--- a/clang/include/clang/Sema/MultiplexExternalSemaSource.h
+++ b/clang/include/clang/Sema/MultiplexExternalSemaSource.h
@@ -40,25 +40,24 @@ class MultiplexExternalSemaSource : public ExternalSemaSource {
static char ID;
private:
- SmallVector<ExternalSemaSource *, 2> Sources; // doesn't own them.
+ SmallVector<ExternalSemaSource *, 2> 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
index bd81d3a7e5ee5..459c1109b852e 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -360,10 +360,7 @@ class Sema final {
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<ExternalSemaSource> ExternalSource;
static bool mightHaveNonExternalLinkage(const DeclaratorDecl *FD);
@@ -1637,7 +1634,7 @@ class Sema final {
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
index 380eba4562b49..c1a9f25a8798c 100644
--- 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<std::unique_ptr<CompilerInstance>> CIs)
+ ChainedIncludesSource(std::vector<std::unique_ptr<CompilerInstance>> 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 @@ class ChainedIncludesSourceImpl : public ExternalSemaSource {
private:
std::vector<std::unique_ptr<CompilerInstance>> CIs;
};
-
-/// Members of ChainedIncludesSource, factored out so we can initialize
-/// them before we initialize the ExternalSemaSource base class.
-struct ChainedIncludesSourceMembers {
- ChainedIncludesSourceMembers(
- std::vector<std::unique_ptr<CompilerInstance>> CIs,
- IntrusiveRefCntPtr<ExternalSemaSource> FinalReader)
- : Impl(std::move(CIs)), FinalReader(std::move(FinalReader)) {}
- ChainedIncludesSourceImpl Impl;
- IntrusiveRefCntPtr<ExternalSemaSource> FinalReader;
-};
-
-/// Use MultiplexExternalSemaSource to dispatch all ExternalSemaSource
-/// calls to the final reader.
-class ChainedIncludesSource
- : private ChainedIncludesSourceMembers,
- public MultiplexExternalSemaSource {
-public:
- ChainedIncludesSource(std::vector<std::unique_ptr<CompilerInstance>> CIs,
- IntrusiveRefCntPtr<ExternalSemaSource> 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 @@ IntrusiveRefCntPtr<ExternalSemaSource> clang::createChainedIncludesSource(
if (!Reader)
return nullptr;
- return IntrusiveRefCntPtr<ChainedIncludesSource>(
- new ChainedIncludesSource(std::move(CIs), Reader));
+ auto ChainedSrc =
+ llvm::makeIntrusiveRefCnt<ChainedIncludesSource>(std::move(CIs));
+ return llvm::makeIntrusiveRefCnt<MultiplexExternalSemaSource>(
+ ChainedSrc.get(), Reader.get());
}
diff --git a/clang/lib/Sema/MultiplexExternalSemaSource.cpp b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
index 072775642d75b..55e015487f3bf 100644
--- a/clang/lib/Sema/MultiplexExternalSemaSource.cpp
+++ b/clang/lib/Sema/MultiplexExternalSemaSource.cpp
@@ -16,24 +16,30 @@ using namespace clang;
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
index 04100324c786e..1158685681af1 100644
--- a/clang/lib/Sema/Sema.cpp
+++ b/clang/lib/Sema/Sema.cpp
@@ -185,11 +185,10 @@ const uint64_t Sema::MaximumAlignment;
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 @@ Sema::~Sema() {
= dyn_cast_or_null<ExternalSemaSource>(Context.getExternalSource()))
ExternalSema->ForgetSema();
- // If Sema's ExternalSource is the multiplexer - we own it.
- if (isMultiplexExternalSource)
- delete ExternalSource;
-
// Delete cached satisfactions.
std::vector<ConstraintSatisfaction *> Satisfactions;
Satisfactions.reserve(Satisfactions.size());
@@ -550,12 +545,10 @@ void Sema::addExternalSource(ExternalSemaSource *E) {
return;
}
- if (isMultiplexExternalSource)
- static_cast<MultiplexExternalSemaSource*>(ExternalSource)->addSource(*E);
- else {
- ExternalSource = new MultiplexExternalSemaSource(*ExternalSource, *E);
- isMultiplexExternalSource = true;
- }
+ if (auto *Ex = dyn_cast<MultiplexExternalSemaSource>(ExternalSource))
+ Ex->AddSource(E);
+ else
+ ExternalSource = new MultiplexExternalSemaSource(ExternalSource.get(), E);
}
/// Print out statistics about the semantic analysis.
@@ -1291,8 +1284,8 @@ void Sema::ActOnEndOfTranslationUnit() {
// translation unit, with an initializer equal to 0.
llvm::SmallSet<VarDecl *, 32> 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 @@ void Sema::ActOnEndOfTranslationUnit() {
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
index 04d06c11e9c03..76db4e5a7a3b9 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -18183,8 +18183,8 @@ void Sema::CheckDelegatingCtorCycles() {
llvm::SmallPtrSet<CXXConstructorDecl*, 4> 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
index b7679fecb4be2..88ab4309065d3 100644
--- a/clang/unittests/Sema/ExternalSemaSourceTest.cpp
+++ b/clang/unittests/Sema/ExternalSemaSourceTest.cpp
@@ -219,6 +219,8 @@ class ExternalSemaSourceInstaller : public clang::ASTFrontendAction {
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<ExternalSemaSourceInstaller>();
@@ -234,14 +236,14 @@ TEST(ExternalSemaSource, DiagCheck) {
// instead of the usual suggestion we would use above.
TEST(ExternalSemaSource, ExternalTypoCorrectionPrioritized) {
auto Installer = std::make_unique<ExternalSemaSourceInstaller>();
- NamespaceTypoProvider Provider("AAB", "BBB");
+ auto Provider = makeIntrusiveRefCnt<NamespaceTypoProvider>("AAB", "BBB");
DiagnosticWatcher Watcher("AAB", "BBB");
- Installer->PushSource(&Provider);
+ Installer->PushSource(Provider.get());
Installer->PushWatcher(&Watcher);
std::vector<std::string> 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 @@ TEST(ExternalSemaSource, ExternalTypoCorrectionPrioritized) {
// ExternalSemaSource.
TEST(ExternalSemaSource, ExternalTypoCorrectionOrdering) {
auto Installer = std::make_unique<ExternalSemaSourceInstaller>();
- NamespaceTypoProvider First("XXX", "BBB");
- NamespaceTypoProvider Second("AAB", "CCC");
- NamespaceTypoProvider Third("AAB", "DDD");
+ auto First = makeIntrusiveRefCnt<NamespaceTypoProvider>("XXX", "BBB");
+ auto Second = makeIntrusiveRefCnt<NamespaceTypoProvider>("AAB", "CCC");
+ auto Third = makeIntrusiveRefCnt<NamespaceTypoProvider>("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<std::string> 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<ExternalSemaSourceInstaller>();
- FunctionTypoProvider Provider("aaa", "bbb");
+ auto Provider = makeIntrusiveRefCnt<FunctionTypoProvider>("aaa", "bbb");
DiagnosticWatcher Watcher("aaa", "bbb");
- Installer->PushSource(&Provider);
+ Installer->PushSource(Provider.get());
Installer->PushWatcher(&Watcher);
std::vector<std::string> 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 @@ TEST(ExternalSemaSource, ExternalDelayedTypoCorrection) {
// solve the problem.
TEST(ExternalSemaSource, TryOtherTacticsBeforeDiagnosing) {
auto Installer = std::make_unique<ExternalSemaSourceInstaller>();
- CompleteTypeDiagnoser Diagnoser(false);
- Installer->PushSource(&Diagnoser);
+ auto Diagnoser = makeIntrusiveRefCnt<CompleteTypeDiagnoser>(false);
+ Installer->PushSource(Diagnoser.get());
std::vector<std::string> 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 @@ TEST(ExternalSemaSource, TryOtherTacticsBeforeDiagnosing) {
std::move(Installer),
"template <typename T> struct S { class C { }; }; S<char>::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<ExternalSemaSourceInstaller>();
- CompleteTypeDiagnoser First(false);
- CompleteTypeDiagnoser Second(true);
- CompleteTypeDiagnoser Third(true);
- Installer->PushSource(&First);
- Installer->PushSource(&Second);
- Installer->PushSource(&Third);
+ auto First = makeIntrusiveRefCnt<CompleteTypeDiagnoser>(false);
+ auto Second = makeIntrusiveRefCnt<CompleteTypeDiagnoser>(true);
+ auto Third = makeIntrusiveRefCnt<CompleteTypeDiagnoser>(true);
+ Installer->PushSource(First.get());
+ Installer->PushSource(Second.get());
+ Installer->PushSource(Third.get());
std::vector<std::string> 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
More information about the cfe-commits
mailing list