From 91fccd8562c162e023fc87cb1b5c0fccf38ede1e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 28 Dec 2021 21:38:24 -0800 Subject: [PATCH 1/6] [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 b00aa74d88c78..42d16daf0bc30 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 ad787a4bb1c24e74a19d0a5c55bb94b8cae15a2d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 28 Dec 2021 21:41:09 -0800 Subject: [PATCH 2/6] [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 42d16daf0bc30..c5377cd010018 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5033,6 +5033,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 f4f58f94a1d93a9c9d6adb2a38d66a295b281d03 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 2 Jan 2022 22:11:04 -0800 Subject: [PATCH 3/6] [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 50f31040160149fd543f52d036dc3586b2bdf4ff Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 5 Jan 2022 16:19:11 -0800 Subject: [PATCH 4/6] [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 7fecda72e37b4f501b255c2fb26d01d4b8a10073 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 8 Jan 2022 12:23:09 -0800 Subject: [PATCH 5/6] [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 172e31d7ce545..7df8e5eed2504 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -10401,6 +10401,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 6435083d8c7a853ef722141c02939502e42d6951 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 8 Jan 2022 13:29:26 -0800 Subject: [PATCH 6/6] [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 7df8e5eed2504..d8f957a993392 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -10428,6 +10428,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()) +}