-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Introduce a DWARFImporter delegate that can look up clang::Decls by name #26493
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,7 @@ | |
#define SWIFT_AST_CLANG_MODULE_LOADER_H | ||
|
||
#include "swift/AST/ModuleLoader.h" | ||
#include "swift/Demangling/Demangle.h" | ||
|
||
namespace clang { | ||
class ASTContext; | ||
|
@@ -25,6 +26,7 @@ class Sema; | |
namespace swift { | ||
|
||
class DeclContext; | ||
class VisibleDeclConsumer; | ||
|
||
class ClangModuleLoader : public ModuleLoader { | ||
private: | ||
|
@@ -58,6 +60,30 @@ class ClangModuleLoader : public ModuleLoader { | |
const DeclContext *overlayDC, | ||
const DeclContext *importedDC) = 0; | ||
|
||
/// Look for declarations associated with the given name. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These virtual methods are necessary to break a cyclic dependency on Linux. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can you explain further? I'm not sure why there'd be a cycle. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem only existed on Linux, and no amount of duplicating llink libraries in the CMakeLists would make it go away. Part of the problem was that ASTDemangler in libAST.a had an explicit cast and a method call to DWARFImporter and libDWARFImporter.a in turn depends on libAST. I do not understand why this wasn't also a problem with ClangImporter, which effectively does the same thing. If you are concerned about the interface/performance I can try a little harder, but I also think having a common interface in the common base class of ClangImporter and DWARFImporter is actually good. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ASTDemangler definitely should not be calling DWARFImporter directly, and I'm sorry I missed that part. Let's come up with a better way to let DWARFImporter intervene in looking up Clang types. |
||
/// | ||
/// \param name The name we're searching for. | ||
virtual void lookupValue(DeclName name, VisibleDeclConsumer &consumer) {} | ||
|
||
/// Look up a type declaration by its Clang name. | ||
/// | ||
/// Note that this method does no filtering. If it finds the type in a loaded | ||
/// module, it returns it. This is intended for use in reflection / debugging | ||
/// contexts where access is not a problem. | ||
virtual void lookupTypeDecl(StringRef clangName, Demangle::Node::Kind kind, | ||
llvm::function_ref<void(TypeDecl *)> receiver) {} | ||
|
||
/// Look up type a declaration synthesized by the Clang importer itself, using | ||
/// a "related entity kind" to determine which type it should be. For example, | ||
/// this can be used to find the synthesized error struct for an | ||
/// NS_ERROR_ENUM. | ||
/// | ||
/// Note that this method does no filtering. If it finds the type in a loaded | ||
/// module, it returns it. This is intended for use in reflection / debugging | ||
/// contexts where access is not a problem. | ||
virtual void | ||
lookupRelatedEntity(StringRef clangName, StringRef relatedEntityKind, | ||
llvm::function_ref<void(TypeDecl *)> receiver) {} | ||
}; | ||
|
||
} // namespace swift | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -75,6 +75,7 @@ enum class ClangTypeKind { | |
/// from Clang ASTs over to Swift ASTs. | ||
class ClangImporter final : public ClangModuleLoader { | ||
friend class ClangModuleUnit; | ||
friend class DWARFImporter; | ||
|
||
public: | ||
class Implementation; | ||
|
@@ -160,15 +161,15 @@ class ClangImporter final : public ClangModuleLoader { | |
/// Look for declarations associated with the given name. | ||
/// | ||
/// \param name The name we're searching for. | ||
void lookupValue(DeclName name, VisibleDeclConsumer &consumer); | ||
void lookupValue(DeclName name, VisibleDeclConsumer &consumer) override; | ||
|
||
/// Look up a type declaration by its Clang name. | ||
/// | ||
/// Note that this method does no filtering. If it finds the type in a loaded | ||
/// module, it returns it. This is intended for use in reflection / debugging | ||
/// contexts where access is not a problem. | ||
void lookupTypeDecl(StringRef clangName, ClangTypeKind kind, | ||
llvm::function_ref<void(TypeDecl*)> receiver); | ||
void lookupTypeDecl(StringRef clangName, Demangle::Node::Kind kind, | ||
llvm::function_ref<void(TypeDecl*)> receiver) override; | ||
|
||
/// Look up type a declaration synthesized by the Clang importer itself, using | ||
/// a "related entity kind" to determine which type it should be. For example, | ||
|
@@ -178,9 +179,9 @@ class ClangImporter final : public ClangModuleLoader { | |
/// Note that this method does no filtering. If it finds the type in a loaded | ||
/// module, it returns it. This is intended for use in reflection / debugging | ||
/// contexts where access is not a problem. | ||
void lookupRelatedEntity(StringRef clangName, ClangTypeKind kind, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is it okay to drop the "kind" parameter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because it was not used by the function. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was deliberately an interface designed to handle arbitrary mangled names, even though we're not using the |
||
StringRef relatedEntityKind, | ||
llvm::function_ref<void(TypeDecl*)> receiver); | ||
void | ||
lookupRelatedEntity(StringRef clangName, StringRef relatedEntityKind, | ||
llvm::function_ref<void(TypeDecl *)> receiver) override; | ||
|
||
/// Look for textually included declarations from the bridging header. | ||
/// | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,8 @@ | |
#define SWIFT_DWARF_IMPORTER_H | ||
|
||
#include "swift/AST/ClangModuleLoader.h" | ||
#include "swift/AST/Module.h" | ||
#include "swift/Demangling/Demangle.h" | ||
|
||
namespace llvm { | ||
} | ||
|
@@ -27,6 +29,25 @@ namespace clang { | |
|
||
namespace swift { | ||
|
||
/// This interface is implemented by LLDB to serve as a fallback when Clang | ||
/// modules can't be imported from source in the debugger. | ||
/// | ||
/// During compile time, ClangImporter-imported Clang modules are compiled with | ||
/// -gmodules, which emits a DWARF rendition of all types defined in the module | ||
/// into the .pcm file. On Darwin, these types can be collected by | ||
/// dsymutil. This delegate allows DWARFImporter to ask LLDB to look up a Clang | ||
/// type by name, synthesize a Clang AST from it. DWARFImporter then hands this | ||
/// Clang AST to ClangImporter to import the type into Swift. | ||
class DWARFImporterDelegate { | ||
public: | ||
virtual ~DWARFImporterDelegate() {} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
/// Perform a qualified lookup of a Clang type with this name. | ||
/// \param kind Only return results with this type kind. | ||
virtual void lookupValue(StringRef name, | ||
llvm::Optional<Demangle::Node::Kind> kind, | ||
SmallVectorImpl<clang::Decl *> &results) {} | ||
}; | ||
|
||
/// Class that imports Clang modules into Swift, mapping directly | ||
/// from Clang ASTs over to Swift ASTs. | ||
class DWARFImporter final : public ClangModuleLoader { | ||
|
@@ -39,6 +60,7 @@ class DWARFImporter final : public ClangModuleLoader { | |
Implementation &Impl; | ||
|
||
DWARFImporter(ASTContext &ctx, const ClangImporterOptions &clangImporterOpts, | ||
std::unique_ptr<DWARFImporterDelegate> delegate, | ||
DependencyTracker *tracker); | ||
|
||
public: | ||
|
@@ -55,8 +77,8 @@ class DWARFImporter final : public ClangModuleLoader { | |
/// \returns a new DWARF module importer, or null (with a diagnostic) if | ||
/// an error occurred. | ||
static std::unique_ptr<DWARFImporter> | ||
create(ASTContext &ctx, | ||
const ClangImporterOptions &importerOpts, | ||
create(ASTContext &ctx, const ClangImporterOptions &importerOpts, | ||
std::unique_ptr<DWARFImporterDelegate> delegate = {}, | ||
DependencyTracker *tracker = nullptr); | ||
|
||
DWARFImporter(const DWARFImporter &) = delete; | ||
|
@@ -80,6 +102,17 @@ class DWARFImporter final : public ClangModuleLoader { | |
ModuleDecl * | ||
loadModule(SourceLoc importLoc, | ||
ArrayRef<std::pair<Identifier, SourceLoc>> path) override; | ||
|
||
ValueDecl *importDecl(clang::Decl *clangDecl); | ||
|
||
void lookupValue(DeclName name, VisibleDeclConsumer &consumer) override {} | ||
/// Behaves like \p ClangImporter::lookupValue. | ||
void lookupValue(ModuleDecl::AccessPathTy accessPath, DeclName name, | ||
NLKind lookupKind, SmallVectorImpl<ValueDecl *> &results); | ||
/// Perform a qualified lookup of a Clang type with this name and only return | ||
/// results with the specified type kind. | ||
void lookupTypeDecl(StringRef rawName, Demangle::Node::Kind kind, | ||
llvm::function_ref<void(TypeDecl *)> receiver) override; | ||
bool | ||
isInOverlayModuleForImportedModule(const DeclContext *overlayDC, | ||
const DeclContext *importedDC) override; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you get rid of this? The Clang importer is special because the Swift compiler has to use it for other things. It would definitely be bad if we had to start special-casing the DWARF loader throughout the compiler as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only use of this method is in ASTDemangler.cpp in
ASTBuilder::findForeignTypeDecl()
, which tries the DWARFImporter after the ClangImporter failed. We could changingASTContext::getClangModuleLoader()
to return a list of ClangModuleLoaders sorted by the order in which they should be queried. Would that be better?