Skip to content

[Module aliasing] Serialize SIL and binaries with module real names #39705

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 8 commits into from
Oct 19, 2021
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
8 changes: 5 additions & 3 deletions include/swift/AST/FileUnit.h
Original file line number Diff line number Diff line change
Expand Up @@ -288,12 +288,14 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
return nullptr;
}

/// Returns the name to use when referencing entities in this file.
/// Returns the real name of the enclosing module to use when referencing entities in this file.
/// The 'real name' is the actual binary name of the module, which can be different from the 'name'
/// if module aliasing was used (via -module-alias flag).
///
/// Usually this is the module name itself, but certain Clang features allow
/// Usually this is the module real name itself, but certain Clang features allow
/// substituting another name instead.
virtual StringRef getExportedModuleName() const {
return getParentModule()->getName().str();
return getParentModule()->getRealName().str();
Copy link
Contributor

Choose a reason for hiding this comment

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

tl;dr: Looks good.

I checked the use sites of getExportedModuleName() and there's one—TypePrinter::printModuleContext()—that looked iffy. It's part of ASTPrinter, which means it serves multiple masters. However, it looks like the exported module name is only used there when we're generating a swiftinterface file, where we really would want the real name, so I think this is okay.

}

/// Traverse the decls within this file.
Expand Down
12 changes: 12 additions & 0 deletions include/swift/AST/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,9 @@ class ModuleDecl
///
/// For a Swift module, this will only ever have one component, but an
/// imported Clang module might actually be a submodule.
///
/// *Note: see `StringRef operator*()` for details on the returned name for printing
/// for a Swift module.
class ReverseFullNameIterator {
public:
// Make this look like a valid STL iterator.
Expand All @@ -194,6 +197,9 @@ class ModuleDecl
current = clangModule;
}

/// Returns the name of the current module.
/// Note that for a Swift module, it returns the current module's real (binary) name,
/// which can be different from the name if module aliasing was used (see `-module-alias`).
StringRef operator*() const;
ReverseFullNameIterator &operator++();

Expand All @@ -208,6 +214,9 @@ class ModuleDecl

/// This is a convenience function that writes the entire name, in forward
/// order, to \p out.
///
/// It calls `StringRef operator*()` under the hood (see for more detail on the
/// returned name for a Swift module).
void printForward(raw_ostream &out, StringRef delim = ".") const;
};

