From 20fc51d4f496f2bc4c506409f8924a65dc8ff99b Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 27 Feb 2020 15:04:43 -0800 Subject: [PATCH] [CSBindings] Open collection before binding parameter only if original argument type failed Instead of always opening argument type represented by a collection without type variables (to support subtyping when element is a labeled tuple), let's try original type first and if that fails use a slower path with indirection which attempts `array upcast`. Doing it this way helps to propagate contextual information faster which fixes a performance regression. Resolves: rdar://problem/54580247 --- lib/Sema/CSBindings.cpp | 33 +++++++++++-------- lib/Sema/CSSimplify.cpp | 8 +++-- test/Constraints/construction.swift | 1 - .../{slow => fast}/rdar54580427.swift | 4 +-- 4 files changed, 25 insertions(+), 21 deletions(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar54580427.swift (51%) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index baa96169881..335f8cb96dc 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -1043,6 +1043,25 @@ bool TypeVarBindingProducer::computeNext() { if (auto simplifiedSuper = CS.checkTypeOfBinding(TypeVar, supertype)) addNewBinding(binding.withType(*simplifiedSuper)); } + + auto srcLocator = binding.getLocator(); + if (srcLocator && + srcLocator->isLastElement() && + !type->hasTypeVariable() && CS.isCollectionType(type)) { + // If the type binding comes from the argument conversion, let's + // instead of binding collection types directly, try to bind + // using temporary type variables substituted for element + // types, that's going to ensure that subtype relationship is + // always preserved. + auto *BGT = type->castTo(); + auto UGT = UnboundGenericType::get(BGT->getDecl(), BGT->getParent(), + BGT->getASTContext()); + + auto dstLocator = TypeVar->getImpl().getLocator(); + auto newType = CS.openUnboundGenericType(UGT, dstLocator) + ->reconstituteSugar(/*recursive=*/false); + addNewBinding(binding.withType(newType)); + } } if (newBindings.empty()) @@ -1062,20 +1081,6 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const { if (Binding.hasDefaultedLiteralProtocol()) { type = cs.openUnboundGenericType(type, dstLocator); type = type->reconstituteSugar(/*recursive=*/false); - } else if (srcLocator && - srcLocator->isLastElement() && - !type->hasTypeVariable() && cs.isCollectionType(type)) { - // If the type binding comes from the argument conversion, let's - // instead of binding collection types directly, try to bind - // using temporary type variables substituted for element - // types, that's going to ensure that subtype relationship is - // always preserved. - auto *BGT = type->castTo(); - auto UGT = UnboundGenericType::get(BGT->getDecl(), BGT->getParent(), - BGT->getASTContext()); - - type = cs.openUnboundGenericType(UGT, dstLocator); - type = type->reconstituteSugar(/*recursive=*/false); } cs.addConstraint(ConstraintKind::Bind, TypeVar, type, srcLocator); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 5a3d0c34d80..fe3d9473232 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -3799,9 +3799,11 @@ bool ConstraintSystem::repairFailures( if (tupleLocator->isLastElement()) break; - // Generic argument failures have a more general fix which is attached to a - // parent type and aggregates all argument failures into a single fix. - if (tupleLocator->isLastElement()) + // Generic argument/requirement failures have a more general fix which + // is attached to a parent type and aggregates all argument failures + // into a single fix. + if (tupleLocator->isLastElement() || + tupleLocator->isLastElement()) break; ConstraintFix *fix; diff --git a/test/Constraints/construction.swift b/test/Constraints/construction.swift index b99ff42dbda..ac8c185bf51 100644 --- a/test/Constraints/construction.swift +++ b/test/Constraints/construction.swift @@ -216,7 +216,6 @@ func rdar_50668864() { struct Foo { init(anchors: [Int]) { // expected-note {{'init(anchors:)' declared here}} self = .init { _ in [] } // expected-error {{trailing closure passed to parameter of type '[Int]' that does not accept a closure}} - // expected-error@-1 {{generic parameter 'Element' could not be inferred}} } } } diff --git a/validation-test/Sema/type_checker_perf/slow/rdar54580427.swift b/validation-test/Sema/type_checker_perf/fast/rdar54580427.swift similarity index 51% rename from validation-test/Sema/type_checker_perf/slow/rdar54580427.swift rename to validation-test/Sema/type_checker_perf/fast/rdar54580427.swift index 071587356ae..d360f1bfc2b 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar54580427.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar54580427.swift @@ -1,7 +1,5 @@ -// FIXME: This should be linear instead of exponential. -// RUN: %scale-test --begin 1 --end 10 --step 1 --select NumLeafScopes --invert-result %s -Xfrontend=-solver-expression-time-threshold=1 +// RUN: %scale-test --begin 1 --end 20 --step 1 --select NumLeafScopes %s -Xfrontend=-solver-expression-time-threshold=1 // REQUIRES: asserts,no_asan -// REQUIRES: rdar57138194,SR11770 enum Val { case d([String: Val])