Skip to content

Commit 8bf2823

Browse files
committed
[Sema] Be more resilient about unavailable conditional requirements in error cases.
When we're trying to diagnose something, it's a bad look if we crash instead. This changes the bad-associated-type recovery path to not require that the conditional requirements have been computed, and instead detects that as a possible error. Fixes https://bugs.swift.org/browse/SR-8033.
1 parent 8307f71 commit 8bf2823

File tree

5 files changed

+43
-12
lines changed

5 files changed

+43
-12
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,9 @@ ERROR(ambiguous_module_type,none,
647647
ERROR(use_nonmatching_operator,none,
648648
"%0 is not a %select{binary|prefix unary|postfix unary}1 operator",
649649
(DeclName, unsigned))
650+
ERROR(unsupported_recursion_in_associated_type_reference,none,
651+
"unsupported recursion for reference to associated type %0 of type %1",
652+
(DeclName, Type))
650653
ERROR(broken_associated_type_witness,none,
651654
"reference to invalid associated type %0 of type %1", (DeclName, Type))
652655

lib/Sema/TypeCheckProtocol.cpp

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3829,11 +3829,18 @@ Optional<ProtocolConformanceRef> TypeChecker::conformsToProtocol(
38293829
markConformanceUsed(*lookupResult, DC);
38303830
}
38313831

3832-
// If we have a concrete conformance with conditional requirements that
3832+
auto condReqs = lookupResult->getConditionalRequirementsIfAvailable();
3833+
// If we have a conditional requirements that
38333834
// we need to check, do so now.
3834-
if (lookupResult->isConcrete() &&
3835-
!lookupResult->getConditionalRequirements().empty() &&
3836-
!options.contains(ConformanceCheckFlags::SkipConditionalRequirements)) {
3835+
if (!condReqs) {
3836+
assert(
3837+
options.contains(
3838+
ConformanceCheckFlags::AllowUnavailableConditionalRequirements) &&
3839+
"unhandled recursion: missing conditional requirements when they're "
3840+
"required");
3841+
} else if (!condReqs->empty() &&
3842+
!options.contains(
3843+
ConformanceCheckFlags::SkipConditionalRequirements)) {
38373844
// Figure out the location of the conditional conformance.
38383845
auto conformanceDC = lookupResult->getConcrete()->getDeclContext();
38393846
SourceLoc noteLoc;
@@ -3846,7 +3853,7 @@ Optional<ProtocolConformanceRef> TypeChecker::conformsToProtocol(
38463853
checkGenericArguments(DC, ComplainLoc, noteLoc, T,
38473854
{ Type(lookupResult->getRequirement()
38483855
->getProtocolSelfType()) },
3849-
lookupResult->getConditionalRequirements(),
3856+
*condReqs,
38503857
[](SubstitutableType *dependentType) {
38513858
return Type(dependentType);
38523859
},

lib/Sema/TypeCheckType.cpp

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -665,11 +665,11 @@ static void maybeDiagnoseBadConformanceRef(TypeChecker &tc,
665665
AssociatedTypeDecl *assocType) {
666666
// If we weren't given a conformance, go look it up.
667667
ProtocolConformance *conformance = nullptr;
668-
if (auto conformanceRef =
669-
tc.conformsToProtocol(
668+
if (auto conformanceRef = tc.conformsToProtocol(
670669
parentTy, assocType->getProtocol(), dc,
671-
(ConformanceCheckFlags::InExpression|
672-
ConformanceCheckFlags::SuppressDependencyTracking))) {
670+
(ConformanceCheckFlags::InExpression |
671+
ConformanceCheckFlags::SuppressDependencyTracking |
672+
ConformanceCheckFlags::AllowUnavailableConditionalRequirements))) {
673673
if (conformanceRef->isConcrete())
674674
conformance = conformanceRef->getConcrete();
675675
}
@@ -679,9 +679,14 @@ static void maybeDiagnoseBadConformanceRef(TypeChecker &tc,
679679
if (tc.Context.Diags.hadAnyError())
680680
return;
681681

682-
tc.diagnose(loc, diag::broken_associated_type_witness,
683-
assocType->getFullName(), parentTy);
684-
};
682+
auto diagCode =
683+
(conformance && !conformance->getConditionalRequirementsIfAvailable())
684+
? diag::unsupported_recursion_in_associated_type_reference
685+
: diag::broken_associated_type_witness;
686+
687+
tc.diagnose(loc, diagCode, assocType->getFullName(), parentTy);
688+
}
689+
685690
/// \brief Returns a valid type or ErrorType in case of an error.
686691
static Type resolveTypeDecl(TypeChecker &TC, TypeDecl *typeDecl, SourceLoc loc,
687692
DeclContext *foundDC,

lib/Sema/TypeChecker.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -702,6 +702,13 @@ enum class ConformanceCheckFlags {
702702
/// correctly used. Otherwise (the default), all of the conditional
703703
/// requirements will be checked.
704704
SkipConditionalRequirements = 0x08,
705+
706+
/// Whether to require that the conditional requirements have been computed.
707+
///
708+
/// When set, if the conditional requirements aren't available, they are just
709+
/// skipped, and the caller is responsible for detecting and handling this
710+
/// case (likely via another call to getConditionalRequirementsIfAvailable).
711+
AllowUnavailableConditionalRequirements = 0x10,
705712
};
706713

707714
/// Options that control protocol conformance checking.
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// RUN: %target-typecheck-verify-swift
2+
3+
struct Foo<T> {}
4+
5+
protocol P1 {
6+
associatedtype A // expected-note {{protocol requires nested type 'A'; do you want to add it?}}
7+
}
8+
extension Foo: P1 where A : P1 {} // expected-error {{unsupported recursion for reference to associated type 'A' of type 'Foo<T>'}}
9+
// expected-error@-1 {{type 'Foo<T>' does not conform to protocol 'P1'}}

0 commit comments

Comments
 (0)