Skip to content

Sema: Only diagnose retroactive conformances once #71596

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1641,6 +1641,7 @@ static void diagnoseRetroactiveConformances(
// At this point, we know we're extending a type declared outside this module.
// We better only be conforming it to protocols declared within this module.
llvm::SmallSetVector<ProtocolDecl *, 8> externalProtocols;
llvm::SmallSet<ProtocolDecl *, 8> protocolsWithRetroactiveAttr;
for (const InheritedEntry &entry : ext->getInherited().getEntries()) {
if (entry.getType().isNull() || !entry.getTypeRepr()) {
continue;
Expand All @@ -1665,7 +1666,6 @@ static void diagnoseRetroactiveConformances(
auto conformanceRef = ext->getParentModule()->lookupConformance(
extendedType, decl);
if (!conformanceRef.isConcrete()) {

return TypeWalker::Action::Continue;
}
auto conformance = conformanceRef.getConcrete();
Expand Down Expand Up @@ -1701,6 +1701,10 @@ static void diagnoseRetroactiveConformances(

// If it's marked @retroactive, no need to warn.
if (entry.isRetroactive()) {
// Note that we encountered this protocol through a conformance marked
// @retroactive in case multiple clauses cause the protocol to be
// inherited.
protocolsWithRetroactiveAttr.insert(decl);
return TypeWalker::Action::Continue;
}

Expand All @@ -1712,6 +1716,11 @@ static void diagnoseRetroactiveConformances(
});
}

// Remove protocols that are reachable through a @retroactive conformance.
for (auto *proto : protocolsWithRetroactiveAttr) {
externalProtocols.remove(proto);
}

// If we didn't find any violations, we're done.
if (externalProtocols.empty()) {
return;
Expand Down
20 changes: 20 additions & 0 deletions test/Sema/extension_retroactive_conformances.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,16 @@
// Define a couple protocols with no requirements that're easy to conform to
public protocol SampleProtocol1 {}
public protocol SampleProtocol2 {}
public protocol SampleProtocol1a: SampleProtocol1 {}
public protocol SampleProtocol1b: SampleProtocol1 {}

public struct Sample1 {}
public struct Sample2 {}
public struct Sample2a {}
public struct Sample2b {}
public struct Sample2c {}
public struct Sample2d {}
public struct Sample2e {}
public struct Sample3 {}
public struct Sample4 {}
public struct Sample5 {}
Expand Down Expand Up @@ -36,6 +43,19 @@ extension Sample2: InheritsSampleProtocol {} // expected-warning {{extension dec

extension SampleAlreadyConforms: InheritsSampleProtocol {} // ok, SampleAlreadyConforms already conforms in the source module

extension Sample2a: @retroactive SampleProtocol1, InheritsSampleProtocol {} // ok, the concrete conformance to SampleProtocol1 has been declared retroactive

extension Sample2b: InheritsSampleProtocol, @retroactive SampleProtocol1 {} // ok, same as above but in the opposite order

// FIXME: It would be better to only suggest marking SampleProtocol1a @retroactive here
extension Sample2c: SampleProtocol1a {} // expected-warning {{extension declares a conformance of imported type 'Sample2c' to imported protocols 'SampleProtocol1a', 'SampleProtocol1'}}
// expected-note @-1 {{add '@retroactive' to silence this warning}} {{21-37=@retroactive SampleProtocol1a}} {{1-1=extension Sample2c: @retroactive SampleProtocol1 {\}\n}}

extension Sample2d: @retroactive SampleProtocol1a {} // ok, retroactive conformance to SampleProtocol1a covers conformance to SampleProtocol1

extension Sample2e: SampleProtocol1a, @retroactive SampleProtocol1b {} // expected-warning {{extension declares a conformance of imported type 'Sample2e' to imported protocol 'SampleProtocol1a'}}
// expected-note @-1 {{add '@retroactive' to silence this warning}} {{21-37=@retroactive SampleProtocol1a}}

extension Sample3: NestedInheritsSampleProtocol {} // expected-warning {{extension declares a conformance of imported type 'Sample3' to imported protocol 'SampleProtocol1'}}
// expected-note @-1 {{add '@retroactive' to silence this warning}} {{1-1=extension Sample3: @retroactive SampleProtocol1 {\}\n}}

Expand Down