Skip to content

Commit ebfcc68

Browse files
committed
[SE-0364] Handle retroactive conformance for types and protocols from underlying modules.
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 swiftlang#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 return `nullptr` and 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 of `SourceFile`s. Resolves rdar://121478556
1 parent 7b6f45c commit ebfcc68

File tree

4 files changed

+74
-2
lines changed

4 files changed

+74
-2
lines changed

include/swift/AST/SourceFile.h

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,9 @@ class SourceFile final : public FileUnit {
144144
/// This is \c None until it is filled in by the import resolution phase.
145145
llvm::Optional<ArrayRef<AttributedImport<ImportedModule>>> Imports;
146146

147+
/// The underlying clang module, if imported in this file.
148+
ModuleDecl *ImportedUnderlyingModule = nullptr;
149+
147150
/// Which imports have made use of @preconcurrency.
148151
llvm::SmallDenseSet<AttributedImport<ImportedModule>>
149152
PreconcurrencyImportsUsed;
@@ -693,6 +696,17 @@ class SourceFile final : public FileUnit {
693696
return isScriptMode() || hasMainDecl();
694697
}
695698

699+
ModuleDecl *getUnderlyingModuleIfOverlay() const override {
700+
return ImportedUnderlyingModule;
701+
}
702+
703+
const clang::Module *getUnderlyingClangModule() const override {
704+
if (!ImportedUnderlyingModule)
705+
return nullptr;
706+
707+
return ImportedUnderlyingModule->findUnderlyingClangModule();
708+
}
709+
696710
/// Get the root refinement context for the file. The root context may be
697711
/// null if the context hierarchy has not been built yet. Use
698712
/// TypeChecker::getOrBuildTypeRefinementContext() to get a built

lib/AST/Module.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2566,6 +2566,20 @@ void
25662566
SourceFile::setImports(ArrayRef<AttributedImport<ImportedModule>> imports) {
25672567
assert(!Imports && "Already computed imports");
25682568
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+
}
25692583
}
25702584

25712585
bool SourceFile::hasImportUsedPreconcurrency(

lib/AST/ModuleDependencies.cpp

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -438,8 +438,6 @@ swift::dependencies::checkImportNotTautological(const ImportPath::Module moduleP
438438
filename, modulePath.front().Item);
439439

440440
return false;
441-
442-
return false;
443441
}
444442

445443
void SwiftDependencyTracker::addCommonSearchPathDeps(
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: split-file %s %t
3+
// RUN: %target-swift-frontend -swift-version 5 %t/OtherLibrary.swift -emit-module -module-name OtherLibrary -o %t
4+
// RUN: %target-swift-frontend -typecheck %t/Library.swift -module-name Library -verify -swift-version 5 -I %t
5+
6+
// REQUIRES: objc_interop
7+
8+
//--- module.modulemap
9+
10+
module Library {
11+
header "Library.h"
12+
}
13+
14+
//--- Library.h
15+
16+
@import Foundation;
17+
18+
@interface UnderlyingLibraryClass : NSObject
19+
@end
20+
21+
@protocol UnderlyingLibraryProtocol
22+
@end
23+
24+
//--- OtherLibrary.swift
25+
26+
public class OtherLibraryClass {}
27+
public protocol OtherLibraryProtocol {}
28+
29+
//--- Library.swift
30+
31+
@_exported import Library
32+
import OtherLibrary
33+
34+
public class LibraryClass {}
35+
public protocol LibraryProtocol {}
36+
37+
extension LibraryClass: UnderlyingLibraryProtocol {}
38+
extension LibraryClass: LibraryProtocol {}
39+
extension LibraryClass: OtherLibraryProtocol {}
40+
extension UnderlyingLibraryClass: OtherLibraryProtocol {}
41+
extension UnderlyingLibraryClass: LibraryProtocol {}
42+
extension UnderlyingLibraryClass: UnderlyingLibraryProtocol {}
43+
extension OtherLibraryClass: UnderlyingLibraryProtocol {}
44+
extension OtherLibraryClass: LibraryProtocol {}
45+
extension OtherLibraryClass: OtherLibraryProtocol {} // expected-warning {{extension declares a conformance of imported type 'OtherLibraryClass' to imported protocol 'OtherLibraryProtocol'}}
46+
// expected-note @-1 {{add '@retroactive' to silence this warning}} {{30-50=@retroactive OtherLibraryProtocol}}

0 commit comments

Comments
 (0)