diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index c73be27773a30..339b5188720e8 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -546,6 +546,11 @@ namespace swift { /// if you have a testcase which requires this, please submit a bug report. bool EnableRequirementMachineLoopNormalization = false; + /// Enable experimental, more correct support for opaque result types as + /// concrete types. This will sometimes fail to produce a convergent + /// rewrite system. + bool EnableRequirementMachineOpaqueArchetypes = false; + /// Enables dumping type witness systems from associated type inference. bool DumpTypeWitnessSystems = false; diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index 778f562cff7c8..82fe523892904 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -362,6 +362,9 @@ def disable_requirement_machine_concrete_contraction : Flag<["-"], "disable-requ def enable_requirement_machine_loop_normalization : Flag<["-"], "enable-requirement-machine-loop-normalization">, HelpText<"Enable stronger minimization algorithm, for debugging only">; +def enable_requirement_machine_opaque_archetypes : Flag<["-"], "enable-requirement-machine-opaque-archetypes">, + HelpText<"Enable more correct opaque archetype support, which is off by default because it might fail to produce a convergent rewrite system">; + def dump_type_witness_systems : Flag<["-"], "dump-type-witness-systems">, HelpText<"Enables dumping type witness systems from associated type inference">; diff --git a/lib/AST/RequirementMachine/ConcreteContraction.cpp b/lib/AST/RequirementMachine/ConcreteContraction.cpp index a90bb78cd48e0..0ce15268953d8 100644 --- a/lib/AST/RequirementMachine/ConcreteContraction.cpp +++ b/lib/AST/RequirementMachine/ConcreteContraction.cpp @@ -235,19 +235,10 @@ Optional ConcreteContraction::substTypeParameter( ->substBaseType(module, *substBaseType); } - auto *decl = (*substBaseType)->getAnyNominal(); - if (decl == nullptr) { - if (Debug) { - llvm::dbgs() << "@@@ Not a nominal type: " << *substBaseType << "\n"; - } - - return None; - } - // An unresolved DependentMemberType stores an identifier. Handle this // by performing a name lookup into the base type. SmallVector concreteDecls; - lookupConcreteNestedType(decl, memberType->getName(), concreteDecls); + lookupConcreteNestedType(*substBaseType, memberType->getName(), concreteDecls); auto *typeDecl = findBestConcreteNestedType(concreteDecls); if (typeDecl == nullptr) { @@ -261,8 +252,9 @@ Optional ConcreteContraction::substTypeParameter( } // Substitute the base type into the member type. + auto *dc = typeDecl->getDeclContext(); auto subMap = (*substBaseType)->getContextSubstitutionMap( - decl->getParentModule(), typeDecl->getDeclContext()); + dc->getParentModule(), dc); return typeDecl->getDeclaredInterfaceType().subst(subMap); } diff --git a/lib/AST/RequirementMachine/ConcreteTypeWitness.cpp b/lib/AST/RequirementMachine/ConcreteTypeWitness.cpp index 9b43018374a7e..c2578ff7e9c17 100644 --- a/lib/AST/RequirementMachine/ConcreteTypeWitness.cpp +++ b/lib/AST/RequirementMachine/ConcreteTypeWitness.cpp @@ -132,9 +132,7 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent( // // This occurs when a pair of rules are inherited from the property map // entry for this key's suffix. - auto pair = std::make_pair(concreteRuleID, conformanceRuleID); - auto found = ConcreteConformances.find(pair); - if (found != ConcreteConformances.end()) + if (!checkRulePairOnce(concreteRuleID, conformanceRuleID)) continue; // FIXME: Either remove the ModuleDecl entirely from conformance lookup, @@ -164,40 +162,44 @@ void PropertyMap::concretizeNestedTypesFromConcreteParent( continue; } - // FIXME: Maybe this can happen if the concrete type is an - // opaque result type? - assert(!conformance.isAbstract()); - - // Save this conformance for later. - auto *concrete = conformance.getConcrete(); - auto inserted = ConcreteConformances.insert( - std::make_pair(pair, concrete)); - assert(inserted.second); - (void) inserted; - auto concreteConformanceSymbol = Symbol::forConcreteConformance( concreteType, substitutions, proto, Context); recordConcreteConformanceRule(concreteRuleID, conformanceRuleID, requirementKind, concreteConformanceSymbol); + // This is disabled by default because we fail to produce a convergent + // rewrite system if the opaque archetype has infinitely-recursive + // nested types. Fixing this requires a better representation for + // concrete conformances in the rewrite system. + if (conformance.isAbstract() && + !Context.getASTContext().LangOpts.EnableRequirementMachineOpaqueArchetypes) { + if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { + llvm::dbgs() << "^^ " << "Skipping abstract conformance of " + << concreteType << " to " << proto->getName() << "\n"; + } + + continue; + } + for (auto *assocType : proto->getAssociatedTypeMembers()) { concretizeTypeWitnessInConformance(key, requirementKind, concreteConformanceSymbol, - concrete, assocType); + conformance, assocType); } // We only infer conditional requirements in top-level generic signatures, // not in protocol requirement signatures. - if (key.getRootProtocol() == nullptr) - inferConditionalRequirements(concrete, substitutions); + if (conformance.isConcrete() && + key.getRootProtocol() == nullptr) + inferConditionalRequirements(conformance.getConcrete(), substitutions); } } void PropertyMap::concretizeTypeWitnessInConformance( Term key, RequirementKind requirementKind, Symbol concreteConformanceSymbol, - ProtocolConformance *concrete, + ProtocolConformanceRef conformance, AssociatedTypeDecl *assocType) const { auto concreteType = concreteConformanceSymbol.getConcreteType(); auto substitutions = concreteConformanceSymbol.getSubstitutions(); @@ -211,17 +213,35 @@ void PropertyMap::concretizeTypeWitnessInConformance( << " on " << concreteType << "\n"; } - auto t = concrete->getTypeWitness(assocType); - if (!t) { - if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { - llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName() - << " of " << concreteType << " could not be inferred\n"; + CanType typeWitness; + if (conformance.isConcrete()) { + auto t = conformance.getConcrete()->getTypeWitness(assocType); + + if (!t) { + if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { + llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName() + << " of " << concreteType << " could not be inferred\n"; + } + + t = ErrorType::get(concreteType); } - t = ErrorType::get(concreteType); - } + typeWitness = t->getCanonicalType(); + } else if (conformance.isAbstract()) { + auto archetype = concreteType->getAs(); + if (archetype == nullptr) { + llvm::errs() << "Should only have an abstract conformance with an " + << "opaque archetype type\n"; + llvm::errs() << "Symbol: " << concreteConformanceSymbol << "\n"; + llvm::errs() << "Term: " << key << "\n"; + dump(llvm::errs()); + abort(); + } - auto typeWitness = t->getCanonicalType(); + typeWitness = archetype->getNestedType(assocType)->getCanonicalType(); + } else if (conformance.isInvalid()) { + typeWitness = CanType(ErrorType::get(Context.getASTContext())); + } if (Debug.contains(DebugFlags::ConcretizeNestedTypes)) { llvm::dbgs() << "^^ " << "Type witness for " << assocType->getName() diff --git a/lib/AST/RequirementMachine/GenericSignatureQueries.cpp b/lib/AST/RequirementMachine/GenericSignatureQueries.cpp index 18a56f95f373d..c7f113ea14ffb 100644 --- a/lib/AST/RequirementMachine/GenericSignatureQueries.cpp +++ b/lib/AST/RequirementMachine/GenericSignatureQueries.cpp @@ -17,6 +17,7 @@ #include "swift/AST/ASTContext.h" #include "swift/AST/Decl.h" +#include "swift/AST/GenericEnvironment.h" #include "swift/AST/GenericSignature.h" #include "swift/AST/Module.h" #include @@ -659,8 +660,7 @@ RequirementMachine::lookupNestedType(Type depType, Identifier name) const { typeToSearch = props->getSuperclassBound(); if (typeToSearch) - if (auto *decl = typeToSearch->getAnyNominal()) - lookupConcreteNestedType(decl, name, concreteDecls); + lookupConcreteNestedType(typeToSearch, name, concreteDecls); } if (bestAssocType) { diff --git a/lib/AST/RequirementMachine/NameLookup.cpp b/lib/AST/RequirementMachine/NameLookup.cpp index d4f75c62a9289..0f8382e4ffd3a 100644 --- a/lib/AST/RequirementMachine/NameLookup.cpp +++ b/lib/AST/RequirementMachine/NameLookup.cpp @@ -12,13 +12,33 @@ #include "NameLookup.h" #include "swift/AST/Decl.h" +#include "swift/AST/GenericEnvironment.h" #include "swift/AST/Module.h" +#include "swift/AST/Types.h" #include "llvm/ADT/SmallVector.h" #include using namespace swift; using namespace rewriting; +void +swift::rewriting::lookupConcreteNestedType( + Type baseType, + Identifier name, + SmallVectorImpl &concreteDecls) { + if (auto *decl = baseType->getAnyNominal()) + lookupConcreteNestedType(decl, name, concreteDecls); + else if (auto *archetype = baseType->getAs()) { + // If our concrete type is an opaque result archetype, look into its + // generic environment recursively. + auto *genericEnv = archetype->getGenericEnvironment(); + auto genericSig = genericEnv->getGenericSignature(); + + concreteDecls.push_back( + genericSig->lookupNestedType(archetype->getInterfaceType(), name)); + } +} + void swift::rewriting::lookupConcreteNestedType( NominalTypeDecl *decl, diff --git a/lib/AST/RequirementMachine/NameLookup.h b/lib/AST/RequirementMachine/NameLookup.h index d44e501aba3a0..55d69683ae550 100644 --- a/lib/AST/RequirementMachine/NameLookup.h +++ b/lib/AST/RequirementMachine/NameLookup.h @@ -19,10 +19,16 @@ namespace swift { class Identifier; class NominalTypeDecl; +class Type; class TypeDecl; namespace rewriting { +void lookupConcreteNestedType( + Type baseType, + Identifier name, + llvm::SmallVectorImpl &concreteDecls); + void lookupConcreteNestedType( NominalTypeDecl *decl, Identifier name, diff --git a/lib/AST/RequirementMachine/PropertyMap.h b/lib/AST/RequirementMachine/PropertyMap.h index 050699f3e440e..3f2c134b89aeb 100644 --- a/lib/AST/RequirementMachine/PropertyMap.h +++ b/lib/AST/RequirementMachine/PropertyMap.h @@ -174,12 +174,6 @@ class PropertyMap { // To avoid wasted work from re-introducing the same induced rules, // we track the rules we've seen already on previous builds. - /// Maps a pair of rules where the first is a conformance rule and the - /// second is a superclass or concrete type rule, to a concrete - /// conformance. - llvm::DenseMap, ProtocolConformance *> - ConcreteConformances; - /// Superclass requirements always imply a layout requirement, and /// concrete type requirements where the type is a class imply a /// superclass requirement. @@ -291,7 +285,7 @@ class PropertyMap { void concretizeTypeWitnessInConformance( Term key, RequirementKind requirementKind, Symbol concreteConformanceSymbol, - ProtocolConformance *concrete, + ProtocolConformanceRef conformance, AssociatedTypeDecl *assocType) const; void inferConditionalRequirements( diff --git a/lib/AST/RequirementMachine/RequirementLowering.cpp b/lib/AST/RequirementMachine/RequirementLowering.cpp index 417c1751a7f7b..297a7af55caf8 100644 --- a/lib/AST/RequirementMachine/RequirementLowering.cpp +++ b/lib/AST/RequirementMachine/RequirementLowering.cpp @@ -168,13 +168,13 @@ static void desugarConformanceRequirement(Type subjectType, Type constraintType, errors.push_back(RequirementError::forRedundantRequirement( {RequirementKind::Conformance, subjectType, constraintType}, loc)); - assert(conformance.isConcrete()); - auto *concrete = conformance.getConcrete(); - - // Introduce conditional requirements if the subject type is concrete. - for (auto req : concrete->getConditionalRequirements()) { - desugarRequirement(req, result, errors); + if (conformance.isConcrete()) { + // Introduce conditional requirements if the conformance is concrete. + for (auto req : conformance.getConcrete()->getConditionalRequirements()) { + desugarRequirement(req, result, errors); + } } + return; } diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 9e9cddc9f2ae7..cfa19243647f2 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1004,6 +1004,9 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args, if (Args.hasArg(OPT_enable_requirement_machine_loop_normalization)) Opts.EnableRequirementMachineLoopNormalization = true; + if (Args.hasArg(OPT_enable_requirement_machine_opaque_archetypes)) + Opts.EnableRequirementMachineOpaqueArchetypes = true; + Opts.DumpTypeWitnessSystems = Args.hasArg(OPT_dump_type_witness_systems); return HadError || UnsupportedOS || UnsupportedArch; diff --git a/test/Generics/opaque_archetype_concrete_requirement.swift b/test/Generics/opaque_archetype_concrete_requirement.swift new file mode 100644 index 0000000000000..db28a679a52e3 --- /dev/null +++ b/test/Generics/opaque_archetype_concrete_requirement.swift @@ -0,0 +1,81 @@ +// RUN: %target-swift-frontend -typecheck -verify %s -disable-availability-checking -debug-generic-signatures -requirement-machine-inferred-signatures=on -enable-requirement-machine-opaque-archetypes 2>&1 | %FileCheck %s + +protocol P1 { + associatedtype T : P2 + associatedtype U +} + +struct S_P1 : P1 { + typealias T = S_P2 + typealias U = Int +} + +protocol P2 {} + +struct S_P2 : P2 {} + +protocol P { + associatedtype T + + var t: T { get } +} + +struct DefinesOpaqueP1 : P { + var t: some P1 { + return S_P1() + } +} + +struct ConcreteHasP {} + +// CHECK-LABEL: ExtensionDecl line={{.*}} base=ConcreteHasP +// CHECK-NEXT: Generic signature: +extension ConcreteHasP where T == DefinesOpaqueP1.T, TT == T.T, TU == T.U { + func checkSameType1(_ t: TT) -> DefinesOpaqueP1.T.T { return t } + func checkSameType2(_ u: TU) -> DefinesOpaqueP1.T.U { return u } + + func checkSameType3(_ t: T.T) -> DefinesOpaqueP1.T.T { return t } + func checkSameType4(_ u: T.U) -> DefinesOpaqueP1.T.U { return u } +} + +struct G {} + +protocol HasP { + associatedtype T : P1 + associatedtype U +} + +// CHECK-LABEL: ExtensionDecl line={{.*}} base=HasP +// CHECK-NEXT: Generic signature: > +extension HasP where T == DefinesOpaqueP1.T, U == G { + func checkSameType1(_ t: T.T) -> DefinesOpaqueP1.T.T { return t } + func checkSameType2(_ u: T.U) -> DefinesOpaqueP1.T.U { return u } +} + +// FIXME: This does not work with -enable-requirement-machine-opaque-archetypes. +// See opaque_archetype_concrete_requirement_recursive.swift for a demonstration +// that it works without the flag (but more involved examples like the above +// won't work). + +protocol RecursiveP { + associatedtype T : RecursiveP +} + +struct S_RecursiveP : RecursiveP { + typealias T = S_RecursiveP +} + +struct DefinesRecursiveP : P { + var t: some RecursiveP { + return S_RecursiveP() + } +} + +protocol HasRecursiveP { + associatedtype T : RecursiveP +} + +extension HasRecursiveP where T == DefinesRecursiveP.T {} +// expected-error@-1 {{cannot build rewrite system for generic signature; rule length limit exceeded}} +// expected-note@-2 {{failed rewrite rule is τ_0_0.[HasRecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[concrete: ((((((((((some RecursiveP).T).T).T).T).T).T).T).T).T).T] => τ_0_0.[HasRecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T].[RecursiveP:T]}} + diff --git a/test/Generics/opaque_archetype_concrete_requirement_recursive.swift b/test/Generics/opaque_archetype_concrete_requirement_recursive.swift new file mode 100644 index 0000000000000..d6db0d55a03a8 --- /dev/null +++ b/test/Generics/opaque_archetype_concrete_requirement_recursive.swift @@ -0,0 +1,36 @@ +// RUN: %target-swift-frontend -typecheck -verify %s -disable-availability-checking -debug-generic-signatures -requirement-machine-inferred-signatures=on 2>&1 | %FileCheck %s + +protocol P { + associatedtype T + + var t: T { get } +} + +// FIXME: This does not work with -enable-requirement-machine-opaque-archetypes. +// See opaque_archetype_concrete_requirement.swift for a demonstration that it +// fails with the flag. + +protocol RecursiveP { + associatedtype T : RecursiveP +} + +struct S_RecursiveP : RecursiveP { + typealias T = S_RecursiveP +} + +struct DefinesRecursiveP : P { + var t: some RecursiveP { + return S_RecursiveP() + } +} + +protocol HasRecursiveP { + associatedtype T : RecursiveP +} + +// CHECK-LABEL: ExtensionDecl line={{.*}} base=HasRecursiveP +// CHECK-NEXT: Generic signature: +extension HasRecursiveP where T == DefinesRecursiveP.T { + func checkSameType1(_ t: T) -> DefinesRecursiveP.T { return t } + func checkSameType2(_ t: T.T) -> DefinesRecursiveP.T.T { return t } +} diff --git a/validation-test/compiler_crashers_2/unsupported_recursive_opaque_conformance.swift b/validation-test/compiler_crashers_2_fixed/unsupported_recursive_opaque_conformance.swift similarity index 70% rename from validation-test/compiler_crashers_2/unsupported_recursive_opaque_conformance.swift rename to validation-test/compiler_crashers_2_fixed/unsupported_recursive_opaque_conformance.swift index 58f10d25472ca..dfcac93afbb10 100644 --- a/validation-test/compiler_crashers_2/unsupported_recursive_opaque_conformance.swift +++ b/validation-test/compiler_crashers_2_fixed/unsupported_recursive_opaque_conformance.swift @@ -1,6 +1,4 @@ -// RUN: not --crash %target-swift-frontend -disable-availability-checking -emit-ir -enable-parameterized-protocol-types %s - -// REQUIRES: asserts +// RUN: %target-swift-frontend -disable-availability-checking -emit-ir -enable-parameterized-protocol-types %s protocol P { var x: X { get }