Skip to content

Commit 8846f4e

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 bd3c15e commit 8846f4e

File tree

5 files changed

+78
-3
lines changed

5 files changed

+78
-3
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(

test/Inputs/clang-importer-sdk/swift-modules/ObjectiveC.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ public func ~=(x: NSObject, y: NSObject) -> Bool {
8282
return true
8383
}
8484

85-
extension NSObject : @retroactive Equatable, @retroactive Hashable {
85+
extension NSObject : Equatable, Hashable {
8686
public static func == (lhs: NSObject, rhs: NSObject) -> Bool {
8787
return lhs.isEqual(rhs)
8888
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
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 -import-underlying-module -I %t
5+
// RUN: %target-swift-frontend -typecheck %t/Library.swift -module-name Library -verify -swift-version 5 -DEXPLICIT_IMPORT -I %t
6+
7+
// REQUIRES: objc_interop
8+
9+
//--- module.modulemap
10+
11+
module Library {
12+
header "Library.h"
13+
}
14+
15+
//--- Library.h
16+
17+
@import Foundation;
18+
19+
@interface UnderlyingLibraryClass : NSObject
20+
@end
21+
22+
@protocol UnderlyingLibraryProtocol
23+
@end
24+
25+
//--- OtherLibrary.swift
26+
27+
public class OtherLibraryClass {}
28+
public protocol OtherLibraryProtocol {}
29+
30+
//--- Library.swift
31+
32+
#if EXPLICIT_IMPORT
33+
@_exported import Library
34+
#endif
35+
import OtherLibrary
36+
37+
public class LibraryClass {}
38+
public protocol LibraryProtocol {}
39+
40+
extension LibraryClass: UnderlyingLibraryProtocol {}
41+
extension LibraryClass: LibraryProtocol {}
42+
extension LibraryClass: OtherLibraryProtocol {}
43+
extension UnderlyingLibraryClass: OtherLibraryProtocol {}
44+
extension UnderlyingLibraryClass: LibraryProtocol {}
45+
extension UnderlyingLibraryClass: UnderlyingLibraryProtocol {}
46+
extension OtherLibraryClass: UnderlyingLibraryProtocol {}
47+
extension OtherLibraryClass: LibraryProtocol {}
48+
extension OtherLibraryClass: OtherLibraryProtocol {} // expected-warning {{extension declares a conformance of imported type 'OtherLibraryClass' to imported protocol 'OtherLibraryProtocol'}}
49+
// expected-note @-1 {{add '@retroactive' to silence this warning}} {{30-50=@retroactive OtherLibraryProtocol}}

0 commit comments

Comments
 (0)