Skip to content

SE-0157: Enable recursive protocol constraints by default. #10940

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
Jul 13, 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
2 changes: 0 additions & 2 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1666,8 +1666,6 @@ ERROR(requires_generic_param_made_equal_to_concrete,none,
(Type))
ERROR(recursive_type_reference,none,
"%0 %1 references itself", (DescriptiveDeclKind, Identifier))
ERROR(recursive_requirement_reference,none,
"type may not reference itself as a requirement",())
ERROR(recursive_same_type_constraint,none,
"same-type constraint %0 == %1 is recursive", (Type, Type))
ERROR(recursive_superclass_constraint,none,
Expand Down
17 changes: 3 additions & 14 deletions include/swift/AST/GenericSignatureBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -417,10 +417,6 @@ class GenericSignatureBuilder {
template<typename F>
void visitPotentialArchetypes(F f);

void markPotentialArchetypeRecursive(PotentialArchetype *pa,
ProtocolDecl *proto,
const RequirementSource *source);

public:
/// Construct a new generic signature builder.
///
Expand Down Expand Up @@ -1332,32 +1328,28 @@ class GenericSignatureBuilder::PotentialArchetype {
/// that share a name.
llvm::MapVector<Identifier, StoredNestedType> NestedTypes;

/// \brief Recursively conforms to itself.
unsigned IsRecursive : 1;

/// \brief Construct a new potential archetype for an unresolved
/// associated type.
PotentialArchetype(PotentialArchetype *parent, Identifier name);

/// \brief Construct a new potential archetype for an associated type.
PotentialArchetype(PotentialArchetype *parent, AssociatedTypeDecl *assocType)
: parentOrBuilder(parent), identifier(assocType), IsRecursive(false)
: parentOrBuilder(parent), identifier(assocType)
{
assert(parent != nullptr && "Not an associated type?");
}

/// \brief Construct a new potential archetype for a concrete declaration.
PotentialArchetype(PotentialArchetype *parent, TypeDecl *concreteDecl)
: parentOrBuilder(parent), identifier(concreteDecl), IsRecursive(false)
: parentOrBuilder(parent), identifier(concreteDecl)
{
assert(parent != nullptr && "Not an associated type?");
}

/// \brief Construct a new potential archetype for a generic parameter.
PotentialArchetype(GenericSignatureBuilder *builder,
GenericParamKey genericParam)
: parentOrBuilder(builder), identifier(genericParam),
IsRecursive(false)
: parentOrBuilder(builder), identifier(genericParam)
{
}

Expand Down Expand Up @@ -1600,9 +1592,6 @@ class GenericSignatureBuilder::PotentialArchetype {
return Type();
}

void setIsRecursive() { IsRecursive = true; }
bool isRecursive() const { return IsRecursive; }

LLVM_ATTRIBUTE_DEPRECATED(
void dump() const,
"only for use within the debugger");
Expand Down
3 changes: 0 additions & 3 deletions include/swift/Basic/LangOptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,6 @@ namespace swift {
/// Should we use \c ASTScope-based resolution for unqualified name lookup?
bool EnableASTScopeLookup = false;

/// Enable SE-0157: Recursive Protocol Constraints.
bool EnableRecursiveConstraints = false;

/// Whether to use the import as member inference system
///
/// When importing a global, try to infer whether we can import it as a
Expand Down
3 changes: 0 additions & 3 deletions include/swift/Option/FrontendOptions.td
Original file line number Diff line number Diff line change
Expand Up @@ -109,9 +109,6 @@ def disable_target_os_checking :
def enable_astscope_lookup : Flag<["-"], "enable-astscope-lookup">,
HelpText<"Enable ASTScope-based unqualified name lookup">;

def enable_recursive_constraints : Flag<["-"], "enable-recursive-constraints">,
HelpText<"Enable SE-0157: Recursive Protocol Constraints">;

def print_clang_stats : Flag<["-"], "print-clang-stats">,
HelpText<"Print Clang importer statistics">;

Expand Down
42 changes: 0 additions & 42 deletions lib/AST/GenericSignatureBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3357,28 +3357,6 @@ ConstraintResult GenericSignatureBuilder::addSameTypeRequirementDirect(
}
}

// Local function to mark the given associated type as recursive,
// diagnosing it if this is the first such occurrence.
void GenericSignatureBuilder::markPotentialArchetypeRecursive(
PotentialArchetype *pa, ProtocolDecl *proto, const RequirementSource *source) {
if (pa->isRecursive())
return;
pa->setIsRecursive();

pa->addConformance(proto, source, *this);
if (!pa->getParent())
return;

auto assocType = pa->getResolvedAssociatedType();
if (!assocType || assocType->isInvalid())
return;

Diags.diagnose(assocType->getLoc(), diag::recursive_requirement_reference);

// Silence downstream errors referencing this associated type.
assocType->setInvalid();
}

ConstraintResult GenericSignatureBuilder::addInheritedRequirements(
TypeDecl *decl,
UnresolvedType type,
Expand Down Expand Up @@ -3429,26 +3407,6 @@ ConstraintResult GenericSignatureBuilder::addInheritedRequirements(
getFloatingSource(typeRepr, /*forInferred=*/true));
}

if (!decl->getASTContext().LangOpts.EnableRecursiveConstraints) {
// Check for direct recursion.
if (auto assocType = dyn_cast<AssociatedTypeDecl>(decl)) {
auto proto = assocType->getProtocol();
if (auto inheritedProto = inheritedType->getAs<ProtocolType>()) {
if (inheritedProto->getDecl() == proto ||
inheritedProto->getDecl()->inheritsFrom(proto)) {
auto source = getFloatingSource(typeRepr, /*forInferred=*/false);
if (auto resolved = resolve(type, source)) {
if (auto pa = resolved->getPotentialArchetype()) {
markPotentialArchetypeRecursive(pa, proto,
source.getSource(pa));
return ConstraintResult::Conflicting;
}
}
}
}
}
}

return addTypeRequirement(type, inheritedType,
getFloatingSource(typeRepr,
/*forInferred=*/false),
Expand Down
2 changes: 0 additions & 2 deletions lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1007,8 +1007,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
}

Opts.EnableASTScopeLookup |= Args.hasArg(OPT_enable_astscope_lookup);
Opts.EnableRecursiveConstraints |=
Args.hasArg(OPT_enable_recursive_constraints);
Opts.DebugConstraintSolver |= Args.hasArg(OPT_debug_constraints);
Opts.EnableConstraintPropagation |= Args.hasArg(OPT_propagate_constraints);
Opts.IterativeTypeChecker |= Args.hasArg(OPT_iterative_type_checker);
Expand Down
2 changes: 1 addition & 1 deletion test/Generics/same_type_constraints.swift
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ extension Dictionary {

// rdar://problem/19245317
protocol P {
associatedtype T: P // expected-error{{type may not reference itself as a requirement}}
associatedtype T: P
}

struct S<A: P> {
Expand Down
10 changes: 5 additions & 5 deletions test/decl/protocol/recursive_requirement.swift
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
// -----

protocol Foo {
associatedtype Bar : Foo // expected-error{{type may not reference itself as a requirement}}
associatedtype Bar : Foo
}

struct Oroborous : Foo {
Expand All @@ -13,7 +13,7 @@ struct Oroborous : Foo {
// -----

protocol P {
associatedtype A : P // expected-error{{type may not reference itself as a requirement}}
associatedtype A : P
}

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

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

protocol P2 : PP2 {
Expand All @@ -41,13 +41,13 @@ struct Y2 : P2 {
}

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

// -----

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

protocol P4 : P3 {}
Expand Down
2 changes: 1 addition & 1 deletion test/decl/protocol/recursive_requirement_ok.swift
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// RUN: %target-typecheck-verify-swift -enable-recursive-constraints
// RUN: %target-typecheck-verify-swift

protocol P {
associatedtype Assoc : P
Expand Down