Skip to content

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

Merged
merged 1 commit into from
Aug 7, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion include/swift/AST/ASTContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -639,8 +639,10 @@ class ASTContext final {
/// \param isClang \c true if this module loader is responsible for loading
/// Clang modules, which are special-cased in some parts of the
/// compiler.
/// \param isDWARF \c true if this module loader can load Clang modules
/// from DWARF.
void addModuleLoader(std::unique_ptr<ModuleLoader> loader,
bool isClang = false);
bool isClang = false, bool isDWARF = false);

/// Load extensions to the given nominal type from the external
/// module loaders.
Expand Down Expand Up @@ -683,6 +685,12 @@ class ASTContext final {
/// The loader is owned by the AST context.
ClangModuleLoader *getClangModuleLoader() const;

/// Retrieve the DWARF module loader for this ASTContext.
///
/// If there is no Clang module loader, returns a null pointer.
/// The loader is owned by the AST context.
ClangModuleLoader *getDWARFModuleLoader() const;
Copy link
Contributor

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.

Copy link
Contributor Author

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 changing ASTContext::getClangModuleLoader() to return a list of ClangModuleLoaders sorted by the order in which they should be queried. Would that be better?


/// Asks every module loader to verify the ASTs it has loaded.
///
/// Does nothing in non-asserts (NDEBUG) builds.
Expand Down
26 changes: 26 additions & 0 deletions include/swift/AST/ClangModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,6 +26,7 @@ class Sema;
namespace swift {

class DeclContext;
class VisibleDeclConsumer;

class ClangModuleLoader : public ModuleLoader {
private:
Expand Down Expand Up @@ -58,6 +60,30 @@ class ClangModuleLoader : public ModuleLoader {
const DeclContext *overlayDC,
const DeclContext *importedDC) = 0;

/// Look for declarations associated with the given name.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These virtual methods are necessary to break a cyclic dependency on Linux.

Copy link
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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.

Copy link
Contributor

Choose a reason for hiding this comment

The 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
Expand Down
1 change: 1 addition & 0 deletions include/swift/AST/ModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class ClangImporterOptions;
class ClassDecl;
class ModuleDecl;
class NominalTypeDecl;
class TypeDecl;

enum class KnownProtocolKind : uint8_t;

Expand Down
13 changes: 7 additions & 6 deletions include/swift/ClangImporter/ClangImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it okay to drop the "kind" parameter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because it was not used by the function.

Copy link
Contributor

Choose a reason for hiding this comment

The 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 kind for anything yet.

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.
///
Expand Down
37 changes: 35 additions & 2 deletions include/swift/DWARFImporter/DWARFImporter.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
}
Expand All @@ -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() {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: = default allows a tiny bit more optimization in some cases.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 {
Expand All @@ -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:
Expand All @@ -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;
Expand All @@ -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;
Expand Down
14 changes: 12 additions & 2 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@ FOR_KNOWN_FOUNDATION_TYPES(CACHE_FOUNDATION_DECL)
/// The module loader used to load Clang modules.
ClangModuleLoader *TheClangModuleLoader = nullptr;

/// The module loader used to load Clang modules from DWARF.
ClangModuleLoader *TheDWARFModuleLoader = nullptr;

/// Map from Swift declarations to raw comments.
llvm::DenseMap<const Decl *, RawComment> RawComments;

Expand Down Expand Up @@ -1446,10 +1449,13 @@ void ASTContext::addSearchPath(StringRef searchPath, bool isFramework,
}

void ASTContext::addModuleLoader(std::unique_ptr<ModuleLoader> loader,
bool IsClang) {
if (IsClang && !getImpl().TheClangModuleLoader)
bool IsClang, bool IsDwarf) {
if (IsClang && !IsDwarf && !getImpl().TheClangModuleLoader)
getImpl().TheClangModuleLoader =
static_cast<ClangModuleLoader *>(loader.get());
if (IsClang && IsDwarf && !getImpl().TheDWARFModuleLoader)
getImpl().TheDWARFModuleLoader =
static_cast<ClangModuleLoader *>(loader.get());

getImpl().ModuleLoaders.push_back(std::move(loader));
}
Expand Down Expand Up @@ -1493,6 +1499,10 @@ ClangModuleLoader *ASTContext::getClangModuleLoader() const {
return getImpl().TheClangModuleLoader;
}

ClangModuleLoader *ASTContext::getDWARFModuleLoader() const {
return getImpl().TheDWARFModuleLoader;
}

ModuleDecl *ASTContext::getLoadedModule(
ArrayRef<std::pair<Identifier, SourceLoc>> ModulePath) const {
assert(!ModulePath.empty());
Expand Down
73 changes: 26 additions & 47 deletions lib/AST/ASTDemangler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,14 @@
#include "swift/AST/ASTDemangler.h"

#include "swift/AST/ASTContext.h"
#include "swift/AST/ClangModuleLoader.h"
#include "swift/AST/Decl.h"
#include "swift/AST/GenericSignature.h"
#include "swift/AST/GenericSignatureBuilder.h"
#include "swift/AST/Module.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/Type.h"
#include "swift/AST/Types.h"
#include "swift/ClangImporter/ClangImporter.h"
#include "swift/Demangling/Demangler.h"
#include "swift/Demangling/ManglingMacros.h"

Expand Down Expand Up @@ -1023,31 +1023,14 @@ ASTBuilder::findTypeDecl(DeclContext *dc,
return result;
}

static Optional<ClangTypeKind>
getClangTypeKindForNodeKind(Demangle::Node::Kind kind) {
switch (kind) {
case Demangle::Node::Kind::Protocol:
return ClangTypeKind::ObjCProtocol;
case Demangle::Node::Kind::Class:
return ClangTypeKind::ObjCClass;
case Demangle::Node::Kind::TypeAlias:
return ClangTypeKind::Typedef;
case Demangle::Node::Kind::Structure:
case Demangle::Node::Kind::Enum:
return ClangTypeKind::Tag;
default:
return None;
}
}

GenericTypeDecl *
ASTBuilder::findForeignTypeDecl(StringRef name,
StringRef relatedEntityKind,
ForeignModuleKind foreignKind,
Demangle::Node::Kind kind) {
GenericTypeDecl *ASTBuilder::findForeignTypeDecl(StringRef name,
StringRef relatedEntityKind,
ForeignModuleKind foreignKind,
Demangle::Node::Kind kind) {
// Check to see if we have an importer loaded.
auto importer = static_cast<ClangImporter *>(Ctx.getClangModuleLoader());
if (!importer) return nullptr;
auto importer = Ctx.getClangModuleLoader();
if (!importer)
return nullptr;

// Find the unique declaration that has the right kind.
struct Consumer : VisibleDeclConsumer {
Expand All @@ -1059,8 +1042,10 @@ ASTBuilder::findForeignTypeDecl(StringRef name,

void foundDecl(ValueDecl *decl, DeclVisibilityKind reason,
DynamicLookupInfo dynamicLookupInfo = {}) override {
if (HadError) return;
if (decl == Result) return;
if (HadError)
return;
if (decl == Result)
return;
if (!Result) {
Result = dyn_cast<GenericTypeDecl>(decl);
HadError |= !Result;
Expand All @@ -1071,33 +1056,27 @@ ASTBuilder::findForeignTypeDecl(StringRef name,
}
} consumer(kind);

auto found = [&](TypeDecl *found) {
consumer.foundDecl(found, DeclVisibilityKind::VisibleAtTopLevel);
};

switch (foreignKind) {
case ForeignModuleKind::SynthesizedByImporter:
if (!relatedEntityKind.empty()) {
Optional<ClangTypeKind> lookupKind = getClangTypeKindForNodeKind(kind);
if (!lookupKind)
return nullptr;
importer->lookupRelatedEntity(name, lookupKind.getValue(),
relatedEntityKind, [&](TypeDecl *found) {
consumer.foundDecl(found, DeclVisibilityKind::VisibleAtTopLevel);
});
importer->lookupRelatedEntity(name, relatedEntityKind, found);
break;
}
importer->lookupValue(Ctx.getIdentifier(name), consumer);
if (consumer.Result) {
consumer.Result =
getAcceptableTypeDeclCandidate(consumer.Result,kind);
}
if (consumer.Result)
consumer.Result = getAcceptableTypeDeclCandidate(consumer.Result, kind);
break;
case ForeignModuleKind::Imported: {
Optional<ClangTypeKind> lookupKind = getClangTypeKindForNodeKind(kind);
if (!lookupKind)
return nullptr;
importer->lookupTypeDecl(name, lookupKind.getValue(),
[&](TypeDecl *found) {
consumer.foundDecl(found, DeclVisibilityKind::VisibleAtTopLevel);
});
}
case ForeignModuleKind::Imported:
importer->lookupTypeDecl(name, kind, found);

// Try the DWARFImporter if it exists.
if (!consumer.Result)
if (auto *dwarf_importer = Ctx.getDWARFModuleLoader())
dwarf_importer->lookupTypeDecl(name, kind, found);
}

return consumer.Result;
Expand Down
Loading