From b4a2fa2aa03120ea1548fbfc91f08ddf3b10cd17 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 28 Dec 2021 21:38:24 -0800 Subject: [PATCH 01/13] [ConstraintSystem] Record all disjunctions selected along the current path --- include/swift/Sema/ConstraintSystem.h | 5 +++++ lib/Sema/CSStep.h | 3 +++ 2 files changed, 8 insertions(+) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 352cd4b950500..b99b24e8b67b4 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2251,6 +2251,7 @@ class ConstraintSystem { friend class SplitterStep; friend class ComponentStep; friend class TypeVariableStep; + friend class DisjunctionStep; friend class ConjunctionStep; friend class ConjunctionElement; friend class RequirementFailure; @@ -2402,6 +2403,10 @@ class ConstraintSystem { /// the current constraint system. llvm::MapVector DisjunctionChoices; + /// The stack of all disjunctions selected during current path in order. + /// This stack is managed by the \c DisjunctionStep. + llvm::SmallVector SelectedDisjunctions; + /// A map from applied disjunction constraints to the corresponding /// argument function type. llvm::SmallMapVector diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index a08b1a16b5c03..c0a37d5ac8b0f 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -653,6 +653,7 @@ class DisjunctionStep final : public BindingStep { assert(Disjunction->getKind() == ConstraintKind::Disjunction); pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; + cs.SelectedDisjunctions.push_back(Disjunction); } ~DisjunctionStep() override { @@ -663,6 +664,8 @@ class DisjunctionStep final : public BindingStep { // Re-enable previously disabled overload choices. for (auto *choice : DisabledChoices) choice->setEnabled(); + + CS.SelectedDisjunctions.pop_back(); } StepResult resume(bool prevFailed) override; From aff5f656af86fde153760881718cfa9fc95ba4f8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 28 Dec 2021 21:41:09 -0800 Subject: [PATCH 02/13] [ConstraintSystem] Extract "best disjunction" selection mechanism into a separate method --- include/swift/Sema/ConstraintSystem.h | 3 +++ lib/Sema/CSSolver.cpp | 12 ++++++++++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index b99b24e8b67b4..fde6590168a02 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5028,6 +5028,9 @@ class ConstraintSystem { /// /// \returns The selected disjunction. Constraint *selectDisjunction(); + /// Select the best possible disjunction for solver to attempt + /// based on the given list. + Constraint *selectBestDisjunction(ArrayRef disjunctions); /// Pick a conjunction from the InactiveConstraints list. /// diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 7e547d4e030a7..0d3a79a9e928e 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1662,8 +1662,9 @@ ConstraintSystem::filterDisjunction( // right-hand side of a conversion constraint, since having a concrete // type that we're converting to can make it possible to split the // constraint system into multiple ones. -static Constraint *selectBestBindingDisjunction( - ConstraintSystem &cs, SmallVectorImpl &disjunctions) { +static Constraint * +selectBestBindingDisjunction(ConstraintSystem &cs, + ArrayRef disjunctions) { if (disjunctions.empty()) return nullptr; @@ -2170,6 +2171,13 @@ Constraint *ConstraintSystem::selectDisjunction() { if (disjunctions.empty()) return nullptr; + return selectBestDisjunction(disjunctions); +} + +Constraint * +ConstraintSystem::selectBestDisjunction(ArrayRef disjunctions) { + assert(!disjunctions.empty()); + if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return disjunction; From dd4c12c02bf4bfa2b813feadd44503422ce14d04 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 3 Jan 2022 15:23:32 -0800 Subject: [PATCH 03/13] [TypeChecker] Disable `ConstraintSystem::shrink` by default --- include/swift/Basic/LangOptions.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index a4680b9ab4dd8..5796147aa2346 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -679,7 +679,7 @@ namespace swift { unsigned SolverShrinkUnsolvedThreshold = 10; /// Disable the shrink phase of the expression type checker. - bool SolverDisableShrink = false; + bool SolverDisableShrink = true; /// Enable experimental operator designated types feature. bool EnableOperatorDesignatedTypes = false; From b4e5b69863c2798838efcd505c6cd51f607c5af9 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 2 Jan 2022 22:11:04 -0800 Subject: [PATCH 04/13] [CSSolver] Prefer disjunction that has at some favored overloads over one that has none If one of the sides has no favored overloads, it's a strong enough indication that we know more about the other side. --- lib/Sema/CSSolver.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 0d3a79a9e928e..c07b62567905a 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2190,8 +2190,15 @@ ConstraintSystem::selectBestDisjunction(ArrayRef disjunctions) { unsigned firstFavored = first->countFavoredNestedConstraints(); unsigned secondFavored = second->countFavoredNestedConstraints(); - if (!isOperatorDisjunction(first) || !isOperatorDisjunction(second)) + if (!isOperatorDisjunction(first) || !isOperatorDisjunction(second)) { + // If one of the sides has favored overloads, let's prefer it + // since it's a strong enough signal that there is something + // known about the arguments associated with the call. + if (firstFavored == 0 || secondFavored == 0) + return firstFavored > secondFavored; + return firstActive < secondActive; + } if (firstFavored == secondFavored) { // Look for additional choices that are "favored" From 7286a4f8e3af8bb6918ff7d09adbdd867fa7e2c1 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 5 Jan 2022 16:19:11 -0800 Subject: [PATCH 05/13] [ConstraintSystem] New "stable" disjunction selection algorithm Prelude. The core idea behind `shrink` is simple - reduce overload sets via a bottom-up walk'n'solve that would utilize previously discovered solutions along the way. This helps in some circumstances but requires rollbacks and AST modification (if choices produced by previous steps fail to produce solutions higher up). For some expressions, especially ones with multiple generic overload sets, `shrink` is actively harmful because it would never be able to produce useful results. The Algorithm. These changes integrate core idea of local information propagation from `shrink` into the disjunction selection algorithm itself. The algorithm itself is as follows - at the beginning use existing selection algorithm (based on favoring, active choices, etc.) to select the first disjunction to attempt, and push it to the stack of "selected" disjunctions; next time solver requests a disjunction, use the last selected one to pick the closest disjunction to it in the AST order preferring parents over children. For example: ``` + / \ * Float( fast}/rdar23682605.swift | 1 - .../rdar46713933_literal_arg.swift | 1 - .../Sema/type_checker_perf/fast/sr10130.swift | 16 ++ 6 files changed, 190 insertions(+), 4 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/mixed_double_and_float_operators.swift rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar23682605.swift (91%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar46713933_literal_arg.swift (85%) create mode 100644 validation-test/Sema/type_checker_perf/fast/sr10130.swift diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index c07b62567905a..f396285b30c87 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2171,6 +2171,168 @@ Constraint *ConstraintSystem::selectDisjunction() { if (disjunctions.empty()) return nullptr; + // If there are only a few disjunctions available, + // let's just use selection alogirthm. + if (disjunctions.size() <= 2) + return selectBestDisjunction(disjunctions); + + if (SelectedDisjunctions.empty()) + return selectBestDisjunction(disjunctions); + + auto *lastDisjunction = SelectedDisjunctions.back()->getLocator(); + + // First, let's built a dictionary of all disjunctions accessible + // via their anchoring expressions. + llvm::SmallDenseMap anchoredDisjunctions; + for (auto *disjunction : disjunctions) { + if (auto anchor = simplifyLocatorToAnchor(disjunction->getLocator())) + anchoredDisjunctions.insert({anchor, disjunction}); + } + + auto lookupDisjunctionInCache = + [&anchoredDisjunctions](Expr *expr) -> Constraint * { + auto disjunction = anchoredDisjunctions.find(expr); + return disjunction != anchoredDisjunctions.end() ? disjunction->second + : nullptr; + }; + + auto findDisjunction = [&](Expr *expr) -> Constraint * { + if (!expr || !(isa(expr) || isa(expr))) + return nullptr; + + // For applications i.e. calls, let's match their function first. + if (auto *apply = dyn_cast(expr)) { + if (auto disjunction = lookupDisjunctionInCache(apply->getFn())) + return disjunction; + } + + return lookupDisjunctionInCache(expr); + }; + + auto findClosestDisjunction = [&](Expr *expr) -> Constraint * { + Constraint *selectedDisjunction = nullptr; + expr->forEachChildExpr([&](Expr *expr) -> Expr * { + if (auto *disjunction = findDisjunction(expr)) { + selectedDisjunction = disjunction; + return nullptr; + } + return expr; + }); + return selectedDisjunction; + }; + + if (auto *expr = getAsExpr(lastDisjunction->getAnchor())) { + // If this disjunction is derived from an overload set expression, + // let's look one level higher since its immediate parent is an + // operator application. + if (isa(expr)) + expr = getParentExpr(expr); + + bool isMemberRef = isa(expr); + + // Implicit `.init` calls need some special handling. + if (lastDisjunction->isLastElement()) { + if (auto *call = dyn_cast(expr)) { + expr = call->getFn(); + isMemberRef = true; + } + } + + if (isMemberRef) { + auto *parent = getParentExpr(expr); + // If this is a member application e.g. `.test(...)`, + // then let's see whether one of the arguments is a + // closure and if so, select the "best" disjunction + // from its body to be attempted next. This helps to + // type-check operator chains in a freshly resolved + // closure before moving to the next member in that + // chain of expressions for example: + // + // arr.map { $0 + 1 * 3 ... }.filter { ... }.reduce(0, +) + // + // Attempting to solve the body of the `.map` right after + // it has been selected helps to split up the constraint + // system. + if (auto *call = dyn_cast_or_null(parent)) { + if (auto *arguments = call->getArgs()) { + for (const auto &argument : *arguments) { + auto *argExpr = argument.getExpr()->getSemanticsProvidingExpr(); + auto *closure = dyn_cast(argExpr); + // Even if the body of this closure participates in type-check + // it would be handled one statement at a time via a conjunction + // constraint. + if (!(closure && closure->hasSingleExpressionBody())) + continue; + + // Note that it's important that we select the best possible + // disjunction from the body of the closure first, it helps + // to prune the solver space. + SmallVector innerDisjunctions; + + for (auto *disjunction : disjunctions) { + auto *choice = disjunction->getNestedConstraints()[0]; + if (choice->getKind() == ConstraintKind::BindOverload && + choice->getOverloadUseDC() == closure) + innerDisjunctions.push_back(disjunction); + } + + if (!innerDisjunctions.empty()) + return selectBestDisjunction(innerDisjunctions); + } + } + } + } + + // First, let's see whether there is a direct parent in scope, since + // parent is the one which is going to use the result type of the + // last disjunction. + if (auto *parent = getParentExpr(expr)) { + if (isMemberRef && isa(parent)) + parent = getParentExpr(parent); + + if (auto disjunction = findDisjunction(parent)) + return disjunction; + + // If parent is a tuple, let's collect disjunctions associated + // with its elements and run selection algorithm on them. + if (auto *tuple = dyn_cast_or_null(parent)) { + auto *elementExpr = expr; + + // If current element has any unsolved disjunctions, let's + // attempt the closest to keep solving the local element. + if (auto disjunction = findClosestDisjunction(elementExpr)) + return disjunction; + + SmallVector tupleDisjunctions; + // Find all of the disjunctions that are nested inside of + // the current tuple for selection. + for (auto *disjunction : disjunctions) { + auto anchor = disjunction->getLocator()->getAnchor(); + if (auto *expr = getAsExpr(anchor)) { + while ((expr = getParentExpr(expr))) { + if (expr == tuple) { + tupleDisjunctions.push_back(disjunction); + break; + } + } + } + } + + // Let's use a pool of all disjunctions associated with + // this tuple. Picking the best one, regardless of the + // element would stir solving towards solving everything + // in a particular element. + if (!tupleDisjunctions.empty()) + return selectBestDisjunction(tupleDisjunctions); + } + } + + // If parent is not available (e.g. because it's already bound), + // let's look into the arguments, and find the closest unbound one. + if (auto *closestDisjunction = findClosestDisjunction(expr)) + return closestDisjunction; + } + return selectBestDisjunction(disjunctions); } diff --git a/test/IDE/complete_ambiguous.swift b/test/IDE/complete_ambiguous.swift index 628e333dcf1c4..62f1177c48fe3 100644 --- a/test/IDE/complete_ambiguous.swift +++ b/test/IDE/complete_ambiguous.swift @@ -448,8 +448,8 @@ struct Struct123: Equatable { } func testBestSolutionFilter() { let a = Struct123(); - let b = [Struct123]().first(where: { $0 == a && 1 + 90 * 5 / 8 == 45 * -10 })?.structMem != .#^BEST_SOLUTION_FILTER?xfail=rdar73282163^# - let c = min(10.3, 10 / 10.4) < 6 / 7 ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2?check=BEST_SOLUTION_FILTER;xfail=rdar73282163^# + let b = [Struct123]().first(where: { $0 == a && 1 + 90 * 5 / 8 == 45 * -10 })?.structMem != .#^BEST_SOLUTION_FILTER^# + min(10.3, 10 / 10.4) < 6 / 7 ? true : Optional(a)?.structMem != .#^BEST_SOLUTION_FILTER2?check=BEST_SOLUTION_FILTER^# } // BEST_SOLUTION_FILTER: Begin completions diff --git a/validation-test/Sema/type_checker_perf/fast/mixed_double_and_float_operators.swift b/validation-test/Sema/type_checker_perf/fast/mixed_double_and_float_operators.swift new file mode 100644 index 0000000000000..666033c8eb7ab --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/mixed_double_and_float_operators.swift @@ -0,0 +1,10 @@ +// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -solver-disable-shrink +// REQUIRES: OS=macosx,tools-release,no_asan + +import Foundation + +var r: Float = 0 +var x: Double = 0 +var y: Double = 0 + +let _ = (1.0 - 1.0 / (1.0 + exp(-5.0 * (r - 0.05)/0.01))) * Float(x) + Float(y) diff --git a/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift b/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift similarity index 91% rename from validation-test/Sema/type_checker_perf/slow/rdar23682605.swift rename to validation-test/Sema/type_checker_perf/fast/rdar23682605.swift index b7bc757fe1989..4be53b4d2ef83 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift @@ -14,7 +14,6 @@ func memoize( body: @escaping ((T)->U, T)->U ) -> (T)->U { } let fibonacci = memoize { - // expected-error@-1 {{reasonable time}} fibonacci, n in n < 2 ? Double(n) : fibonacci(n - 1) + fibonacci(n - 2) } diff --git a/validation-test/Sema/type_checker_perf/slow/rdar46713933_literal_arg.swift b/validation-test/Sema/type_checker_perf/fast/rdar46713933_literal_arg.swift similarity index 85% rename from validation-test/Sema/type_checker_perf/slow/rdar46713933_literal_arg.swift rename to validation-test/Sema/type_checker_perf/fast/rdar46713933_literal_arg.swift index a0628335b9c36..5256a92a787c7 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar46713933_literal_arg.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar46713933_literal_arg.swift @@ -8,5 +8,4 @@ func wrap(_ key: String, _ value: T) -> T { retur func wrapped() -> Int { return wrap("1", 1) + wrap("1", 1) + wrap("1", 1) + wrap("1", 1) + wrap("1", 1) + wrap("1", 1) - // expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}} } diff --git a/validation-test/Sema/type_checker_perf/fast/sr10130.swift b/validation-test/Sema/type_checker_perf/fast/sr10130.swift new file mode 100644 index 0000000000000..91e48d96eac91 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/sr10130.swift @@ -0,0 +1,16 @@ +// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 +// REQUIRES: tools-release,no_asan + +import Foundation + +let itemsPerRow = 10 +let size: CGFloat = 20 +let margin: CGFloat = 10 + +let _ = (0..<100).map { (row: CGFloat($0 / itemsPerRow), col: CGFloat($0 % itemsPerRow)) } + .map { + CGRect(x: $0.col * (size + margin) + margin, + y: $0.row * (size + margin) + margin, + width: size, + height: size) + } From 7c66f70337a66d6633f913b50851c2e21a0359c5 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 8 Jan 2022 12:23:09 -0800 Subject: [PATCH 06/13] [ConstraintSystem] Don't infer common result type from disabled overload choices --- lib/Sema/CSSimplify.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 58b7d97beab2c..da976e43b3544 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -10405,6 +10405,24 @@ bool ConstraintSystem::simplifyAppliedOverloadsImpl( } } + // Disabled overloads need special handling depending mode. + if (constraint->isDisabled()) { + // In diagnostic mode, invalidate previous common result type if + // current overload choice has a fix to make sure that we produce + // the best diagnostics possible. + if (shouldAttemptFixes()) { + if (constraint->getFix()) + commonResultType = ErrorType::get(getASTContext()); + return true; + } + + // In performance mode, let's skip the disabled overload choice + // and continue - this would make sure that common result type + // could be found if one exists among the overloads the solver + // is actually going to attempt. + return false; + } + // Determine the type that this choice will have. Type choiceType = getEffectiveOverloadType( constraint->getLocator(), choice, /*allowMembers=*/true, From b9aac87f7ed9b46661e07916a924a3de4d6f76d2 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 8 Jan 2022 13:29:26 -0800 Subject: [PATCH 07/13] [ConstraintSystem] Exclude not-applicable members (e.g. properties) from common result compulation This is the situation where a property has the same name as a method e.g. ```swift protocol P { var test: String { get } } extension P { var test: String { get { return "" } } } struct S : P { func test() -> Int { 42 } } var s = S() s.test() // disjunction would have two choices here, one // for the property from `P` and one for the method of `S`. ``` In cases like this, let's exclude property overload from common result determination because it cannot be applied. Note that such overloads cannot be disabled, because they still have to be checked in diagnostic mode and there is (currently) no way to re-enable them for diagnostics. --- lib/Sema/CSSimplify.cpp | 28 +++++++++++++++++ ...perty_and_methods_with_same_name.swift.gyb | 30 +++++++++++++++++++ 2 files changed, 58 insertions(+) create mode 100644 validation-test/Sema/type_checker_perf/fast/property_and_methods_with_same_name.swift.gyb diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index da976e43b3544..bb99176dec4f3 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -10432,6 +10432,34 @@ bool ConstraintSystem::simplifyAppliedOverloadsImpl( return true; } + // This is the situation where a property has the same name + // as a method e.g. + // + // protocol P { + // var test: String { get } + // } + // + // extension P { + // var test: String { get { return "" } } + // } + // + // struct S : P { + // func test() -> Int { 42 } + // } + // + // var s = S() + // s.test() <- disjunction would have two choices here, one + // for the property from `P` and one for the method of `S`. + // + // In cases like this, let's exclude property overload from common + // result determination because it cannot be applied. + // + // Note that such overloads cannot be disabled, because they still + // have to be checked in diagnostic mode and there is (currently) + // no way to re-enable them for diagnostics. + if (!choiceType->lookThroughAllOptionalTypes()->is()) + return true; + // If types lined up exactly, let's favor this overload choice. if (Type(argFnType)->isEqual(choiceType)) constraint->setFavored(); diff --git a/validation-test/Sema/type_checker_perf/fast/property_and_methods_with_same_name.swift.gyb b/validation-test/Sema/type_checker_perf/fast/property_and_methods_with_same_name.swift.gyb new file mode 100644 index 0000000000000..e102c55553f24 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/property_and_methods_with_same_name.swift.gyb @@ -0,0 +1,30 @@ +// RUN: %scale-test --begin 1 --end 10 --step 1 --select NumLeafScopes %s +// REQUIRES: asserts,no_asan + +func test(_: [A]) {} + +class A {} + +protocol P { + var arr: Int { get } +} + +extension P { + var arr: Int { get { return 42 } } +} + +class S : P { + func arr() -> [A] { [] } + func arr(_: Int = 42) -> [A] { [] } +} + +// There is a clash between `arr` property and `arr` methods +// returning `[A]` which shouldn't prevent "common result" +// determination. +func run_test(s: S) { + test(s.arr() + + %for i in range(0, N): + s.arr() + + %end + s.arr()) +} From f497956f9d3f9a601301b6a8dad39589859d9a1f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 9 Jan 2022 22:16:17 -0800 Subject: [PATCH 08/13] [CSSolver] Partition arithmetic operators upfront --- include/swift/Sema/ConstraintSystem.h | 24 --- lib/Sema/CSSolver.cpp | 230 ++++++++++++-------------- lib/Sema/CSStep.cpp | 6 - 3 files changed, 108 insertions(+), 152 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index fde6590168a02..5c95d75671e5f 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5847,8 +5847,6 @@ class DisjunctionChoiceProducer : public BindingProducer { unsigned Index = 0; - bool needsGenericOperatorOrdering = true; - public: using Element = DisjunctionChoice; @@ -5866,10 +5864,6 @@ class DisjunctionChoiceProducer : public BindingProducer { partitionDisjunction(Ordering, PartitionBeginning); } - void setNeedsGenericOperatorOrdering(bool flag) { - needsGenericOperatorOrdering = flag; - } - Optional operator()() override { if (isExhausted()) return None; @@ -5882,18 +5876,6 @@ class DisjunctionChoiceProducer : public BindingProducer { ++Index; - auto choice = DisjunctionChoice(CS, currIndex, Choices[Ordering[currIndex]], - IsExplicitConversion, isBeginningOfPartition); - // Partition the generic operators before producing the first generic - // operator disjunction choice. - if (needsGenericOperatorOrdering && choice.isGenericOperator()) { - unsigned nextPartitionIndex = (PartitionIndex < PartitionBeginning.size() ? - PartitionBeginning[PartitionIndex] : Ordering.size()); - partitionGenericOperators(Ordering.begin() + currIndex, - Ordering.begin() + nextPartitionIndex); - needsGenericOperatorOrdering = false; - } - return DisjunctionChoice(CS, currIndex, Choices[Ordering[currIndex]], IsExplicitConversion, isBeginningOfPartition); } @@ -5908,12 +5890,6 @@ class DisjunctionChoiceProducer : public BindingProducer { // have to visit all of the options. void partitionDisjunction(SmallVectorImpl &Ordering, SmallVectorImpl &PartitionBeginning); - - /// Partition the choices in the range \c first to \c last into groups and - /// order the groups in the best order to attempt based on the argument - /// function type that the operator is applied to. - void partitionGenericOperators(SmallVectorImpl::iterator first, - SmallVectorImpl::iterator last); }; class ConjunctionElementProducer : public BindingProducer { diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index f396285b30c87..8d63bb544ca69 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1933,118 +1933,6 @@ static void existingOperatorBindingsForDisjunction(ConstraintSystem &CS, } } -void DisjunctionChoiceProducer::partitionGenericOperators( - SmallVectorImpl::iterator first, - SmallVectorImpl::iterator last) { - auto *argFnType = CS.getAppliedDisjunctionArgumentFunction(Disjunction); - if (!isOperatorDisjunction(Disjunction) || !argFnType) - return; - - auto operatorName = Choices[0]->getOverloadChoice().getName(); - if (!operatorName.getBaseIdentifier().isArithmeticOperator()) - return; - - SmallVector concreteOverloads; - SmallVector numericOverloads; - SmallVector sequenceOverloads; - SmallVector simdOverloads; - SmallVector otherGenericOverloads; - - auto refinesOrConformsTo = [&](NominalTypeDecl *nominal, KnownProtocolKind kind) -> bool { - if (!nominal) - return false; - - auto *protocol = - TypeChecker::getProtocol(CS.getASTContext(), SourceLoc(), kind); - - if (auto *refined = dyn_cast(nominal)) - return refined->inheritsFrom(protocol); - - return (bool)TypeChecker::conformsToProtocol(nominal->getDeclaredType(), protocol, - CS.DC->getParentModule()); - }; - - // Gather Numeric and Sequence overloads into separate buckets. - for (auto iter = first; iter != last; ++iter) { - unsigned index = *iter; - auto *decl = Choices[index]->getOverloadChoice().getDecl(); - auto *nominal = decl->getDeclContext()->getSelfNominalTypeDecl(); - - if (isSIMDOperator(decl)) { - simdOverloads.push_back(index); - } else if (!decl->getInterfaceType()->is()) { - concreteOverloads.push_back(index); - } else if (refinesOrConformsTo(nominal, KnownProtocolKind::AdditiveArithmetic)) { - numericOverloads.push_back(index); - } else if (refinesOrConformsTo(nominal, KnownProtocolKind::Sequence)) { - sequenceOverloads.push_back(index); - } else { - otherGenericOverloads.push_back(index); - } - } - - auto sortPartition = [&](SmallVectorImpl &partition) { - llvm::sort(partition, [&](unsigned lhs, unsigned rhs) -> bool { - auto *declA = - dyn_cast(Choices[lhs]->getOverloadChoice().getDecl()); - auto *declB = - dyn_cast(Choices[rhs]->getOverloadChoice().getDecl()); - - return TypeChecker::isDeclRefinementOf(declA, declB); - }); - }; - - // Sort sequence overloads so that refinements are attempted first. - // If the solver finds a solution with an overload, it can then skip - // subsequent choices that the successful choice is a refinement of. - sortPartition(sequenceOverloads); - - // Attempt concrete overloads first. - first = std::copy(concreteOverloads.begin(), concreteOverloads.end(), first); - - // Check if any of the known argument types conform to one of the standard - // arithmetic protocols. If so, the sovler should attempt the corresponding - // overload choices first. - for (auto arg : argFnType->getParams()) { - auto argType = arg.getPlainType(); - argType = CS.getFixedTypeRecursive(argType, /*wantRValue=*/true); - - if (argType->isTypeVariableOrMember()) - continue; - - if (TypeChecker::conformsToKnownProtocol( - argType, KnownProtocolKind::AdditiveArithmetic, - CS.DC->getParentModule())) { - first = - std::copy(numericOverloads.begin(), numericOverloads.end(), first); - numericOverloads.clear(); - break; - } - - if (TypeChecker::conformsToKnownProtocol( - argType, KnownProtocolKind::Sequence, - CS.DC->getParentModule())) { - first = - std::copy(sequenceOverloads.begin(), sequenceOverloads.end(), first); - sequenceOverloads.clear(); - break; - } - - if (TypeChecker::conformsToKnownProtocol( - argType, KnownProtocolKind::SIMD, - CS.DC->getParentModule())) { - first = std::copy(simdOverloads.begin(), simdOverloads.end(), first); - simdOverloads.clear(); - break; - } - } - - first = std::copy(otherGenericOverloads.begin(), otherGenericOverloads.end(), first); - first = std::copy(numericOverloads.begin(), numericOverloads.end(), first); - first = std::copy(sequenceOverloads.begin(), sequenceOverloads.end(), first); - first = std::copy(simdOverloads.begin(), simdOverloads.end(), first); -} - void DisjunctionChoiceProducer::partitionDisjunction( SmallVectorImpl &Ordering, SmallVectorImpl &PartitionBeginning) { @@ -2081,8 +1969,14 @@ void DisjunctionChoiceProducer::partitionDisjunction( // First collect some things that we'll generally put near the beginning or // end of the partitioning. SmallVector favored; - SmallVector everythingElse; + // start - Operator section + SmallVector concreteOperators; + SmallVector numericOperators; + SmallVector sequenceOperators; SmallVector simdOperators; + SmallVector genericOperators; + // end - operator section + SmallVector everythingElse; SmallVector disabled; SmallVector unavailable; @@ -2126,17 +2020,64 @@ void DisjunctionChoiceProducer::partitionDisjunction( }); } - // Partition SIMD operators. - if (isOperatorDisjunction(Disjunction) && - !Choices[0]->getOverloadChoice().getName().getBaseIdentifier().isArithmeticOperator()) { - forEachChoice(Choices, [&](unsigned index, Constraint *constraint) -> bool { - if (isSIMDOperator(constraint->getOverloadChoice().getDecl())) { + bool isArithmeticOperator = false; + + if (isOperatorDisjunction(Disjunction)) { + auto operatorName = Choices[0]->getOverloadChoice().getName(); + isArithmeticOperator = + operatorName.getBaseIdentifier().isArithmeticOperator(); + } + + if (isArithmeticOperator) { + auto refinesOrConformsTo = [&](NominalTypeDecl *nominal, + KnownProtocolKind kind) -> bool { + if (!nominal) + return false; + + auto *protocol = + TypeChecker::getProtocol(CS.getASTContext(), SourceLoc(), kind); + + if (auto *refined = dyn_cast(nominal)) + return refined->inheritsFrom(protocol); + + return (bool)TypeChecker::conformsToProtocol( + nominal->getDeclaredType(), protocol, CS.DC->getParentModule()); + }; + + forEachChoice(Choices, [&](unsigned index, Constraint *choice) -> bool { + auto *decl = choice->getOverloadChoice().getDecl(); + auto *nominal = decl->getDeclContext()->getSelfNominalTypeDecl(); + + if (isSIMDOperator(decl)) { simdOperators.push_back(index); - return true; + } else if (!decl->getInterfaceType()->is()) { + concreteOperators.push_back(index); + } else if (refinesOrConformsTo(nominal, + KnownProtocolKind::AdditiveArithmetic)) { + numericOperators.push_back(index); + } else if (refinesOrConformsTo(nominal, KnownProtocolKind::Sequence)) { + sequenceOperators.push_back(index); + } else { + genericOperators.push_back(index); } - - return false; + return true; }); + + auto sortPartition = [&](SmallVectorImpl &partition) { + llvm::sort(partition, [&](unsigned lhs, unsigned rhs) -> bool { + auto *declA = + dyn_cast(Choices[lhs]->getOverloadChoice().getDecl()); + auto *declB = + dyn_cast(Choices[rhs]->getOverloadChoice().getDecl()); + + return TypeChecker::isDeclRefinementOf(declA, declB); + }); + }; + + // Sort sequence overloads so that refinements are attempted first. + // If the solver finds a solution with an overload, it can then skip + // subsequent choices that the successful choice is a refinement of. + sortPartition(sequenceOperators); } // Gather the remaining options. @@ -2156,8 +2097,53 @@ void DisjunctionChoiceProducer::partitionDisjunction( }; appendPartition(favored); + + if (isArithmeticOperator) { + appendPartition(concreteOperators); + + if (auto *argFnType = CS.getAppliedDisjunctionArgumentFunction(Disjunction)) { + // Check if any of the known argument types conform to one of the standard + // arithmetic protocols. If so, the solver should attempt the + // corresponding overload choices first. + for (auto arg : argFnType->getParams()) { + auto argType = arg.getPlainType(); + argType = CS.getFixedTypeRecursive(argType, /*wantRValue=*/true); + + if (argType->isTypeVariableOrMember()) + continue; + + if (TypeChecker::conformsToKnownProtocol( + argType, KnownProtocolKind::AdditiveArithmetic, + CS.DC->getParentModule())) { + appendPartition(numericOperators); + numericOperators.clear(); + break; + } + + if (TypeChecker::conformsToKnownProtocol(argType, + KnownProtocolKind::Sequence, + CS.DC->getParentModule())) { + appendPartition(sequenceOperators); + sequenceOperators.clear(); + break; + } + + if (TypeChecker::conformsToKnownProtocol( + argType, KnownProtocolKind::SIMD, CS.DC->getParentModule())) { + appendPartition(simdOperators); + simdOperators.clear(); + break; + } + } + } + + appendPartition(genericOperators); + appendPartition(numericOperators); + appendPartition(sequenceOperators); + appendPartition(simdOperators); + } + appendPartition(everythingElse); - appendPartition(simdOperators); appendPartition(unavailable); appendPartition(disabled); diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index d534fbec95144..f6dd6245404c6 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -546,12 +546,6 @@ StepResult DisjunctionStep::resume(bool prevFailed) { if (!choice.isGenericOperator() && choice.isSymmetricOperator()) { if (!BestNonGenericScore || score < BestNonGenericScore) { BestNonGenericScore = score; - if (shouldSkipGenericOperators()) { - // The disjunction choice producer shouldn't do the work - // to partition the generic operator choices if generic - // operators are going to be skipped. - Producer.setNeedsGenericOperatorOrdering(false); - } } } From 59fd40a557c381dc99ea262905c88aab1f9f2678 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 9 Jan 2022 22:21:31 -0800 Subject: [PATCH 09/13] [CSSolver] Introduce a new partition for partially specialized generic operators --- lib/Sema/CSSolver.cpp | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 8d63bb544ca69..f6438d743d870 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1971,6 +1971,7 @@ void DisjunctionChoiceProducer::partitionDisjunction( SmallVector favored; // start - Operator section SmallVector concreteOperators; + SmallVector partiallySpecializedOperators; SmallVector numericOperators; SmallVector sequenceOperators; SmallVector simdOperators; @@ -2044,6 +2045,36 @@ void DisjunctionChoiceProducer::partitionDisjunction( nominal->getDeclaredType(), protocol, CS.DC->getParentModule()); }; + auto isPartiallySpecialized = [&](ValueDecl *choice) -> bool { + auto choiceType = choice->getInterfaceType(); + + auto *fnType = choiceType->getAs(); + if (!(fnType && fnType->is())) + return false; + + if (choice->getDeclContext()->getSelfNominalTypeDecl()) + fnType = fnType->getResult()->castTo(); + + // Type has to be either bound generic e.g. `S`, + // or unbound generic e.g. `Array`, or concrete. + auto isAcceptableType = [&](Type type) { + if (auto *UGT = type->getAs()) + return isa(UGT->getDecl()); + + return type->is() || + !(type->hasTypeParameter() || type->hasDependentMember()); + }; + + if (llvm::all_of(fnType->getParams(), + [&](const AnyFunctionType::Param ¶m) { + return isAcceptableType(param.getPlainType()); + })) { + return isAcceptableType(fnType->getResult()); + } + + return false; + }; + forEachChoice(Choices, [&](unsigned index, Constraint *choice) -> bool { auto *decl = choice->getOverloadChoice().getDecl(); auto *nominal = decl->getDeclContext()->getSelfNominalTypeDecl(); @@ -2052,6 +2083,8 @@ void DisjunctionChoiceProducer::partitionDisjunction( simdOperators.push_back(index); } else if (!decl->getInterfaceType()->is()) { concreteOperators.push_back(index); + } else if (isPartiallySpecialized(decl)) { + partiallySpecializedOperators.push_back(index); } else if (refinesOrConformsTo(nominal, KnownProtocolKind::AdditiveArithmetic)) { numericOperators.push_back(index); @@ -2100,6 +2133,7 @@ void DisjunctionChoiceProducer::partitionDisjunction( if (isArithmeticOperator) { appendPartition(concreteOperators); + appendPartition(partiallySpecializedOperators); if (auto *argFnType = CS.getAppliedDisjunctionArgumentFunction(Disjunction)) { // Check if any of the known argument types conform to one of the standard From 8819f80addce31fc6e0e62f2a6dc15cf3eadb382 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 9 Jan 2022 22:24:06 -0800 Subject: [PATCH 10/13] [CSSolver] Don't consider other operator choices while partitioning --- lib/Sema/CSSolver.cpp | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index f6438d743d870..cefc775226ab9 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1981,12 +1981,6 @@ void DisjunctionChoiceProducer::partitionDisjunction( SmallVector disabled; SmallVector unavailable; - // Add existing operator bindings to the main partition first. This often - // helps the solver find a solution fast. - existingOperatorBindingsForDisjunction(CS, Choices, everythingElse); - for (auto index : everythingElse) - taken.insert(Choices[index]); - // First collect disabled and favored constraints. forEachChoice(Choices, [&](unsigned index, Constraint *constraint) -> bool { if (constraint->isDisabled()) { From b7233e56c67db9e8618d14e7b84f525800299f8d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 9 Jan 2022 22:24:37 -0800 Subject: [PATCH 11/13] [ConstraintSystem] Remove `LinkedExprAnalyzer` --- lib/Sema/CSGen.cpp | 321 +-------------------------------------------- 1 file changed, 1 insertion(+), 320 deletions(-) diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index fec7d7acd7cca..c04f2303b5b59 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -73,314 +73,6 @@ static bool mergeRepresentativeEquivalenceClasses(ConstraintSystem &CS, } namespace { - - /// Internal struct for tracking information about types within a series - /// of "linked" expressions. (Such as a chain of binary operator invocations.) - struct LinkedTypeInfo { - bool hasLiteral = false; - - llvm::SmallSet collectedTypes; - llvm::SmallVector binaryExprs; - }; - - /// Walks an expression sub-tree, and collects information about expressions - /// whose types are mutually dependent upon one another. - class LinkedExprCollector : public ASTWalker { - - llvm::SmallVectorImpl &LinkedExprs; - ConstraintSystem &CS; - - public: - LinkedExprCollector(llvm::SmallVectorImpl &linkedExprs, - ConstraintSystem &cs) - : LinkedExprs(linkedExprs), CS(cs) {} - - std::pair walkToExprPre(Expr *expr) override { - - if (CS.shouldReusePrecheckedType() && - !CS.getType(expr)->hasTypeVariable()) { - return { false, expr }; - } - - if (isa(expr)) - return {false, expr}; - - // Store top-level binary exprs for further analysis. - if (isa(expr) || - - // Literal exprs are contextually typed, so store them off as well. - isa(expr) || - - // We'd like to look at the elements of arrays and dictionaries. - isa(expr) || - isa(expr) || - - // assignment expression can involve anonymous closure parameters - // as source and destination, so it's beneficial for diagnostics if - // we look at the assignment. - isa(expr)) { - LinkedExprs.push_back(expr); - return {false, expr}; - } - - return { true, expr }; - } - - Expr *walkToExprPost(Expr *expr) override { - return expr; - } - - /// Ignore statements. - std::pair walkToStmtPre(Stmt *stmt) override { - return { false, stmt }; - } - - /// Ignore declarations. - bool walkToDeclPre(Decl *decl) override { return false; } - - /// Ignore patterns. - std::pair walkToPatternPre(Pattern *pat) override { - return { false, pat }; - } - - /// Ignore types. - bool walkToTypeReprPre(TypeRepr *T) override { return false; } - }; - - /// Given a collection of "linked" expressions, analyzes them for - /// commonalities regarding their types. This will help us compute a - /// "best common type" from the expression types. - class LinkedExprAnalyzer : public ASTWalker { - - LinkedTypeInfo <I; - ConstraintSystem &CS; - - public: - - LinkedExprAnalyzer(LinkedTypeInfo <i, ConstraintSystem &cs) : - LTI(lti), CS(cs) {} - - std::pair walkToExprPre(Expr *expr) override { - - if (CS.shouldReusePrecheckedType() && - !CS.getType(expr)->hasTypeVariable()) { - return { false, expr }; - } - - if (isa(expr)) { - LTI.hasLiteral = true; - return { false, expr }; - } - - if (isa(expr)) { - return { true, expr }; - } - - if (auto UDE = dyn_cast(expr)) { - - if (CS.hasType(UDE)) - LTI.collectedTypes.insert(CS.getType(UDE).getPointer()); - - // Don't recurse into the base expression. - return { false, expr }; - } - - - if (isa(expr)) { - return {false, expr}; - } - - if (auto FVE = dyn_cast(expr)) { - LTI.collectedTypes.insert(CS.getType(FVE).getPointer()); - return { false, expr }; - } - - if (auto DRE = dyn_cast(expr)) { - if (auto varDecl = dyn_cast(DRE->getDecl())) { - if (CS.hasType(DRE)) { - LTI.collectedTypes.insert(CS.getType(DRE).getPointer()); - } - return { false, expr }; - } - } - - // In the case of a function application, we would have already captured - // the return type during constraint generation, so there's no use in - // looking any further. - if (isa(expr) && - !(isa(expr) || isa(expr) || - isa(expr))) { - return { false, expr }; - } - - if (auto *binaryExpr = dyn_cast(expr)) { - LTI.binaryExprs.push_back(binaryExpr); - } - - if (auto favoredType = CS.getFavoredType(expr)) { - LTI.collectedTypes.insert(favoredType); - - return { false, expr }; - } - - // Optimize branches of a conditional expression separately. - if (auto IE = dyn_cast(expr)) { - CS.optimizeConstraints(IE->getCondExpr()); - CS.optimizeConstraints(IE->getThenExpr()); - CS.optimizeConstraints(IE->getElseExpr()); - return { false, expr }; - } - - // For exprs of a tuple, avoid favoring. (We need to allow for cases like - // (Int, Int32).) - if (isa(expr)) { - return { false, expr }; - } - - // Coercion exprs have a rigid type, so there's no use in gathering info - // about them. - if (auto *coercion = dyn_cast(expr)) { - // Let's not collect information about types initialized by - // coercions just like we don't for regular initializer calls, - // because that might lead to overly eager type variable merging. - if (!coercion->isLiteralInit()) - LTI.collectedTypes.insert(CS.getType(expr).getPointer()); - return { false, expr }; - } - - // Don't walk into subscript expressions - to do so would risk factoring - // the index expression into edge contraction. (We don't want to do this - // if the index expression is a literal type that differs from the return - // type of the subscript operation.) - if (isa(expr) || isa(expr)) { - return { false, expr }; - } - - // Don't walk into unresolved member expressions - we avoid merging type - // variables inside UnresolvedMemberExpr and those outside, since they - // should be allowed to behave independently in CS. - if (isa(expr)) { - return {false, expr }; - } - - return { true, expr }; - } - - /// Ignore statements. - std::pair walkToStmtPre(Stmt *stmt) override { - return { false, stmt }; - } - - /// Ignore declarations. - bool walkToDeclPre(Decl *decl) override { return false; } - - /// Ignore patterns. - std::pair walkToPatternPre(Pattern *pat) override { - return { false, pat }; - } - - /// Ignore types. - bool walkToTypeReprPre(TypeRepr *T) override { return false; } - }; - - /// For a given expression, given information that is global to the - /// expression, attempt to derive a favored type for it. - void computeFavoredTypeForExpr(Expr *expr, ConstraintSystem &CS) { - LinkedTypeInfo lti; - - expr->walk(LinkedExprAnalyzer(lti, CS)); - - // Check whether we can proceed with favoring. - if (llvm::any_of(lti.binaryExprs, [](const BinaryExpr *op) { - auto *ODRE = dyn_cast(op->getFn()); - if (!ODRE) - return false; - - // Attempting to favor based on operand types is wrong for - // nil-coalescing operator. - auto identifier = ODRE->getDecls().front()->getBaseIdentifier(); - return identifier.isNilCoalescingOperator(); - })) { - return; - } - - if (lti.collectedTypes.size() == 1) { - // TODO: Compute the BCT. - - // It's only useful to favor the type instead of - // binding it directly to arguments/result types, - // which means in case it has been miscalculated - // solver can still make progress. - auto favoredTy = (*lti.collectedTypes.begin())->getWithoutSpecifierType(); - CS.setFavoredType(expr, favoredTy.getPointer()); - - // If we have a chain of identical binop expressions with homogeneous - // argument types, we can directly simplify the associated constraint - // graph. - auto simplifyBinOpExprTyVars = [&]() { - // Don't attempt to do linking if there are - // literals intermingled with other inferred types. - if (lti.hasLiteral) - return; - - for (auto binExp1 : lti.binaryExprs) { - for (auto binExp2 : lti.binaryExprs) { - if (binExp1 == binExp2) - continue; - - auto fnTy1 = CS.getType(binExp1)->getAs(); - auto fnTy2 = CS.getType(binExp2)->getAs(); - - if (!(fnTy1 && fnTy2)) - return; - - auto ODR1 = dyn_cast(binExp1->getFn()); - auto ODR2 = dyn_cast(binExp2->getFn()); - - if (!(ODR1 && ODR2)) - return; - - // TODO: We currently limit this optimization to known arithmetic - // operators, but we should be able to broaden this out to - // logical operators as well. - if (!isArithmeticOperatorDecl(ODR1->getDecls()[0])) - return; - - if (ODR1->getDecls()[0]->getBaseName() != - ODR2->getDecls()[0]->getBaseName()) - return; - - // All things equal, we can merge the tyvars for the function - // types. - auto rep1 = CS.getRepresentative(fnTy1); - auto rep2 = CS.getRepresentative(fnTy2); - - if (rep1 != rep2) { - CS.mergeEquivalenceClasses(rep1, rep2, - /*updateWorkList*/ false); - } - - auto odTy1 = CS.getType(ODR1)->getAs(); - auto odTy2 = CS.getType(ODR2)->getAs(); - - if (odTy1 && odTy2) { - auto odRep1 = CS.getRepresentative(odTy1); - auto odRep2 = CS.getRepresentative(odTy2); - - // Since we'll be choosing the same overload, we can merge - // the overload tyvar as well. - if (odRep1 != odRep2) - CS.mergeEquivalenceClasses(odRep1, odRep2, - /*updateWorkList*/ false); - } - } - } - }; - - simplifyBinOpExprTyVars(); - } - } - /// Determine whether the given parameter type and argument should be /// "favored" because they match exactly. bool isFavoredParamAndArg(ConstraintSystem &CS, Type paramTy, Type argTy, @@ -4230,18 +3922,7 @@ ConstraintSystem::applyPropertyWrapperToParameter( void ConstraintSystem::optimizeConstraints(Expr *e) { if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks) return; - - SmallVector linkedExprs; - - // Collect any linked expressions. - LinkedExprCollector collector(linkedExprs, *this); - e->walk(collector); - - // Favor types, as appropriate. - for (auto linkedExpr : linkedExprs) { - computeFavoredTypeForExpr(linkedExpr, *this); - } - + // Optimize the constraints. ConstraintOptimizer optimizer(*this); e->walk(optimizer); From b64f1fa25dca6fcf7d0fb7aeda13671540356630 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 9 Jan 2022 22:25:25 -0800 Subject: [PATCH 12/13] [TypeChecker] NFC: Mark perf test-case for string-array addition as "fast" --- .../{slow => fast}/mixed_string_array_addition.swift | 1 - 1 file changed, 1 deletion(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/mixed_string_array_addition.swift (76%) diff --git a/validation-test/Sema/type_checker_perf/slow/mixed_string_array_addition.swift b/validation-test/Sema/type_checker_perf/fast/mixed_string_array_addition.swift similarity index 76% rename from validation-test/Sema/type_checker_perf/slow/mixed_string_array_addition.swift rename to validation-test/Sema/type_checker_perf/fast/mixed_string_array_addition.swift index aa0c3979b5ef2..75c7d1050eb4d 100644 --- a/validation-test/Sema/type_checker_perf/slow/mixed_string_array_addition.swift +++ b/validation-test/Sema/type_checker_perf/fast/mixed_string_array_addition.swift @@ -3,7 +3,6 @@ func method(_ arg: String, body: () -> [String]) {} func test(str: String, properties: [String]) { - // expected-error@+1 {{the compiler is unable to type-check this expression in reasonable time}} method(str + "" + str + "") { properties.map { param in "" + param + "" + param + "" + param + "" From 95e3a10fb15584b1d05177300f75dd4b0542317d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 10 Jan 2022 12:50:36 -0800 Subject: [PATCH 13/13] [ConstraintSystem] Don't stop at generic partition for unary operators when implicit conversions are involved --- include/swift/Sema/ConstraintSystem.h | 4 ++++ lib/Sema/CSStep.cpp | 22 ++++++++++++++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 5c95d75671e5f..2391932512291 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5630,6 +5630,10 @@ class DisjunctionChoice { bool isSymmetricOperator() const; bool isUnaryOperator() const; + bool isGenericUnaryOperator() const { + return isGenericOperator() && isUnaryOperator(); + } + void print(llvm::raw_ostream &Out, SourceManager *SM) const { Out << "disjunction choice "; Choice->print(Out, SM); diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index f6dd6245404c6..760015b8db0e7 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -697,16 +697,30 @@ bool DisjunctionStep::shouldStopAt(const DisjunctionChoice &choice) const { bool hasUnavailableOverloads = delta.Data[SK_Unavailable] > 0; bool hasFixes = delta.Data[SK_Fix] > 0; bool hasAsyncMismatch = delta.Data[SK_AsyncInSyncMismatch] > 0; - auto isBeginningOfPartition = choice.isBeginningOfPartition(); + bool isBeginningOfPartition = choice.isBeginningOfPartition(); // Attempt to short-circuit evaluation of this disjunction only // if the disjunction choice we are comparing to did not involve: // 1. selecting unavailable overloads // 2. result in fixes being applied to reach a solution // 3. selecting an overload that results in an async/sync mismatch - return !hasUnavailableOverloads && !hasFixes && !hasAsyncMismatch && - (isBeginningOfPartition || - shortCircuitDisjunctionAt(choice, lastChoice)); + if (hasUnavailableOverloads || hasFixes || hasAsyncMismatch) + return false; + + // Similar to \c shouldSkip - don't stop at the beginning of generic partition + // for unary operators when there was a solution with concrete operator choice + // that required an implicit CGFloat<->Double conversion, because not all of + // such operators have concrete `CGFloat` overloads. + if (isBeginningOfPartition && choice.isGenericUnaryOperator()) { + if (BestNonGenericScore) { + auto &score = BestNonGenericScore->Data; + if (score[SK_ImplicitValueConversion] > 0) + return false; + } + } + + return isBeginningOfPartition || + shortCircuitDisjunctionAt(choice, lastChoice); } bool swift::isSIMDOperator(ValueDecl *value) {