swift
[SE-0364] Handle retroactive conformance for types and protocols from underlying modules
#71302
Merged
[SE-0364] Handle retroactive conformance for types and protocols from underlying modules #71302
@swift-ci please test
[SE-0364] Handle retroactive conformance for types and protocols fromβ¦
@swift-ci please smoke test
To be clear, this is making it so that overlays (where you use an explicit @_exported import MyModule) are treated the same as mixed-language targets (where you use -import-underlying-module or -import-objc-header), right?
| 693 | 696 | return isScriptMode() || hasMainDecl(); | |
| 694 | 697 | } | |
| 695 | 698 | ||
| 699 | ModuleDecl *getUnderlyingModuleIfOverlay() const override { | ||
| 700 | return ImportedUnderlyingModule; | ||
| 701 | } | ||
| 702 | |||
| 703 | const clang::Module *getUnderlyingClangModule() const override { |
beccadax1 year agoπ 1I was a little confused at first, but it looks like these are base class methods used by the existing diagnostic, so overriding them changes its behavior. Nice and clean. π
tshortli1 year agoRight, there are existing ModuleDecl methods that call these methods that I've overridden, and I think it was an oversight that those ModuleDecl methods were not functional on the module instance representing the compiled module for the job.
| 2566 | 2566 | SourceFile::setImports(ArrayRef<AttributedImport<ImportedModule>> imports) { | |
| 2567 | 2567 | assert(!Imports && "Already computed imports"); | |
| 2568 | 2568 | Imports = getASTContext().AllocateCopy(imports); | |
| 2569 | |||
| 2570 | // Find and cache the import of the underlying module, if present. | ||
| 2571 | auto parentModuleName = getParentModule()->getName(); | ||
| 2572 | for (auto import : imports) { | ||
| 2573 | if (!import.options.contains(ImportFlags::Exported)) | ||
| 2574 | continue; | ||
| 2575 | |||
| 2576 | auto importedModule = import.module.importedModule; | ||
| 2577 | if (importedModule->getName() == parentModuleName && | ||
| 2578 | importedModule->findUnderlyingClangModule()) { | ||
| 2579 | ImportedUnderlyingModule = import.module.importedModule; | ||
| 2580 | break; | ||
| 2581 | } | ||
| 2582 | } |
beccadax1 year agoRather than looping over the imports this way, you could modify ImportResolver::getModule() to save off the underlying clang module in a new member variable of ImportResolver (there's a separate code path to look it upβlook for the use of getClangModuleLoader()) and then have performImportResolution() set that on the SourceFile immediately before/after calling setImports(). Would that be a cleaner solution?
tshortli1 year agoπ 1That does seem like a cleaner solution because it keeps the logic for identifying the underlying clang module more centralized. In the interest of time I think I'm going to land this solution first, but I will follow it up with what you've suggested - thanks!
tshortli1 year agoπ 1Implemented here: #71330
To be clear, this is making it so that overlays (where you use an explicit @_exported import MyModule) are treated the same as mixed-language targets (where you use -import-underlying-module or -import-objc-header), right?
It is intended to work for both of them, yes. But in case this was not clear, the diagnostics were not behaving correctly in either case.
@swift-ci please smoke test Linux
tshortli
deleted the suppress-retroactive-warning-for-types-from-underlying-module branch 1 year ago
Login to write a write a comment.
SE-0364 was implemented to discourage "retroactive" conformances that might conflict with conformances that could be introduced by other modules in the future. These diagnostics should not apply to conformances that involve types and protocols imported from the underlying clang module of a Swift module since the two modules are assumed to be developed in tandem by the same owners, despite technically being separate modules from the perspective of the compiler.
The diagnostics implemented in #36068 were designed to take underlying clang modules into account. However, the implementation assumed that
ModuleDecl::getUnderlyingModuleIfOverlay()would behave as expected when called on the Swift module being compiled. Unfortunately, it would always returnnullptrand thus conformances involving the underlying clang module are being diagnosed unexpectedly.The fix is to make
ModuleDecl::getUnderlyingModuleIfOverlay()behave as expected when it is made up ofSourceFiles.Resolves rdar://121478556