[Lldb-commits] [lldb] r373457 - [lldb][NFC] Create the ASTContext in ClangASTContext exactly once.
Raphael Isemann via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 2 05:26:08 PDT 2019
Author: teemperor
Date: Wed Oct 2 05:26:08 2019
New Revision: 373457
URL: http://llvm.org/viewvc/llvm-project?rev=373457&view=rev
Log:
[lldb][NFC] Create the ASTContext in ClangASTContext exactly once.
Reason for this patch is the Ssame reason as for the previous patches:
Having a ClangASTContext and being able to switch the associated ASTContext isn't
a use case we have (or should have), so let's simplify all this code.
This way it becomes clearer in what order we initialize data structures.
The DWARFASTParserClangTests changes are necessary as the test is using
a ClangASTContext but relied on the fact that no called function ever calls
getASTContext() on our ClangASTContext (as that would create the ASTContext).
As we now always create the ASTContext the fact that we had an uninitialized
FileSystem made the test crash.
Modified:
lldb/trunk/include/lldb/Symbol/ClangASTContext.h
lldb/trunk/source/Symbol/ClangASTContext.cpp
lldb/trunk/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
Modified: lldb/trunk/include/lldb/Symbol/ClangASTContext.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Symbol/ClangASTContext.h?rev=373457&r1=373456&r2=373457&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Symbol/ClangASTContext.h (original)
+++ lldb/trunk/include/lldb/Symbol/ClangASTContext.h Wed Oct 2 05:26:08 2019
@@ -53,6 +53,7 @@ public:
// Constructors and Destructors
explicit ClangASTContext(llvm::StringRef triple = "");
+ explicit ClangASTContext(ArchSpec arch);
/// Constructs a ClangASTContext that uses an existing ASTContext internally.
/// Useful when having an existing ASTContext created by Clang.
@@ -114,10 +115,6 @@ public:
const char *GetTargetTriple();
- void SetTargetTriple(llvm::StringRef target_triple);
-
- void SetArchitecture(const ArchSpec &arch);
-
void SetExternalSource(
llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> &ast_source_up);
@@ -1008,11 +1005,14 @@ private:
// For ClangASTContext only
ClangASTContext(const ClangASTContext &);
const ClangASTContext &operator=(const ClangASTContext &);
+ /// Creates the internal ASTContext.
+ void CreateASTContext();
+ void SetTargetTriple(llvm::StringRef target_triple);
};
class ClangASTContextForExpressions : public ClangASTContext {
public:
- ClangASTContextForExpressions(Target &target);
+ ClangASTContextForExpressions(Target &target, ArchSpec arch);
~ClangASTContextForExpressions() override = default;
Modified: lldb/trunk/source/Symbol/ClangASTContext.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Symbol/ClangASTContext.cpp?rev=373457&r1=373456&r2=373457&view=diff
==============================================================================
--- lldb/trunk/source/Symbol/ClangASTContext.cpp (original)
+++ lldb/trunk/source/Symbol/ClangASTContext.cpp Wed Oct 2 05:26:08 2019
@@ -526,6 +526,17 @@ ClangASTContext::ClangASTContext(llvm::S
: TypeSystem(TypeSystem::eKindClang) {
if (!target_triple.empty())
SetTargetTriple(target_triple);
+ // The caller didn't pass an ASTContext so create a new one for this
+ // ClangASTContext.
+ CreateASTContext();
+}
+
+ClangASTContext::ClangASTContext(ArchSpec arch)
+ : TypeSystem(TypeSystem::eKindClang) {
+ SetTargetTriple(arch.GetTriple().str());
+ // The caller didn't pass an ASTContext so create a new one for this
+ // ClangASTContext.
+ CreateASTContext();
}
ClangASTContext::ClangASTContext(ASTContext &existing_ctxt)
@@ -575,26 +586,21 @@ lldb::TypeSystemSP ClangASTContext::Crea
}
if (module) {
- std::shared_ptr<ClangASTContext> ast_sp(new ClangASTContext);
- if (ast_sp) {
- ast_sp->SetArchitecture(fixed_arch);
- }
+ std::shared_ptr<ClangASTContext> ast_sp(
+ new ClangASTContext(fixed_arch));
return ast_sp;
} else if (target && target->IsValid()) {
std::shared_ptr<ClangASTContextForExpressions> ast_sp(
- new ClangASTContextForExpressions(*target));
- if (ast_sp) {
- ast_sp->SetArchitecture(fixed_arch);
- ast_sp->m_scratch_ast_source_up.reset(
- new ClangASTSource(target->shared_from_this()));
- lldbassert(ast_sp->getFileManager());
- ast_sp->m_scratch_ast_source_up->InstallASTContext(
- *ast_sp->getASTContext(), *ast_sp->getFileManager(), true);
- llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> proxy_ast_source(
- ast_sp->m_scratch_ast_source_up->CreateProxy());
- ast_sp->SetExternalSource(proxy_ast_source);
- return ast_sp;
- }
+ new ClangASTContextForExpressions(*target, fixed_arch));
+ ast_sp->m_scratch_ast_source_up.reset(
+ new ClangASTSource(target->shared_from_this()));
+ lldbassert(ast_sp->getFileManager());
+ ast_sp->m_scratch_ast_source_up->InstallASTContext(
+ *ast_sp->getASTContext(), *ast_sp->getFileManager(), true);
+ llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> proxy_ast_source(
+ ast_sp->m_scratch_ast_source_up->CreateProxy());
+ ast_sp->SetExternalSource(proxy_ast_source);
+ return ast_sp;
}
}
}
@@ -638,11 +644,10 @@ void ClangASTContext::Terminate() {
}
void ClangASTContext::Finalize() {
- if (m_ast_up) {
- GetASTMap().Erase(m_ast_up.get());
- if (!m_ast_owned)
- m_ast_up.release();
- }
+ assert(m_ast_up);
+ GetASTMap().Erase(m_ast_up.get());
+ if (!m_ast_owned)
+ m_ast_up.release();
m_builtins_up.reset();
m_selector_table_up.reset();
@@ -652,12 +657,10 @@ void ClangASTContext::Finalize() {
m_diagnostics_engine_up.reset();
m_source_manager_up.reset();
m_language_options_up.reset();
- m_ast_up.reset();
m_scratch_ast_source_up.reset();
}
void ClangASTContext::Clear() {
- m_ast_up.reset();
m_language_options_up.reset();
m_source_manager_up.reset();
m_diagnostics_engine_up.reset();
@@ -684,10 +687,6 @@ void ClangASTContext::SetTargetTriple(ll
m_target_triple = target_triple.str();
}
-void ClangASTContext::SetArchitecture(const ArchSpec &arch) {
- SetTargetTriple(arch.GetTriple().str());
-}
-
void ClangASTContext::SetExternalSource(
llvm::IntrusiveRefCntPtr<ExternalASTSource> &ast_source_up) {
ASTContext *ast = getASTContext();
@@ -698,38 +697,41 @@ void ClangASTContext::SetExternalSource(
}
ASTContext *ClangASTContext::getASTContext() {
- if (m_ast_up == nullptr) {
- m_ast_owned = true;
- m_ast_up.reset(new ASTContext(*getLanguageOptions(), *getSourceManager(),
- *getIdentifierTable(), *getSelectorTable(),
- *getBuiltinContext()));
-
- m_ast_up->getDiagnostics().setClient(getDiagnosticConsumer(), false);
-
- // This can be NULL if we don't know anything about the architecture or if
- // the target for an architecture isn't enabled in the llvm/clang that we
- // built
- TargetInfo *target_info = getTargetInfo();
- if (target_info)
- m_ast_up->InitBuiltinTypes(*target_info);
-
- if ((m_callback_tag_decl || m_callback_objc_decl) && m_callback_baton) {
- m_ast_up->getTranslationUnitDecl()->setHasExternalLexicalStorage();
- // m_ast_up->getTranslationUnitDecl()->setHasExternalVisibleStorage();
- }
-
- GetASTMap().Insert(m_ast_up.get(), this);
-
- llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> ast_source_up(
- new ClangExternalASTSourceCallbacks(
- ClangASTContext::CompleteTagDecl,
- ClangASTContext::CompleteObjCInterfaceDecl, nullptr,
- ClangASTContext::LayoutRecordType, this));
- SetExternalSource(ast_source_up);
- }
+ assert(m_ast_up);
return m_ast_up.get();
}
+void ClangASTContext::CreateASTContext() {
+ assert(!m_ast_up);
+ m_ast_owned = true;
+ m_ast_up.reset(new ASTContext(*getLanguageOptions(), *getSourceManager(),
+ *getIdentifierTable(), *getSelectorTable(),
+ *getBuiltinContext()));
+
+ m_ast_up->getDiagnostics().setClient(getDiagnosticConsumer(), false);
+
+ // This can be NULL if we don't know anything about the architecture or if
+ // the target for an architecture isn't enabled in the llvm/clang that we
+ // built
+ TargetInfo *target_info = getTargetInfo();
+ if (target_info)
+ m_ast_up->InitBuiltinTypes(*target_info);
+
+ if ((m_callback_tag_decl || m_callback_objc_decl) && m_callback_baton) {
+ m_ast_up->getTranslationUnitDecl()->setHasExternalLexicalStorage();
+ // m_ast_up->getTranslationUnitDecl()->setHasExternalVisibleStorage();
+ }
+
+ GetASTMap().Insert(m_ast_up.get(), this);
+
+ llvm::IntrusiveRefCntPtr<clang::ExternalASTSource> ast_source_up(
+ new ClangExternalASTSourceCallbacks(
+ ClangASTContext::CompleteTagDecl,
+ ClangASTContext::CompleteObjCInterfaceDecl, nullptr,
+ ClangASTContext::LayoutRecordType, this));
+ SetExternalSource(ast_source_up);
+}
+
ClangASTContext *ClangASTContext::GetASTContext(clang::ASTContext *ast) {
ClangASTContext *clang_ast = GetASTMap().Lookup(ast);
return clang_ast;
@@ -10242,9 +10244,9 @@ ClangASTContext::DeclContextGetClangASTC
return nullptr;
}
-ClangASTContextForExpressions::ClangASTContextForExpressions(Target &target)
- : ClangASTContext(target.GetArchitecture().GetTriple().getTriple().c_str()),
- m_target_wp(target.shared_from_this()),
+ClangASTContextForExpressions::ClangASTContextForExpressions(Target &target,
+ ArchSpec arch)
+ : ClangASTContext(arch), m_target_wp(target.shared_from_this()),
m_persistent_variables(new ClangPersistentVariables) {}
UserExpression *ClangASTContextForExpressions::GetUserExpression(
Modified: lldb/trunk/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp?rev=373457&r1=373456&r2=373457&view=diff
==============================================================================
--- lldb/trunk/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp (original)
+++ lldb/trunk/unittests/SymbolFile/DWARF/DWARFASTParserClangTests.cpp Wed Oct 2 05:26:08 2019
@@ -15,6 +15,19 @@
using namespace lldb;
using namespace lldb_private;
+class DWARFASTParserClangTests : public testing::Test {
+public:
+ void SetUp() override {
+ FileSystem::Initialize();
+ ClangASTContext::Initialize();
+ }
+
+ void TearDown() override {
+ ClangASTContext::Terminate();
+ FileSystem::Terminate();
+ }
+};
+
namespace {
class DWARFASTParserClangStub : public DWARFASTParserClang {
public:
@@ -32,8 +45,8 @@ public:
// If your implementation needs to dereference the dummy pointers we are
// defining here, causing this test to fail, feel free to delete it.
-TEST(DWARFASTParserClangTests,
- EnsureAllDIEsInDeclContextHaveBeenParsedParsesOnlyMatchingEntries) {
+TEST_F(DWARFASTParserClangTests,
+ EnsureAllDIEsInDeclContextHaveBeenParsedParsesOnlyMatchingEntries) {
ClangASTContext ast_ctx;
DWARFASTParserClangStub ast_parser(ast_ctx);
More information about the lldb-commits
mailing list