Skip to content

Diagnose unsound uses of same-type-to-Self requirements with non-final classes #9830

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 1 commit into from
May 22, 2017
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
24 changes: 16 additions & 8 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -3418,46 +3418,54 @@ class ClassDecl : public NominalTypeDecl {
struct SelfReferenceKind {
bool result;
bool parameter;
bool requirement;
bool other;

/// The type does not refer to 'Self' at all.
static SelfReferenceKind None() {
return SelfReferenceKind(false, false, false);
return SelfReferenceKind(false, false, false, false);
}

/// The type refers to 'Self', but only as the result type of a method.
static SelfReferenceKind Result() {
return SelfReferenceKind(true, false, false);
return SelfReferenceKind(true, false, false, false);
}

/// The type refers to 'Self', but only as the parameter type of a method.
static SelfReferenceKind Parameter() {
return SelfReferenceKind(false, true, false);
return SelfReferenceKind(false, true, false, false);
}

/// The type refers to 'Self' within a same-type requiement.
static SelfReferenceKind Requirement() {
return SelfReferenceKind(false, false, true, false);
}

/// The type refers to 'Self' in a position that is invariant.
static SelfReferenceKind Other() {
return SelfReferenceKind(false, false, true);
return SelfReferenceKind(false, false, false, true);
}

SelfReferenceKind flip() const {
return SelfReferenceKind(parameter, result, other);
return SelfReferenceKind(parameter, result, requirement, other);
}

SelfReferenceKind operator|=(SelfReferenceKind kind) {
result |= kind.result;
requirement |= kind.requirement;
parameter |= kind.parameter;
other |= kind.other;
return *this;
}

operator bool() const {
return result || parameter || other;
return result || parameter || requirement || other;
}

private:
SelfReferenceKind(bool result, bool parameter, bool other)
: result(result), parameter(parameter), other(other) { }
SelfReferenceKind(bool result, bool parameter, bool requirement, bool other)
: result(result), parameter(parameter), requirement(requirement),
other(other) { }
};

/// ProtocolDecl - A declaration of a protocol, for example:
Expand Down
13 changes: 13 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,19 @@ ERROR(witness_self_non_subtype,none,
"(%2) because it uses 'Self' in a non-parameter, non-result type "
"position",
(Type, DeclName, Type))
ERROR(witness_self_same_type,none,
"%0 %1 in non-final class %2 cannot be used to satisfy requirement %3 %4"
" (in protocol %5) due to same-type requirement involving 'Self'",
(DescriptiveDeclKind, DeclName, Type, DescriptiveDeclKind,
DeclName, Type))
WARNING(witness_self_same_type_warn,none,
"%0 %1 in non-final class %2 cannot be used to satisfy requirement %3 %4"
" (in protocol %5) due to same-type requirement involving 'Self'",
(DescriptiveDeclKind, DeclName, Type, DescriptiveDeclKind,
DeclName, Type))
NOTE(witness_self_weaken_same_type,none,
"consider weakening the same-type requirement %0 == %1 to a superclass "
"requirement", (Type, Type))
ERROR(witness_requires_dynamic_self,none,
"method %0 in non-final class %1 must return `Self` to conform to "
"protocol %2",
Expand Down
38 changes: 36 additions & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3043,6 +3043,25 @@ findProtocolSelfReferences(const ProtocolDecl *proto, Type type,
return SelfReferenceKind::None();
}

/// Find Self references in a generic signature's same-type requirements.
static SelfReferenceKind
findProtocolSelfReferences(const ProtocolDecl *protocol,
GenericSignature *genericSig){
if (!genericSig) return SelfReferenceKind::None();

auto selfTy = protocol->getSelfInterfaceType();
for (const auto &req : genericSig->getRequirements()) {
if (req.getKind() != RequirementKind::SameType)
continue;

if (req.getFirstType()->isEqual(selfTy) ||
req.getSecondType()->isEqual(selfTy))
return SelfReferenceKind::Requirement();
}

return SelfReferenceKind::None();
}

/// Find Self references within the given requirement.
SelfReferenceKind
ProtocolDecl::findProtocolSelfReferences(const ValueDecl *value,
Expand All @@ -3062,7 +3081,7 @@ ProtocolDecl::findProtocolSelfReferences(const ValueDecl *value,
if (type->hasError())
return SelfReferenceKind::None();

if (isa<AbstractFunctionDecl>(value)) {
if (auto func = dyn_cast<AbstractFunctionDecl>(value)) {
// Skip the 'self' parameter.
type = type->castTo<AnyFunctionType>()->getResult();

Expand All @@ -3076,9 +3095,24 @@ ProtocolDecl::findProtocolSelfReferences(const ValueDecl *value,
return SelfReferenceKind::Other();
}

// Check the requirements of a generic function.
if (func->isGeneric()) {
if (auto result =
::findProtocolSelfReferences(this, func->getGenericSignature()))
return result;
}

return ::findProtocolSelfReferences(this, type,
skipAssocTypes);
} else if (isa<SubscriptDecl>(value)) {
} else if (auto subscript = dyn_cast<SubscriptDecl>(value)) {
// Check the requirements of a generic subscript.
if (subscript->isGeneric()) {
if (auto result =
::findProtocolSelfReferences(this,
subscript->getGenericSignature()))
return result;
}

return ::findProtocolSelfReferences(this, type,
skipAssocTypes);
} else {
Expand Down
69 changes: 69 additions & 0 deletions lib/Sema/TypeCheckProtocol.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2794,6 +2794,50 @@ witnessHasImplementsAttrForRequirement(ValueDecl *witness,
return false;
}

/// Determine the given witness has a same-type constraint constraining the
/// given 'Self' type, and return the
///
/// \returns None if there is no such constraint; a non-empty optional that
/// may have the \c RequirementRepr for the actual constraint.
static Optional<RequirementRepr *>
getAdopteeSelfSameTypeConstraint(ClassDecl *selfClass, ValueDecl *witness) {
auto genericSig =
witness->getInnermostDeclContext()->getGenericSignatureOfContext();
if (!genericSig) return None;

for (const auto &req : genericSig->getRequirements()) {
if (req.getKind() != RequirementKind::SameType)
continue;

if (req.getFirstType()->getAnyNominal() == selfClass ||
req.getSecondType()->getAnyNominal() == selfClass) {
// Try to find the requirement-as-written.
GenericParamList *genericParams = nullptr;

if (auto func = dyn_cast<AbstractFunctionDecl>(witness))
genericParams = func->getGenericParams();
else if (auto subscript = dyn_cast<SubscriptDecl>(witness))
genericParams = subscript->getGenericParams();
if (genericParams) {
for (auto &req : genericParams->getRequirements()) {
if (req.getKind() != RequirementReprKind::SameType)
continue;

if (req.getFirstType()->getAnyNominal() == selfClass ||
req.getSecondType()->getAnyNominal() == selfClass)
return &req;
}
}

// Form an optional(nullptr) to indicate that we don't have the
// requirement itself.
return nullptr;
}
}

return None;
}

ResolveWitnessResult
ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
assert(!isa<AssociatedTypeDecl>(requirement) && "Use resolveTypeWitnessVia*");
Expand Down Expand Up @@ -3076,6 +3120,31 @@ ConformanceChecker::resolveWitnessViaLookup(ValueDecl *requirement) {
conformance->getType());
});
}
} else if (selfKind.requirement) {
if (auto constraint = getAdopteeSelfSameTypeConstraint(classDecl,
witness)) {
// A "Self ==" constraint works incorrectly with subclasses. Complain.
auto proto = Conformance->getProtocol();
auto &diags = proto->getASTContext().Diags;
diags.diagnose(witness->getLoc(),
proto->getASTContext().LangOpts.isSwiftVersion3()
? diag::witness_self_same_type_warn
: diag::witness_self_same_type,
witness->getDescriptiveKind(),
witness->getFullName(),
Conformance->getType(),
requirement->getDescriptiveKind(),
requirement->getFullName(),
proto->getDeclaredType());

if (auto requirementRepr = *constraint) {
diags.diagnose(requirementRepr->getEqualLoc(),
diag::witness_self_weaken_same_type,
requirementRepr->getFirstType(),
requirementRepr->getSecondType())
.fixItReplace(requirementRepr->getEqualLoc(), ":");
}
}
}

// A non-final class can model a protocol requirement with a
Expand Down
22 changes: 22 additions & 0 deletions test/Compatibility/self_same_type.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %target-typecheck-verify-swift -swift-version 3

protocol P {
associatedtype T
}

protocol Q {
func foo<T: P>(_: T, _: T.T) where T.T == Self
}

class C1: Q {
func foo<T: P>(_: T, _: C1) where T.T == C1 {} // expected-warning{{instance method 'foo' in non-final class 'C1' cannot be used to satisfy requirement instance method 'foo' (in protocol 'Q') due to same-type requirement involving 'Self'}}}}
// expected-note@-1{{consider weakening the same-type requirement 'T.T' == 'C1' to a superclass requirement}}{{41-43=:}}
}

final class C2: Q {
func foo<T: P>(_: T, _: C2) where T.T == C2 {}
}

class C3: Q {
func foo<T: P>(_: T, _: C3) where T.T: C3 {}
}
22 changes: 22 additions & 0 deletions test/decl/protocol/conforms/self_same_type.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
// RUN: %target-typecheck-verify-swift -swift-version 4

protocol P {
associatedtype T
}

protocol Q {
func foo<T: P>(_: T, _: T.T) where T.T == Self
}

class C1: Q {
func foo<T: P>(_: T, _: C1) where T.T == C1 {} // expected-error{{instance method 'foo' in non-final class 'C1' cannot be used to satisfy requirement instance method 'foo' (in protocol 'Q') due to same-type requirement involving 'Self'}}}}
// expected-note@-1{{consider weakening the same-type requirement 'T.T' == 'C1' to a superclass requirement}}{{41-43=:}}
}

final class C2: Q {
func foo<T: P>(_: T, _: C2) where T.T == C2 {}
}

class C3: Q {
func foo<T: P>(_: T, _: C3) where T.T: C3 {}
}