Expand Down Expand Up @@ -793,6 +802,9 @@ class ModuleDecl
///
/// For a Swift module, this will only ever have one component, but an
/// imported Clang module might actually be a submodule.
///
/// *Note: see `StringRef operator*()` for details on the returned name for printing
/// for a Swift module.
ReverseFullNameIterator getReverseFullModuleName() const {
return ReverseFullNameIterator(this);
}
Expand Down
2 changes: 1 addition & 1 deletion include/swift/ClangImporter/ClangModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ class ClangModuleUnit final : public LoadedFile {
ModuleDecl *getOverlayModule() const override;

/// Retrieve the "exported" name of the module, which is usually the module
/// name, but might be the name of the public module through which this
/// real name, but might be the name of the public module through which this
/// (private) module is re-exported.
StringRef getExportedModuleName() const override;

Expand Down
23 changes: 15 additions & 8 deletions lib/AST/ASTPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2337,13 +2337,16 @@ void PrintAST::visitImportDecl(ImportDecl *decl) {
llvm::interleave(decl->getImportPath(),
[&](const ImportPath::Element &Elem) {
if (!Mods.empty()) {
Identifier Name = Elem.Item;
// Should print the module real name in case module
// aliasing is used (see -module-alias), since that's
// the actual binary name.
Identifier Name = decl->getASTContext().getRealModuleName(Elem.Item);
if (Options.MapCrossImportOverlaysToDeclaringModule) {
if (auto *MD = Mods.front().getAsSwiftModule()) {
ModuleDecl *Declaring = const_cast<ModuleDecl*>(MD)
->getDeclaringModuleIfCrossImportOverlay();
if (Declaring)
Name = Declaring->getName();
Name = Declaring->getRealName();
}
}
Printer.printModuleRef(Mods.front(), Name);
Expand Down Expand Up @@ -4321,7 +4324,9 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
Mod = Declaring;
}

Identifier Name = Mod->getName();
// Should use the module real (binary) name here and everywhere else the
// module is printed in case module aliasing is used (see -module-alias)
Identifier Name = Mod->getRealName();
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh. These ASTPrinter cases are interesting, because I think some clients (like .swiftinterface printing) will want the real name and some (diagnostics, possibly some SourceKit clients) will want the alias name.

It might make sense to base this decision on the PrintOptions—either by putting a bool flag there, or by actually putting a map of aliases there.

(It’s probably okay to merge this as-is for now and come back to it later.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good; will create a separate PR to address that.

if (Options.UseExportedModuleNames && !ExportedModuleName.empty()) {
Name = Mod->getASTContext().getIdentifier(ExportedModuleName);
}
Expand All @@ -4342,7 +4347,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
bool isLLDBExpressionModule(ModuleDecl *M) {
if (!M)
return false;
return M->getName().str().startswith(LLDB_EXPRESSIONS_MODULE_NAME_PREFIX);
return M->getRealName().str().startswith(LLDB_EXPRESSIONS_MODULE_NAME_PREFIX);
}

bool shouldPrintFullyQualified(TypeBase *T) {
Expand Down Expand Up @@ -4373,7 +4378,7 @@ class TypePrinter : public TypeVisitor<TypePrinter> {

// Don't print qualifiers for types from the standard library.
if (M->isStdlibModule() ||
M->getName() == M->getASTContext().Id_ObjectiveC ||
M->getRealName() == M->getASTContext().Id_ObjectiveC ||
M->isSystemModule() ||
isLLDBExpressionModule(M))
return false;
Expand Down Expand Up @@ -4636,7 +4641,9 @@ class TypePrinter : public TypeVisitor<TypePrinter> {

void visitModuleType(ModuleType *T) {
Printer << "module<";
Printer.printModuleRef(T->getModule(), T->getModule()->getName());
// Should print the module real name in case module aliasing is
// used (see -module-alias), since that's the actual binary name.
Printer.printModuleRef(T->getModule(), T->getModule()->getRealName());
Printer << ">";
}

Expand Down Expand Up @@ -5696,13 +5703,13 @@ void ProtocolConformance::printName(llvm::raw_ostream &os,
case ProtocolConformanceKind::Normal: {
auto normal = cast<NormalProtocolConformance>(this);
os << normal->getProtocol()->getName()
<< " module " << normal->getDeclContext()->getParentModule()->getName();
<< " module " << normal->getDeclContext()->getParentModule()->getRealName();
break;
}
case ProtocolConformanceKind::Self: {
auto self = cast<SelfProtocolConformance>(this);
os << self->getProtocol()->getName()
<< " module " << self->getDeclContext()->getParentModule()->getName();
<< " module " << self->getDeclContext()->getParentModule()->getRealName();
break;
}
case ProtocolConformanceKind::Specialized: {
Expand Down
6 changes: 4 additions & 2 deletions lib/AST/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1499,9 +1499,11 @@ ModuleDecl::ReverseFullNameIterator::ReverseFullNameIterator(

StringRef ModuleDecl::ReverseFullNameIterator::operator*() const {
assert(current && "all name components exhausted");

// Return the module's real (binary) name, which can be different from
// the name if module aliasing was used (-module-alias flag). The real
// name is used for serialization and loading.
if (auto *swiftModule = current.dyn_cast<const ModuleDecl *>())
return swiftModule->getName().str();
return swiftModule->getRealName().str();

auto *clangModule =
static_cast<const clang::Module *>(current.get<const void *>());
Expand Down
3 changes: 2 additions & 1 deletion lib/ClangImporter/ClangImporter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3582,7 +3582,8 @@ StringRef ClangModuleUnit::getExportedModuleName() const {
if (clangModule && !clangModule->ExportAsModule.empty())
return clangModule->ExportAsModule;

return getParentModule()->getName().str();
// Return module real name (see FileUnit::getExportedModuleName)
return getParentModule()->getRealName().str();
}

ModuleDecl *ClangModuleUnit::getOverlayModule() const {
Expand Down
10 changes: 8 additions & 2 deletions lib/Serialization/Serialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -751,8 +751,10 @@ IdentifierID Serializer::addModuleRef(const ModuleDecl *module) {
return CURRENT_MODULE_ID;
if (module == this->M->getASTContext().TheBuiltinModule)
return BUILTIN_MODULE_ID;
// Use module 'real name', which can be different from 'name'
// in case module aliasing was used (-module-alias flag)
auto moduleName =
module->getASTContext().getIdentifier(module->getName().str());
module->getASTContext().getIdentifier(module->getRealName().str());
return addDeclBaseNameRef(moduleName);
}

Expand Down Expand Up @@ -961,7 +963,9 @@ void Serializer::writeHeader(const SerializationOptions &options) {
control_block::RevisionLayout Revision(Out);
control_block::IsOSSALayout IsOSSA(Out);

ModuleName.emit(ScratchRecord, M->getName().str());
// Write module 'real name', which can be different from 'name'
// in case module aliasing is used (-module-alias flag)
ModuleName.emit(ScratchRecord, M->getRealName().str());

SmallString<32> versionStringBuf;
llvm::raw_svector_ostream versionString(versionStringBuf);
Expand Down Expand Up @@ -1101,6 +1105,8 @@ void Serializer::writeHeader(const SerializationOptions &options) {
static void flattenImportPath(const ImportedModule &import,
SmallVectorImpl<char> &out) {
llvm::raw_svector_ostream outStream(out);
// This will write the module 'real name', which can be different
// from the 'name' in case module aliasing was used (see `-module-alias`)
import.importedModule->getReverseFullModuleName().printForward(
outStream, StringRef("\0", 1));

Expand Down
30 changes: 0 additions & 30 deletions test/Frontend/load-module-with-alias-chained-simple.swift

This file was deleted.

70 changes: 0 additions & 70 deletions test/Frontend/load-module-with-alias-chained1.swift

This file was deleted.

61 changes: 0 additions & 61 deletions test/Frontend/load-module-with-alias-chained2.swift

This file was deleted.

27 changes: 0 additions & 27 deletions test/Frontend/load-module-with-alias-for-interface.swift

This file was deleted.

Loading