Skip to content

[lldb] Use forward decls with redeclared definitions instead of minimal import for records #8222

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
4cefb29
[lldb][DWARFASTParserClang][NFC] Remove redundant parameter to AddMet…
Michael137 Nov 30, 2023
405141d
[ADT] Backport std::to_underlying from C++23 (#70681)
Endilll Oct 30, 2023
8904ba7
[lldb][NFC] Add an ImporterBackedASTSource that serves as base class …
Michael137 Feb 13, 2024
6b186f9
[lldb][TypeSystemClang][NFC] Make TemplateParamterInfos::pack_name a …
Michael137 Feb 13, 2024
fa12f64
[lldb][TypeSystemClang][NFC] Make TemplateParameterInfos copyable
Michael137 Feb 13, 2024
c79c9cc
[lldb][ClangASTImporter][NFCI] Implement CompleteTagDeclWithOrigin in…
Michael137 Feb 14, 2024
f6adaee
[lldb] Make TypeSystemClang work with redeclarations
Michael137 Feb 13, 2024
6b4c274
[lldb] Add typesystem.experimental.redecl-completion setting
Michael137 Jan 31, 2024
3a8d247
[clang][modules] Always let Obj-C decls query the ExternalASTSource f…
Michael137 Feb 14, 2024
3ddb1d6
[lldb][ASTUtils] Short-circuit multiplexed CompleteRedeclChain when w…
Michael137 Feb 14, 2024
a4ee6a9
[lldb][ClangASTImporter] Assert that we never overwrite a record layout
Michael137 Feb 14, 2024
1400e44
[lldb][ClangASTImporter][NFC] Factor completion logic out of ~Complet…
Michael137 Feb 14, 2024
4e1b128
[lldb][ClangASTImporter][NFCI] Factor setting external storage out of…
Michael137 Feb 14, 2024
bc1a466
[lldb][ClangASTSource] Use getDefinition() where appropriate
Michael137 Feb 15, 2024
27246c7
[lldb][ExpressionParser][NFC] Implement CompleteRedeclChain APIs
Michael137 Feb 15, 2024
0c17fe0
[lldb][ClangASTImporter][NFC] Factor out CanImport logic
Michael137 Feb 16, 2024
0335368
[lldb][TypeSystemClang][NFCI] Factor completion logic out of GetCompl…
Michael137 Feb 16, 2024
313a1af
[lldb][ClangASTImporter][NFC] Move ASTImproterDelegate constructor de…
Michael137 Feb 19, 2024
4032875
[clang][ASTImporter] Add support for LLDB's new type lookup mechanism
Michael137 Feb 19, 2024
66e12b4
[lldb][WIP] Use forward decls with redeclared definitions instead of …
Michael137 Feb 15, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions clang/include/clang/AST/ASTImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ class TypeSourceInfo;

/// Whether to perform a minimal import.
bool Minimal;
bool LLDBRedeclCompletion = false;

ODRHandlingType ODRHandling;

Expand Down Expand Up @@ -296,8 +297,10 @@ class TypeSourceInfo;
/// Whether the importer will perform a minimal import, creating
/// to-be-completed forward declarations when possible.
bool isMinimalImport() const { return Minimal; }
bool hasLLDBRedeclCompletion() const { return LLDBRedeclCompletion; }

void setODRHandling(ODRHandlingType T) { ODRHandling = T; }
void setLLDBRedeclCompletion(bool Val) { LLDBRedeclCompletion = Val; }

/// \brief Import the given object, returns the result.
///
Expand Down
10 changes: 6 additions & 4 deletions clang/include/clang/AST/DeclObjC.h
Original file line number Diff line number Diff line change
Expand Up @@ -1238,7 +1238,8 @@ class ObjCInterfaceDecl : public ObjCContainerDecl
/// which will be NULL if this class has not yet been defined.
///
/// The bit indicates when we don't need to check for out-of-date
/// declarations. It will be set unless modules are enabled.
/// declarations. It will be set unless there is an ExternalASTSource that
/// could provide a definition.
llvm::PointerIntPair<DefinitionData *, 1, bool> Data;

ObjCInterfaceDecl(const ASTContext &C, DeclContext *DC, SourceLocation AtLoc,
Expand Down Expand Up @@ -1527,7 +1528,7 @@ class ObjCInterfaceDecl : public ObjCContainerDecl
// If the name of this class is out-of-date, bring it up-to-date, which
// might bring in a definition.
// Note: a null value indicates that we don't have a definition and that
// modules are enabled.
// there is a ExternalASTSource that could provide a definition.
if (!Data.getOpaqueValue())
getMostRecentDecl();

Expand Down Expand Up @@ -2095,7 +2096,8 @@ class ObjCProtocolDecl : public ObjCContainerDecl,
/// which will be NULL if this class has not yet been defined.
///
/// The bit indicates when we don't need to check for out-of-date
/// declarations. It will be set unless modules are enabled.
/// declarations. It will be set unless there is an ExternalASTSource that
/// could provide a definition.
llvm::PointerIntPair<DefinitionData *, 1, bool> Data;

ObjCProtocolDecl(ASTContext &C, DeclContext *DC, IdentifierInfo *Id,
Expand Down Expand Up @@ -2232,7 +2234,7 @@ class ObjCProtocolDecl : public ObjCContainerDecl,
// If the name of this protocol is out-of-date, bring it up-to-date, which
// might bring in a definition.
// Note: a null value indicates that we don't have a definition and that
// modules are enabled.
// there is a ExternalASTSource that could provide a definition.
if (!Data.getOpaqueValue())
getMostRecentDecl();

Expand Down
50 changes: 35 additions & 15 deletions clang/lib/AST/ASTImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1031,7 +1031,8 @@ Expected<LambdaCapture> ASTNodeImporter::import(const LambdaCapture &From) {

template <typename T>
bool ASTNodeImporter::hasSameVisibilityContextAndLinkage(T *Found, T *From) {
if (Found->getLinkageInternal() != From->getLinkageInternal())
if (!Importer.hasLLDBRedeclCompletion() &&
Found->getLinkageInternal() != From->getLinkageInternal())
return false;

if (From->hasExternalFormalLinkage())
Expand Down Expand Up @@ -1482,7 +1483,9 @@ ExpectedType ASTNodeImporter::VisitInjectedClassNameType(
}

ExpectedType ASTNodeImporter::VisitRecordType(const RecordType *T) {
// getCanonicalDecl in order to not trigger redeclaration completion
Expected<RecordDecl *> ToDeclOrErr = import(T->getDecl());

if (!ToDeclOrErr)
return ToDeclOrErr.takeError();

Expand Down Expand Up @@ -1771,8 +1774,9 @@ Error ASTNodeImporter::ImportDefinitionIfNeeded(Decl *FromD, Decl *ToD) {

if (RecordDecl *FromRecord = dyn_cast<RecordDecl>(FromD)) {
if (RecordDecl *ToRecord = cast<RecordDecl>(ToD)) {
if (FromRecord->getDefinition() && FromRecord->isCompleteDefinition() &&
!ToRecord->getDefinition()) {
if (FromRecord->getDefinition() && !ToRecord->getDefinition() &&
(Importer.hasLLDBRedeclCompletion() ||
FromRecord->isCompleteDefinition())) {
if (Error Err = ImportDefinition(FromRecord, ToRecord))
return Err;
}
Expand Down Expand Up @@ -1873,12 +1877,15 @@ ASTNodeImporter::ImportDeclContext(DeclContext *FromDC, bool ForceImport) {
ImportedOrErr.takeError());
continue;
}
FieldDecl *FieldFrom = dyn_cast_or_null<FieldDecl>(From);
Decl *ImportedDecl = *ImportedOrErr;
FieldDecl *FieldTo = dyn_cast_or_null<FieldDecl>(ImportedDecl);
if (FieldFrom && FieldTo) {
Error Err = ImportFieldDeclDefinition(FieldFrom, FieldTo);
HandleChildErrors.handleChildImportResult(ChildErrors, std::move(Err));

if (Importer.hasLLDBRedeclCompletion()) {
FieldDecl *FieldFrom = dyn_cast_or_null<FieldDecl>(From);
Decl *ImportedDecl = *ImportedOrErr;
FieldDecl *FieldTo = dyn_cast_or_null<FieldDecl>(ImportedDecl);
if (FieldFrom && FieldTo) {
Error Err = ImportFieldDeclDefinition(FieldFrom, FieldTo);
HandleChildErrors.handleChildImportResult(ChildErrors, std::move(Err));
}
}
}

Expand Down Expand Up @@ -2039,7 +2046,11 @@ Error ASTNodeImporter::ImportDefinition(
To->completeDefinition();
};

if (To->getDefinition() || To->isBeingDefined()) {
bool hasDef = (Importer.hasLLDBRedeclCompletion() &&
To->isThisDeclarationADefinition()) ||
To->getDefinition();

if (hasDef || To->isBeingDefined()) {
if (Kind == IDK_Everything ||
// In case of lambdas, the class already has a definition ptr set, but
// the contained decls are not imported yet. Also, isBeingDefined was
Expand Down Expand Up @@ -2558,6 +2569,9 @@ ASTNodeImporter::VisitTypedefNameDecl(TypedefNameDecl *D, bool IsAlias) {
!hasSameVisibilityContextAndLinkage(FoundR, FromR))
continue;
}

if (Importer.hasLLDBRedeclCompletion() && Importer.isMinimalImport())
return Importer.MapImported(D, FoundTypedef);
// If the "From" context has a complete underlying type but we
// already have a complete underlying type then return with that.
if (!FromUT->isIncompleteType() && !FoundUT->isIncompleteType())
Expand Down Expand Up @@ -2822,9 +2836,11 @@ ExpectedDecl ASTNodeImporter::VisitEnumDecl(EnumDecl *D) {
return POIOrErr.takeError();
}

auto Kind = Importer.hasLLDBRedeclCompletion() ? IDK_Everything : IDK_Default;

// Import the definition
if (D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2))
if (Error Err = ImportDefinition(D, D2, Kind))
return std::move(Err);

return D2;
Expand Down Expand Up @@ -2902,7 +2918,8 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {

if (IsStructuralMatch(D, FoundRecord)) {
RecordDecl *FoundDef = FoundRecord->getDefinition();
if (D->isThisDeclarationADefinition() && FoundDef) {
if (!Importer.hasLLDBRedeclCompletion() &&
D->isThisDeclarationADefinition() && FoundDef) {
// FIXME: Structural equivalence check should check for same
// user-defined methods.
Importer.MapImported(D, FoundDef);
Expand Down Expand Up @@ -3078,8 +3095,9 @@ ExpectedDecl ASTNodeImporter::VisitRecordDecl(RecordDecl *D) {
if (D->isAnonymousStructOrUnion())
D2->setAnonymousStructOrUnion(true);

auto Kind = Importer.hasLLDBRedeclCompletion() ? IDK_Everything : IDK_Default;
if (D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2, IDK_Default))
if (Error Err = ImportDefinition(D, D2, Kind))
return std::move(Err);

return D2;
Expand Down Expand Up @@ -5203,7 +5221,8 @@ Error ASTNodeImporter::ImportDefinition(
diag::note_odr_objc_missing_superclass);
}

if (shouldForceImportDeclContext(Kind))
if (Importer.hasLLDBRedeclCompletion() ||
shouldForceImportDeclContext(Kind))
if (Error Err = ImportDeclContext(From))
return Err;
return Error::success();
Expand Down Expand Up @@ -6084,8 +6103,9 @@ ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl(
}
}

auto Kind = Importer.hasLLDBRedeclCompletion() ? IDK_Everything : IDK_Default;
if (D->isCompleteDefinition())
if (Error Err = ImportDefinition(D, D2))
if (Error Err = ImportDefinition(D, D2, Kind))
return std::move(Err);

return D2;
Expand Down
8 changes: 4 additions & 4 deletions clang/lib/AST/DeclObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1546,7 +1546,7 @@ ObjCInterfaceDecl *ObjCInterfaceDecl::Create(const ASTContext &C,
auto *Result = new (C, DC)
ObjCInterfaceDecl(C, DC, atLoc, Id, typeParamList, ClassLoc, PrevDecl,
isInternal);
Result->Data.setInt(!C.getLangOpts().Modules);
Result->Data.setInt(!C.getExternalSource());
C.getObjCInterfaceType(Result, PrevDecl);
return Result;
}
Expand All @@ -1556,7 +1556,7 @@ ObjCInterfaceDecl *ObjCInterfaceDecl::CreateDeserialized(const ASTContext &C,
auto *Result = new (C, ID)
ObjCInterfaceDecl(C, nullptr, SourceLocation(), nullptr, nullptr,
SourceLocation(), nullptr, false);
Result->Data.setInt(!C.getLangOpts().Modules);
Result->Data.setInt(!C.getExternalSource());
return Result;
}

Expand Down Expand Up @@ -1947,7 +1947,7 @@ ObjCProtocolDecl *ObjCProtocolDecl::Create(ASTContext &C, DeclContext *DC,
ObjCProtocolDecl *PrevDecl) {
auto *Result =
new (C, DC) ObjCProtocolDecl(C, DC, Id, nameLoc, atStartLoc, PrevDecl);
Result->Data.setInt(!C.getLangOpts().Modules);
Result->Data.setInt(!C.getExternalSource());
return Result;
}

Expand All @@ -1956,7 +1956,7 @@ ObjCProtocolDecl *ObjCProtocolDecl::CreateDeserialized(ASTContext &C,
ObjCProtocolDecl *Result =
new (C, ID) ObjCProtocolDecl(C, nullptr, nullptr, SourceLocation(),
SourceLocation(), nullptr);
Result->Data.setInt(!C.getLangOpts().Modules);
Result->Data.setInt(!C.getExternalSource());
return Result;
}

Expand Down
8 changes: 8 additions & 0 deletions lldb/include/lldb/Core/PluginManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,7 @@ class PluginManager {
// TypeSystem
static bool RegisterPlugin(llvm::StringRef name, llvm::StringRef description,
TypeSystemCreateInstance create_callback,
DebuggerInitializeCallback debugger_callback,
LanguageSet supported_languages_for_types,
LanguageSet supported_languages_for_expressions);

Expand Down Expand Up @@ -528,6 +529,13 @@ class PluginManager {
Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
llvm::StringRef description, bool is_global_property);

static lldb::OptionValuePropertiesSP
GetSettingForTypeSystemPlugin(Debugger &debugger, ConstString setting_name);

static bool CreateSettingForTypeSystemPlugin(
Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
ConstString description, bool is_global_property);

static bool CreateSettingForTracePlugin(
Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
llvm::StringRef description, bool is_global_property);
Expand Down
28 changes: 24 additions & 4 deletions lldb/source/Core/PluginManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1423,10 +1423,11 @@ PluginManager::GetInstrumentationRuntimeCreateCallbackAtIndex(uint32_t idx) {
struct TypeSystemInstance : public PluginInstance<TypeSystemCreateInstance> {
TypeSystemInstance(llvm::StringRef name, llvm::StringRef description,
CallbackType create_callback,
DebuggerInitializeCallback debugger_init_callback,
LanguageSet supported_languages_for_types,
LanguageSet supported_languages_for_expressions)
: PluginInstance<TypeSystemCreateInstance>(name, description,
create_callback),
: PluginInstance<TypeSystemCreateInstance>(
name, description, create_callback, debugger_init_callback),
supported_languages_for_types(supported_languages_for_types),
supported_languages_for_expressions(
supported_languages_for_expressions) {}
Expand All @@ -1445,11 +1446,12 @@ static TypeSystemInstances &GetTypeSystemInstances() {
bool PluginManager::RegisterPlugin(
llvm::StringRef name, llvm::StringRef description,
TypeSystemCreateInstance create_callback,
DebuggerInitializeCallback debugger_init_callback,
LanguageSet supported_languages_for_types,
LanguageSet supported_languages_for_expressions) {
return GetTypeSystemInstances().RegisterPlugin(
name, description, create_callback, supported_languages_for_types,
supported_languages_for_expressions);
name, description, create_callback, debugger_init_callback,
supported_languages_for_types, supported_languages_for_expressions);
}

bool PluginManager::UnregisterPlugin(TypeSystemCreateInstance create_callback) {
Expand Down Expand Up @@ -1536,6 +1538,7 @@ void PluginManager::DebuggerInitialize(Debugger &debugger) {
GetOperatingSystemInstances().PerformDebuggerCallback(debugger);
GetStructuredDataPluginInstances().PerformDebuggerCallback(debugger);
GetTracePluginInstances().PerformDebuggerCallback(debugger);
GetTypeSystemInstances().PerformDebuggerCallback(debugger);
}

// This is the preferred new way to register plugin specific settings. e.g.
Expand Down Expand Up @@ -1660,6 +1663,7 @@ static constexpr llvm::StringLiteral kProcessPluginName("process");
static constexpr llvm::StringLiteral kTracePluginName("trace");
static constexpr llvm::StringLiteral kObjectFilePluginName("object-file");
static constexpr llvm::StringLiteral kSymbolFilePluginName("symbol-file");
static constexpr llvm::StringLiteral kTypeSystemPluginName("typesystem");
static constexpr llvm::StringLiteral kJITLoaderPluginName("jit-loader");
static constexpr llvm::StringLiteral
kStructuredDataPluginName("structured-data");
Expand Down Expand Up @@ -1744,6 +1748,22 @@ bool PluginManager::CreateSettingForSymbolFilePlugin(
properties_sp, description, is_global_property);
}

lldb::OptionValuePropertiesSP
PluginManager::GetSettingForTypeSystemPlugin(Debugger &debugger,
ConstString setting_name) {
return GetSettingForPlugin(debugger, setting_name,
ConstString(kTypeSystemPluginName));
}

bool PluginManager::CreateSettingForTypeSystemPlugin(
Debugger &debugger, const lldb::OptionValuePropertiesSP &properties_sp,
ConstString description, bool is_global_property) {
return CreateSettingForPlugin(
debugger, ConstString(kTypeSystemPluginName),
ConstString("Settings for type system plug-ins"), properties_sp,
description, is_global_property);
}

lldb::OptionValuePropertiesSP
PluginManager::GetSettingForJITLoaderPlugin(Debugger &debugger,
llvm::StringRef setting_name) {
Expand Down
14 changes: 11 additions & 3 deletions lldb/source/Plugins/ExpressionParser/Clang/ASTUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#ifndef LLDB_SOURCE_PLUGINS_EXPRESSIONPARSER_CLANG_ASTUTILS_H
#define LLDB_SOURCE_PLUGINS_EXPRESSIONPARSER_CLANG_ASTUTILS_H

#include "Plugins/TypeSystem/Clang/ImporterBackedASTSource.h"
#include "clang/Basic/Module.h"
#include "clang/Sema/Lookup.h"
#include "clang/Sema/MultiplexExternalSemaSource.h"
Expand All @@ -20,7 +21,7 @@ namespace lldb_private {

/// Wraps an ExternalASTSource into an ExternalSemaSource. Doesn't take
/// ownership of the provided source.
class ExternalASTSourceWrapper : public clang::ExternalSemaSource {
class ExternalASTSourceWrapper : public ImporterBackedASTSource {
ExternalASTSource *m_Source;

public:
Expand Down Expand Up @@ -240,7 +241,7 @@ class ASTConsumerForwarder : public clang::SemaConsumer {
/// provide more accurate replies to the requests, but might not be able to
/// answer all requests. The debug information will be used as a fallback then
/// to provide information that is not in the C++ module.
class SemaSourceWithPriorities : public clang::ExternalSemaSource {
class SemaSourceWithPriorities : public ImporterBackedASTSource {

private:
/// The sources ordered in decreasing priority.
Expand Down Expand Up @@ -273,9 +274,16 @@ class SemaSourceWithPriorities : public clang::ExternalSemaSource {
return nullptr;
}

/// Call ExternalASTSource::CompleteRedeclChain(D)
/// on each AST source. Returns as soon as we got
/// a definition for D.
void CompleteRedeclChain(const clang::Decl *D) override {
for (size_t i = 0; i < Sources.size(); ++i)
for (size_t i = 0; i < Sources.size(); ++i) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
for (size_t i = 0; i < Sources.size(); ++i) {
for (auto &Source : Sources) {

Sources[i]->CompleteRedeclChain(D);
if (auto *td = llvm::dyn_cast<clang::TagDecl>(D))
if (td->getDefinition())
return;
}
}

clang::Selector GetExternalSelector(uint32_t ID) override {
Expand Down
Loading