Skip to content

[Type checker] Downgrade some "redundant conformance" errors to warnings. #9160

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
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
7 changes: 7 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1542,6 +1542,13 @@ WARNING(witness_unavailable,none,

ERROR(redundant_conformance,none,
"redundant conformance of %0 to protocol %1", (Type, DeclName))
WARNING(redundant_conformance_adhoc,none,
"conformance of %0 to protocol %1 was already stated in "
"%select{the protocol's|the type's}2 module %3",
(Type, DeclName, bool, Identifier))
NOTE(redundant_conformance_witness_ignored,none,
"%0 %1 will not be used to satisfy the conformance to %2",
(DescriptiveDeclKind, DeclName, DeclName))

// "Near matches"
WARNING(optional_req_near_match,none,
Expand Down
54 changes: 50 additions & 4 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5884,10 +5884,56 @@ void TypeChecker::checkConformancesInContext(DeclContext *dc,
if (!existingDecl)
existingDecl = cast<ExtensionDecl>(diag.ExistingDC);

// Complain about redundant conformances.
diagnose(diag.Loc, diag::redundant_conformance,
dc->getDeclaredInterfaceType(),
diag.Protocol->getName());
// Complain about the redundant conformance.

// If we've redundantly stated a conformance for which the original
// conformance came from the module of the type or the module of the
// protocol, just warn; we'll pick up the original conformance.
auto existingModule = diag.ExistingDC->getParentModule();
auto extendedNominal =
diag.ExistingDC->getAsNominalTypeOrNominalTypeExtensionContext();
if (existingModule != dc->getParentModule() &&
(existingModule == extendedNominal->getParentModule() ||
existingModule == diag.Protocol->getParentModule())) {
// Warn about the conformance.
diagnose(diag.Loc, diag::redundant_conformance_adhoc,
dc->getDeclaredInterfaceType(),
diag.Protocol->getName(),
existingModule == extendedNominal->getParentModule(),
existingModule->getName());

// Complain about any declarations in this extension whose names match
// a requirement in that protocol.
SmallPtrSet<DeclName, 4> diagnosedNames;
for (auto decl : idc->getMembers()) {
if (decl->isImplicit())
continue;

auto value = dyn_cast<ValueDecl>(decl);
if (!value) continue;

if (!diagnosedNames.insert(value->getFullName()).second)
continue;

bool valueIsType = isa<TypeDecl>(value);
for (auto requirement
: diag.Protocol->lookupDirect(value->getFullName(),
/*ignoreNewExtensions=*/true)) {
auto requirementIsType = isa<TypeDecl>(requirement);
if (valueIsType != requirementIsType)
continue;

diagnose(value, diag::redundant_conformance_witness_ignored,
value->getDescriptiveKind(), value->getFullName(),
diag.Protocol->getFullName());
break;
}
}
} else {
diagnose(diag.Loc, diag::redundant_conformance,
dc->getDeclaredInterfaceType(),
diag.Protocol->getName());
}

// Special case: explain that 'RawRepresentable' conformance
// is implied for enums which already declare a raw type.
Expand Down
5 changes: 5 additions & 0 deletions lib/Serialization/Deserialization.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1128,6 +1128,11 @@ static void filterValues(Type expectedTy, ModuleDecl *expectedModule,

auto newEnd = std::remove_if(values.begin(), values.end(),
[=](ValueDecl *value) {
// Ignore anything that was parsed (vs. deserialized), because a serialized
// module cannot refer to it.
if (value->getDeclContext()->getParentSourceFile())
return true;

if (isType != isa<TypeDecl>(value))
return true;
if (!value->hasInterfaceType())
Expand Down
15 changes: 15 additions & 0 deletions test/Compatibility/string_collection.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// RUN: %target-typecheck-verify-swift -swift-version 3

extension String : Collection { // expected-warning{{conformance of 'String' to protocol 'Collection' was already stated in the type's module 'Swift'}}
subscript (i: Index) -> String { // expected-note{{subscript 'subscript' will not be used to satisfy the conformance to 'Collection'}}
get { return self }
set { }
}

var isEmpty: Bool { return false } // expected-note{{var 'isEmpty' will not be used to satisfy the conformance to 'Collection'}}

}

func testStringOps(s: String) {
_ = s.isEmpty
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,5 @@

import enum_with_raw_type

// expected-error@+1{{redundant conformance of 'Foo' to protocol 'RawRepresentable'}}
// expected-warning@+1{{conformance of 'Foo' to protocol 'RawRepresentable' was already stated in the type's module 'enum_with_raw_type'}}
extension Foo: RawRepresentable {}
14 changes: 14 additions & 0 deletions test/decl/protocol/conforms/Inputs/redundant_conformance_A.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
public protocol P1 {
associatedtype A

func f() -> A
}

public struct ConformsToP : P1 {
public func f() -> Int { return 0 }
}

public struct OtherConformsToP {
public func f() -> Int { return 0 }
}

14 changes: 14 additions & 0 deletions test/decl/protocol/conforms/Inputs/redundant_conformance_B.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import redundant_conformance_A

public protocol P2 {
associatedtype A

func f() -> A
}

extension OtherConformsToP: P1 {
}

extension ConformsToP: P2 {
public func f() -> Int { return 0 }
}
26 changes: 13 additions & 13 deletions test/decl/protocol/conforms/placement.swift
Original file line number Diff line number Diff line change
Expand Up @@ -189,31 +189,31 @@ enum MFSynthesizedEnum2 : Int { case none = 0 }
// ===========================================================================
// Tests with conformances in imported modules
// ===========================================================================
extension MMExplicit1 : MMP1 { } // expected-error{{redundant conformance of 'MMExplicit1' to protocol 'MMP1'}}
extension MMExplicit1 : MMP1 { } // expected-warning{{conformance of 'MMExplicit1' to protocol 'MMP1' was already stated in the type's module 'placement_module_A'}}

extension MMExplicit1 : MMP2a { } // expected-error{{redundant conformance of 'MMExplicit1' to protocol 'MMP2a'}}
extension MMExplicit1 : MMP3a { } // expected-error{{redundant conformance of 'MMExplicit1' to protocol 'MMP3a'}}
extension MMExplicit1 : MMP2a { } // expected-warning{{MMExplicit1' to protocol 'MMP2a' was already stated in the type's module 'placement_module_A}}
extension MMExplicit1 : MMP3a { } // expected-warning{{conformance of 'MMExplicit1' to protocol 'MMP3a' was already stated in the type's module 'placement_module_A'}}

extension MMExplicit1 : MMP3b { } // okay

extension MMSuper1 : MMP1 { } // expected-error{{redundant conformance of 'MMSuper1' to protocol 'MMP1'}}
extension MMSuper1 : MMP2a { } // expected-error{{redundant conformance of 'MMSuper1' to protocol 'MMP2a'}}
extension MMSuper1 : MMP1 { } // expected-warning{{conformance of 'MMSuper1' to protocol 'MMP1' was already stated in the type's module 'placement_module_A'}}
extension MMSuper1 : MMP2a { } // expected-warning{{conformance of 'MMSuper1' to protocol 'MMP2a' was already stated in the type's module 'placement_module_A'}}
extension MMSuper1 : MMP3b { } // okay

extension MMSub1 : AnyObject { } // expected-error{{redundant conformance of 'MMSub1' to protocol 'AnyObject'}}
extension MMSub1 : AnyObject { } // expected-warning{{conformance of 'MMSub1' to protocol 'AnyObject' was already stated in the type's module 'placement_module_A'}}

extension MMSub2 : MMP1 { } // expected-error{{redundant conformance of 'MMSub2' to protocol 'MMP1'}}
extension MMSub2 : MMP2a { } // expected-error{{redundant conformance of 'MMSub2' to protocol 'MMP2a'}}
extension MMSub2 : MMP1 { } // expected-warning{{conformance of 'MMSub2' to protocol 'MMP1' was already stated in the type's module 'placement_module_A'}}
extension MMSub2 : MMP2a { } // expected-warning{{conformance of 'MMSub2' to protocol 'MMP2a' was already stated in the type's module 'placement_module_A'}}
extension MMSub2 : MMP3b { } // okay

extension MMSub2 : MMAnyObjectRefinement { } // okay

extension MMSub3 : MMP1 { } // expected-error{{redundant conformance of 'MMSub3' to protocol 'MMP1'}}
extension MMSub3 : MMP2a { } // expected-error{{redundant conformance of 'MMSub3' to protocol 'MMP2a'}}
extension MMSub3 : MMP1 { } // expected-warning{{conformance of 'MMSub3' to protocol 'MMP1' was already stated in the type's module 'placement_module_A'}}
extension MMSub3 : MMP2a { } // expected-warning{{conformance of 'MMSub3' to protocol 'MMP2a' was already stated in the type's module 'placement_module_A'}}
extension MMSub3 : MMP3b { } // okay
extension MMSub3 : AnyObject { } // expected-error{{redundant conformance of 'MMSub3' to protocol 'AnyObject'}}
extension MMSub3 : AnyObject { } // expected-warning{{conformance of 'MMSub3' to protocol 'AnyObject' was already stated in the type's module 'placement_module_A'}}

extension MMSub4 : MMP1 { } // expected-error{{redundant conformance of 'MMSub4' to protocol 'MMP1'}}
extension MMSub4 : MMP2a { } // expected-error{{redundant conformance of 'MMSub4' to protocol 'MMP2a'}}
extension MMSub4 : MMP1 { } // expected-warning{{conformance of 'MMSub4' to protocol 'MMP1' was already stated in the type's module 'placement_module_B'}}
extension MMSub4 : MMP2a { } // expected-warning{{conformance of 'MMSub4' to protocol 'MMP2a' was already stated in the type's module 'placement_module_B'}}
extension MMSub4 : MMP3b { } // okay
extension MMSub4 : AnyObjectRefinement { } // okay
32 changes: 32 additions & 0 deletions test/decl/protocol/conforms/redundant_conformance.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
// RUN: rm -rf %t
// RUN: mkdir -p %t

// RUN: %target-swift-frontend -emit-module -o %t %S/Inputs/redundant_conformance_A.swift
// RUN: %target-swift-frontend -emit-module -o %t -I %t %S/Inputs/redundant_conformance_B.swift
// RUN: %target-typecheck-verify-swift -I %t %s

import redundant_conformance_A
import redundant_conformance_B

extension ConformsToP
: P1 { // expected-warning{{conformance of 'ConformsToP' to protocol 'P1' was already stated in the type's module 'redundant_conformance_A'}}
typealias A = Double // expected-note{{type alias 'A' will not be used to satisfy the conformance to 'P1'}}

func f() -> Double { return 0.0 } // expected-note{{instance method 'f()' will not be used to satisfy the conformance to 'P1'}}
// expected-note@-1{{found this candidate}}
}

extension ConformsToP
: P2 { // expected-warning{{conformance of 'ConformsToP' to protocol 'P2' was already stated in the protocol's module 'redundant_conformance_B'}}
}

extension OtherConformsToP : P1 { // expected-error{{redundant conformance of 'OtherConformsToP' to protocol 'P1'}}
func f() -> Int { return 0 }
}

func testConformsToP(cp1: ConformsToP, ocp1: OtherConformsToP) {
// Note:
let _ = cp1.f() // expected-error{{ambiguous use of 'f()'}}

let _ = ocp1.f() // okay: picks "our" OtherConformsToP.f()
}