From a3f2b6786ab966d78104aab1b38c69f262cff1b6 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 4 Oct 2016 20:42:03 -0400 Subject: [PATCH 1/2] Add changelog entry for concrete constrained extensions PR --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8a179e62108d..beeaa399cdfc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,14 @@ CHANGELOG Swift 3.1 --------- +* [SR-1009](https://bugs.swift.org/browse/SR-1009): + + Constrained extensions allow same-type constraints between generic parameters and concrete types. + + ```swift + extension Array where Element == Int { } + ``` + * [SE-0045][]: The `Sequence` protocol adds new members `prefix(while:)` and From 7069fbc9be222afae8adcdcecca395591df1ed53 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 4 Oct 2016 21:07:29 -0400 Subject: [PATCH 2/2] Sema: Remove unnecessary error path in generic signature validation The idea here is that if a generic signature has invalid requirements, we would drop all the requirements and build a new set of archetypes without requirements. When this logic was added, it fixed 700 compiler_crashers: Nowadays it appears that all the underlying issues were solved, so removing this error path actually fixed two crashers and improved a couple of diagnostics. --- lib/Sema/TypeCheckDecl.cpp | 124 +++++++----------- lib/Sema/TypeCheckGeneric.cpp | 95 +++----------- lib/Sema/TypeChecker.h | 17 +-- .../invalid_constraint_lookup.swift | 4 +- test/Generics/associated_type_typo.swift | 4 +- ...28432-swift-typechecker-validatedecl.swift | 2 +- ...8433-swift-typechecker-typecheckdecl.swift | 2 +- 7 files changed, 78 insertions(+), 170 deletions(-) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/28432-swift-typechecker-validatedecl.swift (89%) rename validation-test/{compiler_crashers => compiler_crashers_fixed}/28433-swift-typechecker-typecheckdecl.swift (88%) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index b8018c9257d9c..0f4949cae3fcd 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -728,12 +728,9 @@ TypeChecker::handleSILGenericParams(GenericParamList *genericParams, for (unsigned i = 0, e = nestedList.size(); i < e; i++) { auto genericParams = nestedList.rbegin()[i]; - bool invalid = false; auto *genericSig = validateGenericSignature(genericParams, DC, parentSig, /*allowConcreteGenericParams=*/true, - nullptr, invalid); - if (invalid) - return std::make_pair(nullptr, nullptr); + nullptr); revertGenericParamList(genericParams); @@ -4820,43 +4817,38 @@ class DeclChecker : public DeclVisitor { if (auto gp = FD->getGenericParams()) { gp->setOuterParameters(FD->getDeclContext()->getGenericParamsOfContext()); - if (TC.validateGenericFuncSignature(FD)) { - auto *env = TC.markInvalidGenericSignature(FD); - FD->setGenericEnvironment(env); - } else { - // Create a fresh archetype builder. - ArchetypeBuilder builder = - TC.createArchetypeBuilder(FD->getModuleContext()); - auto *parentSig = FD->getDeclContext()->getGenericSignatureOfContext(); - auto *parentEnv = FD->getDeclContext()->getGenericEnvironmentOfContext(); - TC.checkGenericParamList(&builder, gp, parentSig, parentEnv, nullptr); - - // Infer requirements from parameter patterns. - for (auto pattern : FD->getParameterLists()) { - builder.inferRequirements(pattern, gp); - } + TC.validateGenericFuncSignature(FD); - // Infer requirements from the result type. - if (!FD->getBodyResultTypeLoc().isNull()) { - builder.inferRequirements(FD->getBodyResultTypeLoc(), gp); - } - - // Revert the types within the signature so it can be type-checked with - // archetypes below. - TC.revertGenericFuncSignature(FD); + // Create a fresh archetype builder. + ArchetypeBuilder builder = + TC.createArchetypeBuilder(FD->getModuleContext()); + auto *parentSig = FD->getDeclContext()->getGenericSignatureOfContext(); + auto *parentEnv = FD->getDeclContext()->getGenericEnvironmentOfContext(); + TC.checkGenericParamList(&builder, gp, parentSig, parentEnv, nullptr); - // Assign archetypes. - auto *sig = FD->getGenericSignature(); - auto *env = builder.getGenericEnvironment(sig->getGenericParams()); - FD->setGenericEnvironment(env); + // Infer requirements from parameter patterns. + for (auto pattern : FD->getParameterLists()) { + builder.inferRequirements(pattern, gp); + } - TC.finalizeGenericParamList(gp, sig, env, FD); + // Infer requirements from the result type. + if (!FD->getBodyResultTypeLoc().isNull()) { + builder.inferRequirements(FD->getBodyResultTypeLoc(), gp); } + + // Revert the types within the signature so it can be type-checked with + // archetypes below. + TC.revertGenericFuncSignature(FD); + + // Assign archetypes. + auto *sig = FD->getGenericSignature(); + auto *env = builder.getGenericEnvironment(sig->getGenericParams()); + FD->setGenericEnvironment(env); + + TC.finalizeGenericParamList(gp, sig, env, FD); } else if (FD->getDeclContext()->getGenericSignatureOfContext()) { - if (TC.validateGenericFuncSignature(FD)) { - auto *env = TC.markInvalidGenericSignature(FD); - FD->setGenericEnvironment(env); - } else if (!FD->hasType()) { + TC.validateGenericFuncSignature(FD); + if (!FD->hasType()) { // Revert all of the types within the signature of the function. TC.revertGenericFuncSignature(FD); FD->setGenericEnvironment( @@ -6523,41 +6515,34 @@ class DeclChecker : public DeclVisitor { // Write up generic parameters and check the generic parameter list. gp->setOuterParameters(CD->getDeclContext()->getGenericParamsOfContext()); - if (TC.validateGenericFuncSignature(CD)) { - auto *env = TC.markInvalidGenericSignature(CD); - CD->setGenericEnvironment(env); - } else { - ArchetypeBuilder builder = - TC.createArchetypeBuilder(CD->getModuleContext()); - auto *parentSig = CD->getDeclContext()->getGenericSignatureOfContext(); - auto *parentEnv = CD->getDeclContext()->getGenericEnvironmentOfContext(); - TC.checkGenericParamList(&builder, gp, parentSig, parentEnv, nullptr); + TC.validateGenericFuncSignature(CD); - // Infer requirements from the parameters of the constructor. - builder.inferRequirements(CD->getParameterList(1), gp); + auto builder = TC.createArchetypeBuilder(CD->getModuleContext()); + auto *parentSig = CD->getDeclContext()->getGenericSignatureOfContext(); + auto *parentEnv = CD->getDeclContext()->getGenericEnvironmentOfContext(); + TC.checkGenericParamList(&builder, gp, parentSig, parentEnv, nullptr); - // Revert the types within the signature so it can be type-checked with - // archetypes below. - TC.revertGenericFuncSignature(CD); + // Infer requirements from the parameters of the constructor. + builder.inferRequirements(CD->getParameterList(1), gp); - // Assign archetypes. - auto *sig = CD->getGenericSignature(); - auto *env = builder.getGenericEnvironment(sig->getGenericParams()); - CD->setGenericEnvironment(env); + // Revert the types within the signature so it can be type-checked with + // archetypes below. + TC.revertGenericFuncSignature(CD); - TC.finalizeGenericParamList(gp, sig, env, CD); - } + // Assign archetypes. + auto *sig = CD->getGenericSignature(); + auto *env = builder.getGenericEnvironment(sig->getGenericParams()); + CD->setGenericEnvironment(env); + + TC.finalizeGenericParamList(gp, sig, env, CD); } else if (CD->getDeclContext()->getGenericSignatureOfContext()) { - if (TC.validateGenericFuncSignature(CD)) { - auto *env = TC.markInvalidGenericSignature(CD); - CD->setGenericEnvironment(env); - } else { - // Revert all of the types within the signature of the constructor. - TC.revertGenericFuncSignature(CD); + TC.validateGenericFuncSignature(CD); - CD->setGenericEnvironment( - CD->getDeclContext()->getGenericEnvironmentOfContext()); - } + // Revert all of the types within the signature of the constructor. + TC.revertGenericFuncSignature(CD); + + CD->setGenericEnvironment( + CD->getDeclContext()->getGenericEnvironmentOfContext()); } { @@ -7543,21 +7528,14 @@ static Type checkExtensionGenericParams( SWIFT_DEFER { ext->setIsBeingTypeChecked(false); }; // Validate the generic type signature. - bool invalid = false; auto *parentSig = ext->getDeclContext()->getGenericSignatureOfContext(); auto *parentEnv = ext->getDeclContext()->getGenericEnvironmentOfContext(); auto *sig = tc.validateGenericSignature(genericParams, ext->getDeclContext(), parentSig, /*allowConcreteGenericParams=*/true, - inferExtendedTypeReqs, invalid); + inferExtendedTypeReqs); ext->setGenericSignature(sig); - if (invalid) { - auto *env = tc.markInvalidGenericSignature(ext); - ext->setGenericEnvironment(env); - return nullptr; - } - // Validate the generic parameters for the last time. tc.revertGenericParamList(genericParams); ArchetypeBuilder builder = tc.createArchetypeBuilder(ext->getModuleContext()); diff --git a/lib/Sema/TypeCheckGeneric.cpp b/lib/Sema/TypeCheckGeneric.cpp index 5a5760a977d3f..e1f8bb404acc3 100644 --- a/lib/Sema/TypeCheckGeneric.cpp +++ b/lib/Sema/TypeCheckGeneric.cpp @@ -253,13 +253,11 @@ Type CompleteGenericTypeResolver::resolveTypeOfDecl(TypeDecl *decl) { /// Check the generic parameters in the given generic parameter list (and its /// parent generic parameter lists) according to the given resolver. -bool TypeChecker::checkGenericParamList(ArchetypeBuilder *builder, +void TypeChecker::checkGenericParamList(ArchetypeBuilder *builder, GenericParamList *genericParams, GenericSignature *parentSig, GenericEnvironment *parentEnv, GenericTypeResolver *resolver) { - bool invalid = false; - // If there is a parent context, add the generic parameters and requirements // from that context. if (builder) @@ -267,7 +265,7 @@ bool TypeChecker::checkGenericParamList(ArchetypeBuilder *builder, // If there aren't any generic parameters at this level, we're done. if (!genericParams) - return false; + return; assert(genericParams->size() > 0 && "Parsed an empty generic parameter list?"); @@ -302,8 +300,7 @@ bool TypeChecker::checkGenericParamList(ArchetypeBuilder *builder, // Infer requirements from the inherited types. for (const auto &inherited : param->getInherited()) { - if (builder->inferRequirements(inherited, genericParams)) - invalid = true; + builder->inferRequirements(inherited, genericParams); } } } @@ -319,14 +316,12 @@ bool TypeChecker::checkGenericParamList(ArchetypeBuilder *builder, case RequirementReprKind::TypeConstraint: { // Validate the types. if (validateType(req.getSubjectLoc(), lookupDC, options, resolver)) { - invalid = true; req.setInvalid(); continue; } if (validateType(req.getConstraintLoc(), lookupDC, options, resolver)) { - invalid = true; req.setInvalid(); continue; } @@ -338,7 +333,6 @@ bool TypeChecker::checkGenericParamList(ArchetypeBuilder *builder, diag::requires_conformance_nonprotocol, req.getSubjectLoc(), req.getConstraintLoc()); req.getConstraintLoc().setInvalidType(Context); - invalid = true; req.setInvalid(); continue; } @@ -349,14 +343,12 @@ bool TypeChecker::checkGenericParamList(ArchetypeBuilder *builder, case RequirementReprKind::SameType: if (validateType(req.getFirstTypeLoc(), lookupDC, options, resolver)) { - invalid = true; req.setInvalid(); continue; } if (validateType(req.getSecondTypeLoc(), lookupDC, options, resolver)) { - invalid = true; req.setInvalid(); continue; } @@ -364,13 +356,9 @@ bool TypeChecker::checkGenericParamList(ArchetypeBuilder *builder, break; } - if (builder && builder->addRequirement(req)) { - invalid = true; + if (builder && builder->addRequirement(req)) req.setInvalid(); - } } - - return invalid; } /// Collect all of the generic parameter types at every level in the generic @@ -490,37 +478,7 @@ static Type getResultType(TypeChecker &TC, FuncDecl *fn, Type resultType) { return resultType; } -GenericEnvironment * -TypeChecker::markInvalidGenericSignature(DeclContext *DC) { - GenericParamList *genericParams = DC->getGenericParamsOfContext(); - GenericSignature *genericSig = DC->getGenericSignatureOfContext(); - - // Build new archetypes without any generic requirements. - DeclContext *parentDC = DC->getParent(); - auto builder = createArchetypeBuilder(parentDC->getParentModule()); - - auto parentSig = parentDC->getGenericSignatureOfContext(); - auto parentEnv = parentDC->getGenericEnvironmentOfContext(); - - assert(parentSig != nullptr || DC->isInnermostContextGeneric()); - - if (parentSig != nullptr) - builder.addGenericSignature(parentSig, parentEnv); - - if (DC->isInnermostContextGeneric()) { - // Visit each of the generic parameters. - for (auto param : *genericParams) - builder.addGenericParameter(param); - } - - // Wire up the archetypes. - auto genericEnv = builder.getGenericEnvironment( - genericSig->getGenericParams()); - - return genericEnv; -} - -bool TypeChecker::validateGenericFuncSignature(AbstractFunctionDecl *func) { +void TypeChecker::validateGenericFuncSignature(AbstractFunctionDecl *func) { bool invalid = false; // Create the archetype builder. @@ -535,7 +493,7 @@ bool TypeChecker::validateGenericFuncSignature(AbstractFunctionDecl *func) { // If this triggered a recursive validation, back out: we're done. // FIXME: This is an awful hack. if (func->hasType()) - return !func->isInvalid(); + return; // Finalize the generic requirements. (void)builder.finalize(func->getLoc()); @@ -577,11 +535,10 @@ bool TypeChecker::validateGenericFuncSignature(AbstractFunctionDecl *func) { if (invalid) { func->overwriteType(ErrorType::get(Context)); func->setInterfaceType(ErrorType::get(Context)); - return true; + return; } configureInterfaceType(func); - return false; } void TypeChecker::configureInterfaceType(AbstractFunctionDecl *func) { @@ -726,8 +683,7 @@ GenericSignature *TypeChecker::validateGenericSignature( DeclContext *dc, GenericSignature *parentSig, bool allowConcreteGenericParams, - std::function inferRequirements, - bool &invalid) { + std::function inferRequirements) { assert(genericParams && "Missing generic parameters?"); // Create the archetype builder. @@ -737,15 +693,12 @@ GenericSignature *TypeChecker::validateGenericSignature( // Type check the generic parameters, treating all generic type // parameters as dependent, unresolved. DependentGenericTypeResolver dependentResolver(builder); - if (checkGenericParamList(&builder, genericParams, parentSig, - nullptr, &dependentResolver)) { - invalid = true; - } + checkGenericParamList(&builder, genericParams, parentSig, + nullptr, &dependentResolver); /// Perform any necessary requirement inference. - if (inferRequirements && inferRequirements(builder)) { - invalid = true; - } + if (inferRequirements) + inferRequirements(builder); // Finalize the generic requirements. (void)builder.finalize(genericParams->getSourceRange().Start, @@ -756,10 +709,8 @@ GenericSignature *TypeChecker::validateGenericSignature( // and type-check it again, completely. revertGenericParamList(genericParams); CompleteGenericTypeResolver completeResolver(*this, builder); - if (checkGenericParamList(nullptr, genericParams, nullptr, - nullptr, &completeResolver)) { - invalid = true; - } + checkGenericParamList(nullptr, genericParams, nullptr, + nullptr, &completeResolver); // The generic signature is complete and well-formed. Gather the // generic parameter types at all levels. @@ -900,11 +851,9 @@ void TypeChecker::revertGenericParamList(GenericParamList *genericParams) { } } -bool TypeChecker::validateGenericTypeSignature(GenericTypeDecl *typeDecl) { - bool invalid = false; - +void TypeChecker::validateGenericTypeSignature(GenericTypeDecl *typeDecl) { if (typeDecl->isValidatingGenericSignature()) - return invalid; + return; typeDecl->setIsValidatingGenericSignature(); @@ -918,22 +867,16 @@ bool TypeChecker::validateGenericTypeSignature(GenericTypeDecl *typeDecl) { auto *parentEnv = dc->getGenericEnvironmentOfContext(); typeDecl->setGenericSignature(parentSig); typeDecl->setGenericEnvironment(parentEnv); - return invalid; + return; } auto *sig = validateGenericSignature(gp, dc, dc->getGenericSignatureOfContext(), /*allowConcreteGenericParams=*/false, - nullptr, invalid); + nullptr); assert(sig->getInnermostGenericParams().size() == typeDecl->getGenericParams()->size()); typeDecl->setGenericSignature(sig); - if (invalid) { - auto *env = markInvalidGenericSignature(typeDecl); - typeDecl->setGenericEnvironment(env); - return invalid; - } - revertGenericParamList(gp); ArchetypeBuilder builder = @@ -946,8 +889,6 @@ bool TypeChecker::validateGenericTypeSignature(GenericTypeDecl *typeDecl) { typeDecl->setGenericEnvironment(env); finalizeGenericParamList(gp, sig, env, typeDecl); - - return invalid; } void TypeChecker::revertGenericFuncSignature(AbstractFunctionDecl *func) { diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index b19113b38b544..7ed61829f594a 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -1013,17 +1013,13 @@ class TypeChecker final : public LazyResolver { bool isProtocolExtensionUsable(DeclContext *dc, Type type, ExtensionDecl *protocolExtension) override; - GenericEnvironment *markInvalidGenericSignature(DeclContext *dc); - /// Configure the interface type of a function declaration. void configureInterfaceType(AbstractFunctionDecl *func); /// Validate the signature of a generic function. /// /// \param func The generic function. - /// - /// \returns true if an error occurred, or false otherwise. - bool validateGenericFuncSignature(AbstractFunctionDecl *func); + void validateGenericFuncSignature(AbstractFunctionDecl *func); /// Revert the signature of a generic function to its pre-type-checked state, /// so that it can be type checked again when we have resolved its generic @@ -1048,8 +1044,6 @@ class TypeChecker final : public LazyResolver { /// to perform any additional requirement inference that contributes to the /// generic signature. Returns true if an error occurred. /// - /// \param invalid Will be set true if an error occurs during validation. - /// /// \returns the generic signature that captures the generic /// parameters and inferred requirements. GenericSignature *validateGenericSignature( @@ -1057,8 +1051,7 @@ class TypeChecker final : public LazyResolver { DeclContext *dc, GenericSignature *outerSignature, bool allowConcreteGenericParams, - std::function inferRequirements, - bool &invalid); + std::function inferRequirements); /// Perform any final semantic checks on the given generic parameter list. void finalizeGenericParamList(GenericParamList *genericParams, @@ -1069,13 +1062,11 @@ class TypeChecker final : public LazyResolver { /// Validate the signature of a generic type. /// /// \param nominal The generic type. - /// - /// \returns true if an error occurred, or false otherwise. - bool validateGenericTypeSignature(GenericTypeDecl *nominal); + void validateGenericTypeSignature(GenericTypeDecl *nominal); /// Check the generic parameters in the given generic parameter list (and its /// parent generic parameter lists) according to the given resolver. - bool checkGenericParamList(ArchetypeBuilder *builder, + void checkGenericParamList(ArchetypeBuilder *builder, GenericParamList *genericParams, GenericSignature *parentSig, GenericEnvironment *parentEnv, diff --git a/test/Constraints/invalid_constraint_lookup.swift b/test/Constraints/invalid_constraint_lookup.swift index 9ac34ea2e5cf3..247b640a76163 100644 --- a/test/Constraints/invalid_constraint_lookup.swift +++ b/test/Constraints/invalid_constraint_lookup.swift @@ -5,9 +5,7 @@ protocol P { func makeIterator() -> Int } func f(_ rhs: U) -> X { // expected-error {{use of undeclared type 'X'}} - // FIXME: This diagnostic isn't great, it happens because the generic constraint - // 'U' from the invalid type signature never gets resolved. - let iter = rhs.makeIterator() // expected-error {{cannot invoke 'makeIterator' with no arguments}} + let iter = rhs.makeIterator() } struct Zzz { diff --git a/test/Generics/associated_type_typo.swift b/test/Generics/associated_type_typo.swift index 225563eaec12d..17b39bd460c96 100644 --- a/test/Generics/associated_type_typo.swift +++ b/test/Generics/associated_type_typo.swift @@ -51,11 +51,11 @@ func typoAssoc4(_: T) where T.Assocp2.assoc : P3 {} // func typoFunc1(x: TypoType) { // expected-error{{use of undeclared type 'TypoType'}} - let _: T.Assoc -> () = { let _ = $0 } // expected-error{{'Assoc' is not a member type of 'T'}} + let _: (T.Assoc) -> () = { let _ = $0 } } func typoFunc2(x: TypoType, y: T) { // expected-error{{use of undeclared type 'TypoType'}} - let _: T.Assoc -> () = { let _ = $0 } // expected-error{{'Assoc' is not a member type of 'T'}} + let _: (T.Assoc) -> () = { let _ = $0 } } func typoFunc3(x: TypoType, y: (T.Assoc) -> ()) { // expected-error{{use of undeclared type 'TypoType'}} diff --git a/validation-test/compiler_crashers/28432-swift-typechecker-validatedecl.swift b/validation-test/compiler_crashers_fixed/28432-swift-typechecker-validatedecl.swift similarity index 89% rename from validation-test/compiler_crashers/28432-swift-typechecker-validatedecl.swift rename to validation-test/compiler_crashers_fixed/28432-swift-typechecker-validatedecl.swift index 5d92dbaa5a407..52dde5fe5e9b9 100644 --- a/validation-test/compiler_crashers/28432-swift-typechecker-validatedecl.swift +++ b/validation-test/compiler_crashers_fixed/28432-swift-typechecker-validatedecl.swift @@ -5,7 +5,7 @@ // See http://swift.org/LICENSE.txt for license information // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// RUN: not --crash %target-swift-frontend %s -parse +// RUN: not %target-swift-frontend %s -parse // REQUIRES: asserts {protocol A {func e diff --git a/validation-test/compiler_crashers/28433-swift-typechecker-typecheckdecl.swift b/validation-test/compiler_crashers_fixed/28433-swift-typechecker-typecheckdecl.swift similarity index 88% rename from validation-test/compiler_crashers/28433-swift-typechecker-typecheckdecl.swift rename to validation-test/compiler_crashers_fixed/28433-swift-typechecker-typecheckdecl.swift index 3c0fa4ddd2652..ece7f30f4ad28 100644 --- a/validation-test/compiler_crashers/28433-swift-typechecker-typecheckdecl.swift +++ b/validation-test/compiler_crashers_fixed/28433-swift-typechecker-typecheckdecl.swift @@ -5,7 +5,7 @@ // See http://swift.org/LICENSE.txt for license information // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// RUN: not --crash %target-swift-frontend %s -parse +// RUN: not %target-swift-frontend %s -parse // REQUIRES: asserts for in[{protocol a{func e typealias e=()