[Module aliasing] Serialize SIL and binaries with module real names #39705
Conversation
Resolves rdar://83632529
elsh
requested review from
xedin,
tomerd,
xymus,
beccadax,
DougGregor and
CodaFi
Rename test
|
@swift-ci smoke test |
There was a problem hiding this comment.
This looks good functionally, but I'd like to see more testing.
(Many of the comments I put in one particular test file are also applicable to some of the others—in particular, the request for negative tests and module interface tests. Please use your best judgment here. 🙂)
| /// substituting another name instead. | ||
| virtual StringRef getExportedModuleName() const { | ||
| return getParentModule()->getName().str(); | ||
| return getParentModule()->getRealName().str(); |
There was a problem hiding this comment.
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.
elsh
force-pushed
the
es-module-alias-serialize
branch
from
ac0eb8e to
e1dfdba
Compare
elsh
force-pushed
the
es-module-alias-serialize
branch
from
ef54b76 to
7a3a032
Compare
|
@swift-ci smoke test |
|
@swift-ci smoke test |
| 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.
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.)
There was a problem hiding this comment.
Sounds good; will create a separate PR to address that.
elsh
changed the title
Use module real names for containing modules and imported modules when serializing.
Resolves rdar://83632529