Skip to content

Commit 502773d

Browse files
committed
[lldb][NFC] Remove ImportInProgress lock in ClangASTSource
Summary: The ClangASTSource has a lock that globally disables all lookups into the external AST source when we explicitly "guarded" copy a type. It's not used for anything else, so importing declarations or importing types that are dependencies of a declaration actually won't activate that lock. The lookups it is supposed to prevent also don't actually happen in our test suite. The check in `ClangExpressionDeclMap::FindExternalVisibleDecls` is never executed and the check in the `ClangASTSource::FindExternalVisibleDeclsByName` is only ever reached by the `Import-std-module` tests (which explicitly do a lookup into the expression context on purpose). This lock was added in 6abfabf as a replacement for a list of types we already looked up which appeared to be an optimisation strategy. I assume back then this lock had a purpose but these days the ASTImporter and LLDB seem to be smart enough to avoid whatever lookups this tried to prevent. I would say we remove it from LLDB. The main reason is that it blocks D81561 (which explicitly does a specific lookup to resolve placeholder types produced by `-flimit-debug-info`) but it's semantics are also very confusing. The naming implies it's a flag to indicate when we import something at the moment which is practically never true as described above. Also the fact that it makes our ExternalASTSource alternate between doing lookups into the debug info and pretending it doesn't know any external decls could really break our lookup in some weird way if Clang decides to cache a fake empty lookup result that was generated while the lock was active. Reviewers: labath, shafik, JDevlieghere, aprantl Reviewed By: labath, JDevlieghere, aprantl Subscribers: aprantl, abidh Differential Revision: https://reviews.llvm.org/D81749
1 parent 6764869 commit 502773d

File tree

3 files changed

+3
-23
lines changed

3 files changed

+3
-23
lines changed

lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.cpp

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,9 +53,9 @@ class ScopedLexicalDeclEraser {
5353
ClangASTSource::ClangASTSource(
5454
const lldb::TargetSP &target,
5555
const std::shared_ptr<ClangASTImporter> &importer)
56-
: m_import_in_progress(false), m_lookups_enabled(false), m_target(target),
57-
m_ast_context(nullptr), m_ast_importer_sp(importer),
58-
m_active_lexical_decls(), m_active_lookups() {
56+
: m_lookups_enabled(false), m_target(target), m_ast_context(nullptr),
57+
m_ast_importer_sp(importer), m_active_lexical_decls(),
58+
m_active_lookups() {
5959
assert(m_ast_importer_sp && "No ClangASTImporter passed to ClangASTSource?");
6060
}
6161

@@ -103,11 +103,6 @@ bool ClangASTSource::FindExternalVisibleDeclsByName(
103103
return false;
104104
}
105105

106-
if (GetImportInProgress()) {
107-
SetNoExternalVisibleDeclsForName(decl_ctx, clang_decl_name);
108-
return false;
109-
}
110-
111106
std::string decl_name(clang_decl_name.getAsString());
112107

113108
switch (clang_decl_name.getNameKind()) {
@@ -1744,13 +1739,9 @@ CompilerType ClangASTSource::GuardedCopyType(const CompilerType &src_type) {
17441739
if (src_ast == nullptr)
17451740
return CompilerType();
17461741

1747-
SetImportInProgress(true);
1748-
17491742
QualType copied_qual_type = ClangUtil::GetQualType(
17501743
m_ast_importer_sp->CopyType(*m_clang_ast_context, src_type));
17511744

1752-
SetImportInProgress(false);
1753-
17541745
if (copied_qual_type.getAsOpaquePtr() &&
17551746
copied_qual_type->getCanonicalTypeInternal().isNull())
17561747
// this shouldn't happen, but we're hardening because the AST importer

lldb/source/Plugins/ExpressionParser/Clang/ClangASTSource.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -197,11 +197,6 @@ class ClangASTSource : public clang::ExternalASTSource,
197197

198198
clang::Sema *getSema();
199199

200-
void SetImportInProgress(bool import_in_progress) {
201-
m_import_in_progress = import_in_progress;
202-
}
203-
bool GetImportInProgress() { return m_import_in_progress; }
204-
205200
void SetLookupsEnabled(bool lookups_enabled) {
206201
m_lookups_enabled = lookups_enabled;
207202
}
@@ -376,7 +371,6 @@ class ClangASTSource : public clang::ExternalASTSource,
376371

377372
friend struct NameSearchContext;
378373

379-
bool m_import_in_progress;
380374
bool m_lookups_enabled;
381375

382376
/// The target to use in finding variables and types.

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -632,11 +632,6 @@ void ClangExpressionDeclMap::FindExternalVisibleDecls(
632632

633633
Log *log(lldb_private::GetLogIfAllCategoriesSet(LIBLLDB_LOG_EXPRESSIONS));
634634

635-
if (GetImportInProgress()) {
636-
LLDB_LOGV(log, "Ignoring a query during an import");
637-
return;
638-
}
639-
640635
if (log) {
641636
if (!context.m_decl_context)
642637
LLDB_LOG(log,

0 commit comments

Comments
 (0)