Skip to content

Commit 939de4f

Browse files
committed
Extend candidate missing conformance checking to other types of requirements so that we check superclass and same type requirements in the same way.
1 parent 002c04e commit 939de4f

File tree

8 files changed

+101
-46
lines changed

8 files changed

+101
-46
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1814,8 +1814,9 @@ NOTE(protocol_witness_kind_conflict,none,
18141814
"a subscript}0", (RequirementKind))
18151815
NOTE(protocol_witness_type_conflict,none,
18161816
"candidate has non-matching type %0%1", (Type, StringRef))
1817-
NOTE(protocol_witness_missing_conformance,none,
1818-
"candidate would match if %0 conformed to %1", (Type, Type))
1817+
NOTE(protocol_witness_missing_requirement,none,
1818+
"candidate would match if %0 %select{conformed to|subclassed|"
1819+
"was the same type as}2 %1", (Type, Type, unsigned))
18191820

18201821
NOTE(protocol_witness_optionality_conflict,none,
18211822
"candidate %select{type has|result type has|parameter type has|"

lib/Sema/CSFix.h

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,11 @@ enum class FixKind : uint8_t {
7575
/// Skip same-type generic requirement constraint,
7676
/// and assume that types are equal.
7777
SkipSameTypeRequirement,
78+
79+
/// Skip superclass generic requirement constraint,
80+
/// and assume that types are related.
81+
SkipSuperclassRequirement,
82+
7883
};
7984

8085
class ConstraintFix {
@@ -311,6 +316,9 @@ class SkipSameTypeRequirement final : public ConstraintFix {
311316

312317
bool diagnose(Expr *root, bool asNote = false) const override;
313318

319+
Type lhsType() { return LHS; }
320+
Type rhsType() { return RHS; }
321+
314322
static SkipSameTypeRequirement *create(ConstraintSystem &cs, Type lhs,
315323
Type rhs, ConstraintLocator *locator);
316324
};
@@ -322,8 +330,8 @@ class SkipSuperclassRequirement final : public ConstraintFix {
322330

323331
SkipSuperclassRequirement(ConstraintSystem &cs, Type lhs, Type rhs,
324332
ConstraintLocator *locator)
325-
: ConstraintFix(cs, FixKind::SkipSameTypeRequirement, locator), LHS(lhs),
326-
RHS(rhs) {}
333+
: ConstraintFix(cs, FixKind::SkipSuperclassRequirement, locator),
334+
LHS(lhs), RHS(rhs) {}
327335

328336
public:
329337
std::string getName() const override {
@@ -332,6 +340,9 @@ class SkipSuperclassRequirement final : public ConstraintFix {
332340

333341
bool diagnose(Expr *root, bool asNote = false) const override;
334342

343+
Type subclassType() { return LHS; }
344+
Type superclassType() { return RHS; }
345+
335346
static SkipSuperclassRequirement *
336347
create(ConstraintSystem &cs, Type lhs, Type rhs, ConstraintLocator *locator);
337348
};

lib/Sema/CSSimplify.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5038,7 +5038,8 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(
50385038
return result;
50395039
}
50405040

5041-
case FixKind::SkipSameTypeRequirement: {
5041+
case FixKind::SkipSameTypeRequirement:
5042+
case FixKind::SkipSuperclassRequirement: {
50425043
return recordFix(fix) ? SolutionKind::Error : SolutionKind::Solved;
50435044
}
50445045

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 48 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -845,26 +845,58 @@ swift::matchWitness(TypeChecker &tc,
845845

846846
// If the types would match but for some other missing conformance, find and
847847
// call that out.
848-
if (solution && conformance && solution->Fixes.size() == 1 &&
849-
solution->Fixes.front()->getKind() == FixKind::AddConformance) {
850-
auto fix = (MissingConformance *)solution->Fixes.front();
851-
auto type = fix->getNonConformingType();
852-
auto protocolType = fix->getProtocol()->getDeclaredType();
848+
if (solution && conformance && solution->Fixes.size() == 1) {
849+
auto fix = solution->Fixes.front();
850+
Type type, missingType;
851+
RequirementKind requirementKind;
852+
switch (fix->getKind()) {
853+
case FixKind::AddConformance: {
854+
auto missingConform = (MissingConformance *)fix;
855+
requirementKind = RequirementKind::Conformance;
856+
type = missingConform->getNonConformingType();
857+
missingType = missingConform->getProtocol()->getDeclaredType();
858+
break;
859+
}
860+
case FixKind::SkipSameTypeRequirement: {
861+
requirementKind = RequirementKind::SameType;
862+
auto requirementFix = (SkipSameTypeRequirement *)fix;
863+
type = requirementFix->lhsType();
864+
missingType = requirementFix->rhsType();
865+
break;
866+
}
867+
case FixKind::SkipSuperclassRequirement: {
868+
requirementKind = RequirementKind::Superclass;
869+
auto requirementFix = (SkipSuperclassRequirement *)fix;
870+
type = requirementFix->subclassType();
871+
missingType = requirementFix->superclassType();
872+
break;
873+
}
874+
default:
875+
return RequirementMatch(witness, MatchKind::TypeConflict, witnessType);
876+
}
877+
878+
auto missingRequirementMatch = [&](Type type) -> RequirementMatch {
879+
Requirement requirement(requirementKind, type, missingType);
880+
return RequirementMatch(witness, MatchKind::MissingRequirement,
881+
requirement);
882+
};
883+
853884
if (auto memberTy = type->getAs<DependentMemberType>())
854-
return RequirementMatch(witness, MatchKind::MissingConformance, type,
855-
protocolType);
885+
return missingRequirementMatch(type);
856886

857887
type = type->mapTypeOutOfContext();
858888
if (auto typeParamTy = type->getAs<GenericTypeParamType>())
859889
if (auto env = conformance->getGenericEnvironment())
860890
if (auto assocType = env->mapTypeIntoContext(typeParamTy))
861-
return RequirementMatch(witness, MatchKind::MissingConformance,
862-
assocType, protocolType);
863-
if (type->isEqual(conformance->getType())) {
891+
return missingRequirementMatch(assocType);
892+
893+
auto reqSubMap = reqEnvironment.getRequirementToSyntheticMap();
894+
Type selfTy = proto->getSelfInterfaceType().subst(reqSubMap);
895+
if (type->isEqual(selfTy) /*type->isEqual(conformance->getType())*/) {
896+
type = conformance->getType();
864897
if (auto agt = type->getAs<AnyGenericType>())
865898
type = agt->getDecl()->getDeclaredInterfaceType();
866-
return RequirementMatch(witness, MatchKind::MissingConformance, type,
867-
protocolType);
899+
return missingRequirementMatch(type);
868900
}
869901
}
870902

@@ -2051,9 +2083,10 @@ diagnoseMatch(ModuleDecl *module, NormalProtocolConformance *conformance,
20512083
break;
20522084
}
20532085

2054-
case MatchKind::MissingConformance:
2055-
diags.diagnose(match.Witness, diag::protocol_witness_missing_conformance,
2056-
match.WitnessType, match.SecondType);
2086+
case MatchKind::MissingRequirement:
2087+
diags.diagnose(match.Witness, diag::protocol_witness_missing_requirement,
2088+
match.WitnessType, match.MissingRequirement->getSecondType(),
2089+
(unsigned)match.MissingRequirement->getKind());
20572090
break;
20582091

20592092
case MatchKind::ThrowsConflict:

lib/Sema/TypeCheckProtocol.h

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -168,8 +168,8 @@ enum class MatchKind : uint8_t {
168168
/// \brief The types conflict.
169169
TypeConflict,
170170

171-
/// \brief The witness would match with an additional conformance.
172-
MissingConformance,
171+
/// \brief The witness would match if an additional requirement were met.
172+
MissingRequirement,
173173

174174
/// The witness throws, but the requirement does not.
175175
ThrowsConflict,
@@ -355,17 +355,15 @@ struct RequirementMatch {
355355
"Should (or should not) have witness type");
356356
}
357357

358-
RequirementMatch(ValueDecl *witness, MatchKind kind,
359-
Type witnessType, Type secondType,
358+
RequirementMatch(ValueDecl *witness, MatchKind kind, Requirement requirement,
360359
Optional<RequirementEnvironment> env = None,
361360
ArrayRef<OptionalAdjustment> optionalAdjustments = {})
362-
: Witness(witness), Kind(kind), WitnessType(witnessType),
363-
SecondType(secondType), ReqEnv(std::move(env)),
364-
OptionalAdjustments(optionalAdjustments.begin(),
365-
optionalAdjustments.end())
366-
{
367-
assert(hasWitnessType() && hasSecondType() &&
368-
"Should have witness type and second type");
361+
: Witness(witness), Kind(kind), WitnessType(requirement.getFirstType()),
362+
MissingRequirement(requirement), ReqEnv(std::move(env)),
363+
OptionalAdjustments(optionalAdjustments.begin(),
364+
optionalAdjustments.end()) {
365+
assert(hasWitnessType() && hasRequirement() &&
366+
"Should have witness type and requirement");
369367
}
370368

371369
/// \brief The witness that matches the (implied) requirement.
@@ -377,8 +375,8 @@ struct RequirementMatch {
377375
/// \brief The type of the witness when it is referenced.
378376
Type WitnessType;
379377

380-
/// \brief Explanatory second type.
381-
Type SecondType;
378+
/// \brief Requirement not met.
379+
Optional<Requirement> MissingRequirement;
382380

383381
/// \brief The requirement environment to use for the witness thunk.
384382
Optional<RequirementEnvironment> ReqEnv;
@@ -401,7 +399,7 @@ struct RequirementMatch {
401399
case MatchKind::WitnessInvalid:
402400
case MatchKind::KindConflict:
403401
case MatchKind::TypeConflict:
404-
case MatchKind::MissingConformance:
402+
case MatchKind::MissingRequirement:
405403
case MatchKind::StaticNonStaticConflict:
406404
case MatchKind::SettableConflict:
407405
case MatchKind::PrefixNonPrefixConflict:
@@ -424,7 +422,7 @@ struct RequirementMatch {
424422
case MatchKind::ExactMatch:
425423
case MatchKind::RenamedMatch:
426424
case MatchKind::TypeConflict:
427-
case MatchKind::MissingConformance:
425+
case MatchKind::MissingRequirement:
428426
case MatchKind::OptionalityConflict:
429427
return true;
430428

@@ -446,10 +444,8 @@ struct RequirementMatch {
446444
llvm_unreachable("Unhandled MatchKind in switch.");
447445
}
448446

449-
/// \brief Determine whether this requirement match has a second type.
450-
bool hasSecondType() {
451-
return Kind == MatchKind::MissingConformance;
452-
}
447+
/// \brief Determine whether this requirement match has a requirement.
448+
bool hasRequirement() { return Kind == MatchKind::MissingRequirement; }
453449

454450
swift::Witness getWitness(ASTContext &ctx) const;
455451
};

test/decl/protocol/conforms/self.swift

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,7 @@ class SeriousClass {}
6161

6262
extension HasDefault where Self : SeriousClass {
6363
func foo() {}
64-
// expected-note@-1 {{candidate has non-matching type '<Self> () -> ()'}}
65-
66-
// FIXME: the above diangostic is from trying to check conformance for
67-
// 'SillyClass' and not 'SeriousClass'. Evidently name lookup finds members
68-
// from all constrained extensions, and then if any don't have a matching
69-
// generic signature, diagnostics doesn't really know what to do about it.
64+
// expected-note@-1 {{candidate would match if 'SillyClass' subclassed 'SeriousClass'}}
7065
}
7166

7267
extension SeriousClass : HasDefault {}

test/decl/protocol/req/missing_conformance.swift

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,21 @@ protocol P8 {
8080
struct B : P8 { // expected-error {{type 'B' does not conform to protocol 'P8'}}
8181
func g(t: (Int, String)) {} // expected-note {{candidate can not infer 'T' = '(Int, String)' because '(Int, String)' is not a nominal type and so can't conform to 'P7'}}
8282
}
83+
84+
protocol P9 {
85+
func foo() // expected-note {{protocol requires function 'foo()' with type '() -> ()'; do you want to add a stub?}}
86+
}
87+
class C2 {}
88+
extension P9 where Self : C2 {
89+
func foo() {} // expected-note {{candidate would match if 'C3' subclassed 'C2'}}
90+
}
91+
class C3 : P9 {} // expected-error {{type 'C3' does not conform to protocol 'P9'}}
92+
93+
protocol P10 {
94+
associatedtype A
95+
func bar() // expected-note {{protocol requires function 'bar()' with type '() -> ()'; do you want to add a stub?}}
96+
}
97+
extension P10 where A == Int {
98+
func bar() {} // expected-note {{candidate would match if 'A' was the same type as 'Int'}}
99+
}
100+
struct S2<A> : P10 {} // expected-error {{type 'S2<A>' does not conform to protocol 'P10'}}

test/decl/protocol/req/unsatisfiable.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ protocol P {
1010

1111
extension P {
1212
func f<T: P>(_: T) where T.A == Self.A, T.A == Self.B { }
13-
// expected-note@-1 {{candidate has non-matching type '<Self, T> (T) -> ()'}}
13+
// expected-note@-1 {{candidate would match if 'X' was the same type as 'X.B' (aka 'Int')}}
1414
}
1515

1616
struct X : P { // expected-error {{type 'X' does not conform to protocol 'P'}}

0 commit comments

Comments
 (0)