-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[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
Changes from all commits
96fb3d6
adf33e0
1d0c84c
4b3ee94
e1dfdba
e3d5ae0
7a3a032
9143bd4
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 |
---|---|---|
|
@@ -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); | ||
|
@@ -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(); | ||
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. 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 (It’s probably okay to merge this as-is for now and come back to it later.) 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. Sounds good; will create a separate PR to address that. |
||
if (Options.UseExportedModuleNames && !ExportedModuleName.empty()) { | ||
Name = Mod->getASTContext().getIdentifier(ExportedModuleName); | ||
} | ||
|
@@ -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) { | ||
|
@@ -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; | ||
|
@@ -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 << ">"; | ||
} | ||
|
||
|
@@ -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: { | ||
|
This file was deleted.
This file was deleted.
This file was deleted.
This file was deleted.
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.
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.