Skip to content

Commit ed23eb3

Browse files
committed
Revert "SE-0157: Enable recursive protocol constraints by default."
This reverts commit afbdbae. Commit ded45a6 more than triples the type checking time when building Swift.o, so I am going to revert that , and it looks like this needs to be reverted as well if that commit is reverted.
1 parent 16b7de0 commit ed23eb3

File tree

9 files changed

+73
-10
lines changed

9 files changed

+73
-10
lines changed

include/swift/AST/DiagnosticsSema.def

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1666,6 +1666,8 @@ ERROR(requires_generic_param_made_equal_to_concrete,none,
16661666
(Type))
16671667
ERROR(recursive_type_reference,none,
16681668
"%0 %1 references itself", (DescriptiveDeclKind, Identifier))
1669+
ERROR(recursive_requirement_reference,none,
1670+
"type may not reference itself as a requirement",())
16691671
ERROR(recursive_same_type_constraint,none,
16701672
"same-type constraint %0 == %1 is recursive", (Type, Type))
16711673
ERROR(recursive_superclass_constraint,none,

include/swift/AST/GenericSignatureBuilder.h

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,10 @@ class GenericSignatureBuilder {
417417
template<typename F>
418418
void visitPotentialArchetypes(F f);
419419

420+
void markPotentialArchetypeRecursive(PotentialArchetype *pa,
421+
ProtocolDecl *proto,
422+
const RequirementSource *source);
423+
420424
public:
421425
/// Construct a new generic signature builder.
422426
///
@@ -1328,28 +1332,32 @@ class GenericSignatureBuilder::PotentialArchetype {
13281332
/// that share a name.
13291333
llvm::MapVector<Identifier, StoredNestedType> NestedTypes;
13301334

1335+
/// \brief Recursively conforms to itself.
1336+
unsigned IsRecursive : 1;
1337+
13311338
/// \brief Construct a new potential archetype for an unresolved
13321339
/// associated type.
13331340
PotentialArchetype(PotentialArchetype *parent, Identifier name);
13341341

13351342
/// \brief Construct a new potential archetype for an associated type.
13361343
PotentialArchetype(PotentialArchetype *parent, AssociatedTypeDecl *assocType)
1337-
: parentOrBuilder(parent), identifier(assocType)
1344+
: parentOrBuilder(parent), identifier(assocType), IsRecursive(false)
13381345
{
13391346
assert(parent != nullptr && "Not an associated type?");
13401347
}
13411348

13421349
/// \brief Construct a new potential archetype for a concrete declaration.
13431350
PotentialArchetype(PotentialArchetype *parent, TypeDecl *concreteDecl)
1344-
: parentOrBuilder(parent), identifier(concreteDecl)
1351+
: parentOrBuilder(parent), identifier(concreteDecl), IsRecursive(false)
13451352
{
13461353
assert(parent != nullptr && "Not an associated type?");
13471354
}
13481355

13491356
/// \brief Construct a new potential archetype for a generic parameter.
13501357
PotentialArchetype(GenericSignatureBuilder *builder,
13511358
GenericParamKey genericParam)
1352-
: parentOrBuilder(builder), identifier(genericParam)
1359+
: parentOrBuilder(builder), identifier(genericParam),
1360+
IsRecursive(false)
13531361
{
13541362
}
13551363

@@ -1592,6 +1600,9 @@ class GenericSignatureBuilder::PotentialArchetype {
15921600
return Type();
15931601
}
15941602

1603+
void setIsRecursive() { IsRecursive = true; }
1604+
bool isRecursive() const { return IsRecursive; }
1605+
15951606
LLVM_ATTRIBUTE_DEPRECATED(
15961607
void dump() const,
15971608
"only for use within the debugger");

include/swift/Basic/LangOptions.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,9 @@ namespace swift {
206206
/// Should we use \c ASTScope-based resolution for unqualified name lookup?
207207
bool EnableASTScopeLookup = false;
208208

209+
/// Enable SE-0157: Recursive Protocol Constraints.
210+
bool EnableRecursiveConstraints = false;
211+
209212
/// Whether to use the import as member inference system
210213
///
211214
/// When importing a global, try to infer whether we can import it as a

include/swift/Option/FrontendOptions.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,9 @@ def disable_target_os_checking :
109109
def enable_astscope_lookup : Flag<["-"], "enable-astscope-lookup">,
110110
HelpText<"Enable ASTScope-based unqualified name lookup">;
111111

112+
def enable_recursive_constraints : Flag<["-"], "enable-recursive-constraints">,
113+
HelpText<"Enable SE-0157: Recursive Protocol Constraints">;
114+
112115
def print_clang_stats : Flag<["-"], "print-clang-stats">,
113116
HelpText<"Print Clang importer statistics">;
114117

lib/AST/GenericSignatureBuilder.cpp

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3357,6 +3357,28 @@ ConstraintResult GenericSignatureBuilder::addSameTypeRequirementDirect(
33573357
}
33583358
}
33593359

3360+
// Local function to mark the given associated type as recursive,
3361+
// diagnosing it if this is the first such occurrence.
3362+
void GenericSignatureBuilder::markPotentialArchetypeRecursive(
3363+
PotentialArchetype *pa, ProtocolDecl *proto, const RequirementSource *source) {
3364+
if (pa->isRecursive())
3365+
return;
3366+
pa->setIsRecursive();
3367+
3368+
pa->addConformance(proto, source, *this);
3369+
if (!pa->getParent())
3370+
return;
3371+
3372+
auto assocType = pa->getResolvedAssociatedType();
3373+
if (!assocType || assocType->isInvalid())
3374+
return;
3375+
3376+
Diags.diagnose(assocType->getLoc(), diag::recursive_requirement_reference);
3377+
3378+
// Silence downstream errors referencing this associated type.
3379+
assocType->setInvalid();
3380+
}
3381+
33603382
ConstraintResult GenericSignatureBuilder::addInheritedRequirements(
33613383
TypeDecl *decl,
33623384
UnresolvedType type,
@@ -3407,6 +3429,26 @@ ConstraintResult GenericSignatureBuilder::addInheritedRequirements(
34073429
getFloatingSource(typeRepr, /*forInferred=*/true));
34083430
}
34093431

3432+
if (!decl->getASTContext().LangOpts.EnableRecursiveConstraints) {
3433+
// Check for direct recursion.
3434+
if (auto assocType = dyn_cast<AssociatedTypeDecl>(decl)) {
3435+
auto proto = assocType->getProtocol();
3436+
if (auto inheritedProto = inheritedType->getAs<ProtocolType>()) {
3437+
if (inheritedProto->getDecl() == proto ||
3438+
inheritedProto->getDecl()->inheritsFrom(proto)) {
3439+
auto source = getFloatingSource(typeRepr, /*forInferred=*/false);
3440+
if (auto resolved = resolve(type, source)) {
3441+
if (auto pa = resolved->getPotentialArchetype()) {
3442+
markPotentialArchetypeRecursive(pa, proto,
3443+
source.getSource(pa));
3444+
return ConstraintResult::Conflicting;
3445+
}
3446+
}
3447+
}
3448+
}
3449+
}
3450+
}
3451+
34103452
return addTypeRequirement(type, inheritedType,
34113453
getFloatingSource(typeRepr,
34123454
/*forInferred=*/false),

lib/Frontend/CompilerInvocation.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1007,6 +1007,8 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
10071007
}
10081008

10091009
Opts.EnableASTScopeLookup |= Args.hasArg(OPT_enable_astscope_lookup);
1010+
Opts.EnableRecursiveConstraints |=
1011+
Args.hasArg(OPT_enable_recursive_constraints);
10101012
Opts.DebugConstraintSolver |= Args.hasArg(OPT_debug_constraints);
10111013
Opts.EnableConstraintPropagation |= Args.hasArg(OPT_propagate_constraints);
10121014
Opts.IterativeTypeChecker |= Args.hasArg(OPT_iterative_type_checker);

test/Generics/same_type_constraints.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ extension Dictionary {
154154

155155
// rdar://problem/19245317
156156
protocol P {
157-
associatedtype T: P
157+
associatedtype T: P // expected-error{{type may not reference itself as a requirement}}
158158
}
159159

160160
struct S<A: P> {

test/decl/protocol/recursive_requirement.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
// -----
44

55
protocol Foo {
6-
associatedtype Bar : Foo
6+
associatedtype Bar : Foo // expected-error{{type may not reference itself as a requirement}}
77
}
88

99
struct Oroborous : Foo {
@@ -13,7 +13,7 @@ struct Oroborous : Foo {
1313
// -----
1414

1515
protocol P {
16-
associatedtype A : P
16+
associatedtype A : P // expected-error{{type may not reference itself as a requirement}}
1717
}
1818

1919
struct X<T: P> {
@@ -26,7 +26,7 @@ func f<T : P>(_ z: T) {
2626
// -----
2727

2828
protocol PP2 {
29-
associatedtype A : P2 = Self
29+
associatedtype A : P2 = Self // expected-error{{type may not reference itself as a requirement}}
3030
}
3131

3232
protocol P2 : PP2 {
@@ -41,13 +41,13 @@ struct Y2 : P2 {
4141
}
4242

4343
func f<T : P2>(_ z: T) {
44-
_ = X2<T.A>()
44+
_ = X2<T.A>() // expected-error{{type 'T.A' does not conform to protocol 'P2'}}
4545
}
4646

4747
// -----
4848

4949
protocol P3 {
50-
associatedtype A: P4 = Self
50+
associatedtype A: P4 = Self // expected-error{{type may not reference itself as a requirement}}
5151
}
5252

5353
protocol P4 : P3 {}

test/decl/protocol/recursive_requirement_ok.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %target-typecheck-verify-swift
1+
// RUN: %target-typecheck-verify-swift -enable-recursive-constraints
22

33
protocol P {
44
associatedtype Assoc : P

0 commit comments

Comments
 (0)