From bccdd704cf36a0ced1bcbbd56d76c82ccd8c5d0c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:04:36 -0800 Subject: [PATCH 01/72] Revert "[CSOptimizer] Look through `OptionalEvaluationExpr`s when dealing with unapplied disjunctions" This reverts commit 72340f39b875a7a77bee67b343217d9bf8448254. --- lib/Sema/CSOptimizer.cpp | 8 +------- .../type_checker_perf/fast/swift_package_index_1.swift | 2 +- 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index a93ee36bf7423..c354fe728c4ab 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -317,13 +317,7 @@ static void determineBestChoicesInContext( if (applicableFn.isNull()) { auto *locator = disjunction->getLocator(); if (auto expr = getAsExpr(locator->getAnchor())) { - auto *parentExpr = cs.getParentExpr(expr); - // Look through optional evaluation, so - // we can cover expressions like `a?.b + 2`. - if (isExpr(parentExpr)) - parentExpr = cs.getParentExpr(parentExpr); - - if (parentExpr) { + if (auto *parentExpr = cs.getParentExpr(expr)) { // If this is a chained member reference or a direct operator // argument it could be prioritized since it helps to establish // context for other calls i.e. `(a.)b + 2` if `a` and/or `b` diff --git a/validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift b/validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift index f02ec4fe258ca..f415f68e9994b 100644 --- a/validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift +++ b/validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=500 +// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=1000 // REQUIRES: tools-release,no_asan public class Cookie { From d096dfdd7ef2dd872c06161b21b07cdc97ecf972 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:04:38 -0800 Subject: [PATCH 02/72] Revert "[CSOptimizer] Don't consider disabled overloads when checking whether disjunction is supported" This reverts commit 6bc23b50577fcc47d1ffbd0890b141d54c232a77. --- lib/Sema/CSOptimizer.cpp | 3 --- 1 file changed, 3 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c354fe728c4ab..27ee174a66158 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -125,9 +125,6 @@ static bool isSupportedDisjunction(Constraint *disjunction) { // Non-operator disjunctions are supported only if they don't // have any generic choices. return llvm::all_of(choices, [&](Constraint *choice) { - if (choice->isDisabled()) - return true; - if (choice->getKind() != ConstraintKind::BindOverload) return false; From e55b763f5a77cc0345aa8e30d2ecabbf3df2634a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:04:53 -0800 Subject: [PATCH 03/72] Revert "[CSOptimizer] Disjunctions with IUO overload choices are unsupported" This reverts commit 471ee21535d00e28fc0318169ad306c192593235. --- lib/Sema/CSOptimizer.cpp | 8 +------- validation-test/Sema/rdar143799118.swift | 23 ----------------------- 2 files changed, 1 insertion(+), 30 deletions(-) delete mode 100644 validation-test/Sema/rdar143799118.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 27ee174a66158..d447c45840f4d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -128,14 +128,8 @@ static bool isSupportedDisjunction(Constraint *disjunction) { if (choice->getKind() != ConstraintKind::BindOverload) return false; - if (auto *decl = getOverloadChoiceDecl(choice)) { - // Cannot optimize declarations that return IUO because - // they form a disjunction over a result type once attempted. - if (decl->isImplicitlyUnwrappedOptional()) - return false; - + if (auto *decl = getOverloadChoiceDecl(choice)) return decl->getInterfaceType()->is(); - } return false; }); diff --git a/validation-test/Sema/rdar143799118.swift b/validation-test/Sema/rdar143799118.swift deleted file mode 100644 index 7ed8a24e02026..0000000000000 --- a/validation-test/Sema/rdar143799118.swift +++ /dev/null @@ -1,23 +0,0 @@ -// RUN: %target-typecheck-verify-swift -target %target-swift-5.1-abi-triple - -func test1(v: Int!) -> [Any]! { nil } -// This is important because it defeats old solver hack that -// checked the number of matching overloads purely based on -// how many parameters there are. -func test1(v: Int!) async throws -> [Int]! { nil } -func test1(v: Int!, other: String = "") throws -> [Int] { [] } - -func test2(v: Int!) -> [Any]! { nil } -func test2(v: Int!, other: String = "") throws -> [Int] { [] } - -func performTest(v: Int!) { - guard let _ = test1(v: v) as? [Int] else { // Ok - return - } - - guard let _ = test2(v: v) as? [Int] else { - // expected-error@-1 {{call can throw, but it is not marked with 'try' and the error is not handled}} - // expected-warning@-2 {{conditional cast from '[Int]' to '[Int]' always succeeds}} - return - } -} From 31bcba64aaf7cafe93e2741d597a99b95c841dab Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:05:12 -0800 Subject: [PATCH 04/72] Revert "[CSOptimizer] MemberImportVisibility: Don't consider overloads that come from implicit imports" This reverts commit aa4a2b9071a68f97e269bafe2e7e8d8e60a79e67. --- lib/Sema/CSOptimizer.cpp | 12 ------ .../member_import_visibility.swift | 39 ------------------- 2 files changed, 51 deletions(-) delete mode 100644 test/Constraints/member_import_visibility.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index d447c45840f4d..b154ccf98b5e2 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -175,18 +175,6 @@ void forEachDisjunctionChoice( if (!decl) continue; - // Ignore declarations that come from implicitly imported modules - // when `MemberImportVisibility` feature is enabled otherwise - // we might end up favoring an overload that would be diagnosed - // as unavailable later. - if (cs.getASTContext().LangOpts.hasFeature( - Feature::MemberImportVisibility)) { - if (auto *useDC = constraint->getOverloadUseDC()) { - if (!useDC->isDeclImported(decl)) - continue; - } - } - // If disjunction choice is unavailable or disfavored we cannot // do anything with it. if (decl->getAttrs().hasAttribute() || diff --git a/test/Constraints/member_import_visibility.swift b/test/Constraints/member_import_visibility.swift deleted file mode 100644 index cda1c01513a7a..0000000000000 --- a/test/Constraints/member_import_visibility.swift +++ /dev/null @@ -1,39 +0,0 @@ -// RUN: %empty-directory(%t) -// RUN: %empty-directory(%t/src) -// RUN: split-file %s %t/src - -/// Build the library A -// RUN: %target-swift-frontend -emit-module %t/src/A.swift \ -// RUN: -module-name A \ -// RUN: -emit-module-path %t/A.swiftmodule - -/// Build the library B -// RUN: %target-swift-frontend -I %t -emit-module %t/src/B.swift \ -// RUN: -module-name B \ -// RUN: -emit-module-path %t/B.swiftmodule - -// RUN: %target-swift-frontend -typecheck -I %t %t/src/Main.swift %t/src/Other.swift -enable-upcoming-feature MemberImportVisibility - -// REQUIRES: swift_feature_MemberImportVisibility - -//--- A.swift -public struct Test { - public init(a: Double) { } -} - -//--- B.swift -import A - -extension Test { - public init(a: Int) { fatalError() } -} - -//--- Main.swift -import A - -func test() { - _ = Test(a: 0) // Ok, selects the overload that takes Double. -} - -//--- Other.swift -import B From 0a27ba1e273b99c8dc15a7bdc8923dca6b5b544e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:05:40 -0800 Subject: [PATCH 05/72] Revert "[CSOptimizer] Don't consider CGFloat widening when explicit initializer is used" This reverts commit 3cc76eacdd3997c4ef001869cfd3b483939db7dc. --- lib/Sema/CSOptimizer.cpp | 64 +++++++------------ .../implicit_double_cgfloat_conversion.swift | 15 ----- 2 files changed, 24 insertions(+), 55 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index b154ccf98b5e2..2f0aa13fb082a 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -370,23 +370,9 @@ static void determineBestChoicesInContext( FunctionType::relabelParams(argsWithLabels, argumentList); } - struct ArgumentCandidate { - Type type; - // The candidate type is derived from a literal expression. - bool fromLiteral : 1; - // The candidate type is derived from a call to an - // initializer i.e. `Double(...)`. - bool fromInitializerCall : 1; - - ArgumentCandidate(Type type, bool fromLiteral = false, - bool fromInitializerCall = false) - : type(type), fromLiteral(fromLiteral), - fromInitializerCall(fromInitializerCall) {} - }; - - SmallVector, 2> - argumentCandidates; - argumentCandidates.resize(argFuncType->getNumParams()); + SmallVector, 2>, 2> + candidateArgumentTypes; + candidateArgumentTypes.resize(argFuncType->getNumParams()); llvm::TinyPtrVector resultTypes; @@ -394,12 +380,12 @@ static void determineBestChoicesInContext( const auto ¶m = argFuncType->getParams()[i]; auto argType = cs.simplifyType(param.getPlainType()); - SmallVector types; + SmallVector, 2> types; if (auto *typeVar = argType->getAs()) { auto bindingSet = cs.getBindingsFor(typeVar); for (const auto &binding : bindingSet.Bindings) { - types.push_back({binding.BindingType}); + types.push_back({binding.BindingType, /*fromLiteral=*/false}); } for (const auto &literal : bindingSet.Literals) { @@ -422,8 +408,7 @@ static void determineBestChoicesInContext( if (auto *typeExpr = dyn_cast(call->getFn())) { auto instanceTy = cs.getType(typeExpr)->getMetatypeInstanceType(); if (instanceTy->isDouble() || instanceTy->isCGFloat()) - types.push_back({instanceTy, /*fromLiteral=*/false, - /*fromInitializerCall=*/true}); + types.push_back({instanceTy, /*fromLiteral=*/false}); } } } @@ -431,7 +416,7 @@ static void determineBestChoicesInContext( types.push_back({argType, /*fromLiteral=*/false}); } - argumentCandidates[i].append(types); + candidateArgumentTypes[i].append(types); } auto resultType = cs.simplifyType(argFuncType->getResult()); @@ -452,9 +437,9 @@ static void determineBestChoicesInContext( argFuncType->getNumParams() > 0 && llvm::none_of( indices(argFuncType->getParams()), [&](const unsigned argIdx) { - auto &candidates = argumentCandidates[argIdx]; + auto &candidates = candidateArgumentTypes[argIdx]; return llvm::any_of(candidates, [&](const auto &candidate) { - return !candidate.fromLiteral; + return !candidate.second; }); }); @@ -861,7 +846,7 @@ static void determineBestChoicesInContext( auto argIdx = argIndices.front(); // Looks like there is nothing know about the argument. - if (argumentCandidates[argIdx].empty()) + if (candidateArgumentTypes[argIdx].empty()) continue; const auto paramFlags = param.getParameterFlags(); @@ -888,25 +873,32 @@ static void determineBestChoicesInContext( // at this parameter position and remove the overload choice // from consideration. double bestCandidateScore = 0; - llvm::BitVector mismatches(argumentCandidates[argIdx].size()); + llvm::BitVector mismatches(candidateArgumentTypes[argIdx].size()); for (unsigned candidateIdx : - indices(argumentCandidates[argIdx])) { + indices(candidateArgumentTypes[argIdx])) { // If one of the candidates matched exactly there is no reason // to continue checking. if (bestCandidateScore == 1) break; - auto candidate = argumentCandidates[argIdx][candidateIdx]; + Type candidateType; + bool isLiteralDefault; + + std::tie(candidateType, isLiteralDefault) = + candidateArgumentTypes[argIdx][candidateIdx]; // `inout` parameter accepts only l-value argument. - if (paramFlags.isInOut() && !candidate.type->is()) { + if (paramFlags.isInOut() && !candidateType->is()) { mismatches.set(candidateIdx); continue; } + // The specifier only matters for `inout` check. + candidateType = candidateType->getWithoutSpecifierType(); + MatchOptions options(MatchFlag::OnParam); - if (candidate.fromLiteral) + if (isLiteralDefault) options |= MatchFlag::Literal; if (favorExactMatchesOnly) options |= MatchFlag::ExactOnly; @@ -921,16 +913,8 @@ static void determineBestChoicesInContext( if (n == 1 && decl->isOperator()) options |= MatchFlag::DisableCGFloatDoubleConversion; - // Disable implicit CGFloat -> Double widening conversion if - // argument is an explicit call to `CGFloat` initializer. - if (candidate.type->isCGFloat() && - candidate.fromInitializerCall) - options |= MatchFlag::DisableCGFloatDoubleConversion; - - // The specifier for a candidate only matters for `inout` check. auto candidateScore = scoreCandidateMatch( - genericSig, decl, candidate.type->getWithoutSpecifierType(), - paramType, options); + genericSig, decl, candidateType, paramType, options); if (!candidateScore) continue; @@ -944,7 +928,7 @@ static void determineBestChoicesInContext( // Only established arguments could be considered mismatches, // literal default types should be regarded as holes if they // didn't match. - if (!candidate.fromLiteral && !candidate.type->hasTypeVariable()) + if (!isLiteralDefault && !candidateType->hasTypeVariable()) mismatches.set(candidateIdx); } diff --git a/test/Constraints/implicit_double_cgfloat_conversion.swift b/test/Constraints/implicit_double_cgfloat_conversion.swift index b34d8a88e566d..83c6955851b58 100644 --- a/test/Constraints/implicit_double_cgfloat_conversion.swift +++ b/test/Constraints/implicit_double_cgfloat_conversion.swift @@ -366,18 +366,3 @@ func test_cgfloat_operator_is_attempted_with_literal_arguments(v: CGFloat?) { let ratio = v ?? (2.0 / 16.0) let _: CGFloat = ratio // Ok } - -// Make sure that optimizer doesn't favor CGFloat -> Double conversion -// in presence of CGFloat initializer, otherwise it could lead to ambiguities. -func test_explicit_cgfloat_use_avoids_ambiguity(v: Int) { - func test(_: CGFloat) -> CGFloat { 0 } - func test(_: Double) -> Double { 0 } - - func hasCGFloatElement(_: C) where C.Element == CGFloat {} - - let arr = [test(CGFloat(v))] - hasCGFloatElement(arr) // Ok - - var total = 0.0 // This is Double by default - total += test(CGFloat(v)) + CGFloat(v) // Ok -} From 62b4c947f502151b8bcfc32752153735c8f11586 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:06:04 -0800 Subject: [PATCH 06/72] Revert "[CSOptimizer] Literal arguments should cause score reset only for operators" This reverts commit e3987beffb036b3d653340e20e77576392e79975. --- lib/Sema/CSOptimizer.cpp | 2 +- validation-test/Sema/issue78371.swift | 22 ---------------------- 2 files changed, 1 insertion(+), 23 deletions(-) delete mode 100644 validation-test/Sema/issue78371.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 2f0aa13fb082a..f3ea92915d0e7 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -958,7 +958,7 @@ static void determineBestChoicesInContext( // Preferring outer disjunction first works better in situations // when contextual type for the whole chain becomes available at // some point during solving at it would allow for faster pruning. - if (score > 0 && onlyLiteralCandidates && decl->isOperator()) + if (score > 0 && onlyLiteralCandidates) score = 0.1; // If one of the result types matches exactly, that's a good diff --git a/validation-test/Sema/issue78371.swift b/validation-test/Sema/issue78371.swift deleted file mode 100644 index e986ad173e291..0000000000000 --- a/validation-test/Sema/issue78371.swift +++ /dev/null @@ -1,22 +0,0 @@ -// RUN: %target-typecheck-verify-swift - -// https://github.com/swiftlang/swift/issues/78371 - -// Note that the test has to use a standard operator because -// custom ones are not optimized. - -struct Scalar : Equatable { - init(_: UInt8) {} - init?(_: Int) {} -} - -func ==(_: Scalar, _: Scalar) -> Bool { } - -extension Optional where Wrapped == Scalar { - static func ==(_: Wrapped?, _: Wrapped?) -> Wrapped { } -} - -func test(a: Scalar) { - let result = a == Scalar(0x07FD) - let _: Scalar = result // Ok -} From 955eead724c67a44e0fbae66cc2d4f4d0090ed4f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:06:07 -0800 Subject: [PATCH 07/72] Revert "[CSOptimizer] NFC: check whether a choice is of operator instead of whole disjunction" This reverts commit 6c82892c3c937d254da20c13b9a2050dc05635aa. --- lib/Sema/CSOptimizer.cpp | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index f3ea92915d0e7..2e9b1976d8c1a 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -2,7 +2,7 @@ // // This source file is part of the Swift.org open source project // -// Copyright (c) 2014 - 2025 Apple Inc. and the Swift project authors +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors // Licensed under Apache License v2.0 with Runtime Library Exception // // See https://swift.org/LICENSE.txt for license information @@ -527,11 +527,10 @@ static void determineBestChoicesInContext( // type matches a parameter type (i.e. when partially resolved generic // types are matched) this function is going to produce \c std::nullopt // instead of `0` that indicates "not a match". - std::function(GenericSignature, ValueDecl *, Type, - Type, MatchOptions)> + std::function(GenericSignature, Type, Type, + MatchOptions)> scoreCandidateMatch = - [&](GenericSignature genericSig, ValueDecl *choice, - Type candidateType, Type paramType, + [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> std::optional { auto areEqual = [&](Type a, Type b) { return a->getDesugaredType()->isEqual(b->getDesugaredType()); @@ -619,12 +618,13 @@ static void determineBestChoicesInContext( if (!candidateOptionals.empty() || !paramOptionals.empty()) { if (paramOptionals.size() >= candidateOptionals.size()) { - auto score = scoreCandidateMatch(genericSig, choice, candidateType, + auto score = scoreCandidateMatch(genericSig, candidateType, paramType, options); // Injection lowers the score slightly to comply with // old behavior where exact matches on operator parameter // types were always preferred. - return score == 1 && choice->isOperator() ? 0.9 : score; + return score == 1 && isOperatorDisjunction(disjunction) ? 0.9 + : score; } // Optionality mismatch. @@ -746,7 +746,7 @@ static void determineBestChoicesInContext( // everything else the solver should try both concrete and // generic and disambiguate during ranking. if (result == CheckRequirementsResult::Success) - return choice->isOperator() ? 0.9 : 1.0; + return isOperatorDisjunction(disjunction) ? 0.9 : 1.0; return 0; } @@ -914,7 +914,7 @@ static void determineBestChoicesInContext( options |= MatchFlag::DisableCGFloatDoubleConversion; auto candidateScore = scoreCandidateMatch( - genericSig, decl, candidateType, paramType, options); + genericSig, candidateType, paramType, options); if (!candidateScore) continue; @@ -984,7 +984,7 @@ static void determineBestChoicesInContext( ->isCGFloat()) return false; - return scoreCandidateMatch(genericSig, decl, + return scoreCandidateMatch(genericSig, overloadType->getResult(), candidateResultTy, /*options=*/{}) > 0; From 25ddf1a2711988859ffc9ea26de8eb72dbbdf117 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:06:34 -0800 Subject: [PATCH 08/72] Revert "[CSOptimizer/Tests] NFC: Add a perf test-case fixed by improved literal array handling" This reverts commit cfd34e54c4f242a9944e6a095844625f9351317e. --- ...ding_dot_syntax_in_literal_array.swift.gyb | 23 ------------------- 1 file changed, 23 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/leading_dot_syntax_in_literal_array.swift.gyb diff --git a/validation-test/Sema/type_checker_perf/fast/leading_dot_syntax_in_literal_array.swift.gyb b/validation-test/Sema/type_checker_perf/fast/leading_dot_syntax_in_literal_array.swift.gyb deleted file mode 100644 index a022b5c29a3d3..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/leading_dot_syntax_in_literal_array.swift.gyb +++ /dev/null @@ -1,23 +0,0 @@ -// RUN: %scale-test --begin 1 --end 15 --step 1 --select NumLeafScopes %s --expected-exit-code 0 -// REQUIRES: asserts,no_asan - -enum E { - case a - case b - case c(Int32) -} - -struct Tester { - mutating func test(arr: [E], cond: Bool = false) {} - mutating func test(arr: E..., cond: Bool = false) {} -} - -func test() { - var tester = Tester() - tester.test(arr: [ - .c(1), .a, -%for i in range(N): - .c(1 << 4 | 8), .c(0), -%end - .c(1), .b]) -} From 51ab75d00fd079c5dee99e50bf4948ff7ea76373 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:06:37 -0800 Subject: [PATCH 09/72] Revert "[CSOptimizer] Extend candidate/parameter matching to support array literals" This reverts commit 8a304f88c6cf5ace86975297c885725362032517. --- lib/Sema/CSOptimizer.cpp | 25 ------------------- .../fast/array_count_property_vs_method.swift | 3 +-- 2 files changed, 1 insertion(+), 27 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 2e9b1976d8c1a..3dca203c11947 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -15,7 +15,6 @@ //===----------------------------------------------------------------------===// #include "TypeChecker.h" -#include "swift/AST/ConformanceLookup.h" #include "swift/AST/ExistentialLayout.h" #include "swift/AST/GenericSignature.h" #include "swift/Basic/OptionSet.h" @@ -555,30 +554,6 @@ static void determineBestChoicesInContext( } } - // Match `[...]` to Array<...> and/or `ExpressibleByArrayLiteral` - // conforming types. - if (options.contains(MatchFlag::OnParam) && - options.contains(MatchFlag::Literal) && - isUnboundArrayType(candidateType)) { - // If an exact match is requested favor only `[...]` to `Array<...>` - // since everything else is going to increase to score. - if (options.contains(MatchFlag::ExactOnly)) - return paramType->isArrayType() ? 1 : 0; - - // Otherwise, check if the other side conforms to - // `ExpressibleByArrayLiteral` protocol (in some way). - // We want an overly optimistic result here to avoid - // under-favoring. - auto &ctx = cs.getASTContext(); - return checkConformanceWithoutContext( - paramType, - ctx.getProtocol( - KnownProtocolKind::ExpressibleByArrayLiteral), - /*allowMissing=*/true) - ? 0.3 - : 0; - } - if (options.contains(MatchFlag::ExactOnly)) return areEqual(candidateType, paramType) ? 1 : 0; diff --git a/validation-test/Sema/type_checker_perf/fast/array_count_property_vs_method.swift b/validation-test/Sema/type_checker_perf/fast/array_count_property_vs_method.swift index d4c0d407afa70..bf3d6703df3e2 100644 --- a/validation-test/Sema/type_checker_perf/fast/array_count_property_vs_method.swift +++ b/validation-test/Sema/type_checker_perf/fast/array_count_property_vs_method.swift @@ -1,6 +1,5 @@ -// RUN: %target-typecheck-verify-swift -solver-scope-threshold=11000 +// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=5 // REQUIRES: tools-release,no_asan -// REQUIRES: OS=macosx func f(n: Int, a: [Int]) { let _ = [(0 ..< n + a.count).map { Int8($0) }] + From 0eca9bed31d667287796ccfd3ff83663b1b216e6 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:06:39 -0800 Subject: [PATCH 10/72] Revert "[CSOptimizer] Favor choices that don't require application" This reverts commit 0737542da8813444efa9f9056bc9a8ef09ac4fdf. --- lib/Sema/CSOptimizer.cpp | 20 ++----------------- .../fast/array_count_property_vs_method.swift | 7 ------- 2 files changed, 2 insertions(+), 25 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/array_count_property_vs_method.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 3dca203c11947..a7770f728b47c 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -61,12 +61,6 @@ static bool isFloatType(Type type) { return type->isFloat() || type->isDouble() || type->isFloat80(); } -static bool isUnboundArrayType(Type type) { - if (auto *UGT = type->getAs()) - return UGT->getDecl() == type->getASTContext().getArrayDecl(); - return false; -} - static bool isSupportedOperator(Constraint *disjunction) { if (!isOperatorDisjunction(disjunction)) return false; @@ -304,19 +298,9 @@ static void determineBestChoicesInContext( case ExprKind::Binary: case ExprKind::PrefixUnary: case ExprKind::PostfixUnary: - case ExprKind::UnresolvedDot: { - llvm::SmallVector favoredChoices; - // Favor choices that don't require application. - llvm::copy_if( - disjunction->getNestedConstraints(), - std::back_inserter(favoredChoices), [](Constraint *choice) { - auto *decl = getOverloadChoiceDecl(choice); - return decl && - !decl->getInterfaceType()->is(); - }); - recordResult(disjunction, {/*score=*/1.0, favoredChoices}); + case ExprKind::UnresolvedDot: + recordResult(disjunction, {/*score=*/1.0}); continue; - } default: break; diff --git a/validation-test/Sema/type_checker_perf/fast/array_count_property_vs_method.swift b/validation-test/Sema/type_checker_perf/fast/array_count_property_vs_method.swift deleted file mode 100644 index bf3d6703df3e2..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/array_count_property_vs_method.swift +++ /dev/null @@ -1,7 +0,0 @@ -// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=5 -// REQUIRES: tools-release,no_asan - -func f(n: Int, a: [Int]) { - let _ = [(0 ..< n + a.count).map { Int8($0) }] + - [(0 ..< n + a.count).map { Int8($0) }.reversed()] // Ok -} From 224deb3b5d826180975b794b1e3dc294d3c52453 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:08:47 -0800 Subject: [PATCH 11/72] Revert "[CSOptimizer] Disable CGFloat -> Double conversion for unary operators" This reverts commit bc3a15fbe67e13e1ba8b6808b27508f8881a2608. --- lib/Sema/CSOptimizer.cpp | 20 +------------------ .../fast/cgfloat_with_unary_operators.swift | 19 ------------------ 2 files changed, 1 insertion(+), 38 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index a7770f728b47c..5be8ed6cd030e 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -490,7 +490,6 @@ static void determineBestChoicesInContext( OnParam = 0x01, Literal = 0x02, ExactOnly = 0x04, - DisableCGFloatDoubleConversion = 0x08, }; using MatchOptions = OptionSet; @@ -519,20 +518,13 @@ static void determineBestChoicesInContext( return a->getDesugaredType()->isEqual(b->getDesugaredType()); }; - auto isCGFloatDoubleConversionSupported = [&options]() { - // CGFloat <-> Double conversion is supposed only while - // match argument candidates to parameters. - return options.contains(MatchFlag::OnParam) && - !options.contains(MatchFlag::DisableCGFloatDoubleConversion); - }; - // Allow CGFloat -> Double widening conversions between // candidate argument types and parameter types. This would // make sure that Double is always preferred over CGFloat // when using literals and ranking supported disjunction // choices. Narrowing conversion (Double -> CGFloat) should // be delayed as much as possible. - if (isCGFloatDoubleConversionSupported()) { + if (options.contains(MatchFlag::OnParam)) { if (candidateType->isCGFloat() && paramType->isDouble()) { return options.contains(MatchFlag::Literal) ? 0.2 : 0.9; } @@ -862,16 +854,6 @@ static void determineBestChoicesInContext( if (favorExactMatchesOnly) options |= MatchFlag::ExactOnly; - // Disable CGFloat -> Double conversion for unary operators. - // - // Some of the unary operators, i.e. prefix `-`, don't have - // CGFloat variants and expect generic `FloatingPoint` overload - // to match CGFloat type. Let's not attempt `CGFloat` -> `Double` - // conversion for unary operators because it always leads - // to a worse solutions vs. generic overloads. - if (n == 1 && decl->isOperator()) - options |= MatchFlag::DisableCGFloatDoubleConversion; - auto candidateScore = scoreCandidateMatch( genericSig, candidateType, paramType, options); diff --git a/validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift b/validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift deleted file mode 100644 index ebf7490e9cfdf..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift +++ /dev/null @@ -1,19 +0,0 @@ -// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 - -// REQUIRES: OS=macosx,no_asan -// REQUIRES: objc_interop - -import Foundation -import CoreGraphics -import simd - -func test( - a: CGFloat, - b: CGFloat -) -> CGFloat { - exp(-a * b) * - (a * -sin(a * b) * a + ((a * b + a) / b) * cos(a * b) * a) + - exp(-a * b) * - (-b) * - (a * cos(a * b) + ((a * b + a) / b) * sin(a * b)) -} From 0ac1fa0c519ab7f90309e32cf46236fa8d1a085b Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:18:53 -0800 Subject: [PATCH 12/72] Revert "[CSOptimizer] Mark bitwise operators as supported" This reverts commit 860ae08d1b9e647eb2911991f715b4a80fcb7a32. --- lib/Sema/CSOptimizer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 5be8ed6cd030e..1a8fb5d34c7fa 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -70,7 +70,7 @@ static bool isSupportedOperator(Constraint *disjunction) { auto name = decl->getBaseIdentifier(); if (name.isArithmeticOperator() || name.isStandardComparisonOperator() || - name.isBitwiseOperator()) { + name.is("^")) { return true; } From 2f1b186573aa06de5ec58fbd22c0c1a6b47a06a8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:18:56 -0800 Subject: [PATCH 13/72] Revert "[CSOptimizer] Simplify handling of non-applied disjunctions" This reverts commit 43ca7dfff981a927f4b82a2f045ed5bd58099c00. --- lib/Sema/CSOptimizer.cpp | 56 +++++++++++++++++++++------------ test/Constraints/operator.swift | 17 ---------- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 1a8fb5d34c7fa..460c892a41f81 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -253,6 +253,37 @@ static void findFavoredChoicesBasedOnArity( favoredChoice(choice); } +/// Determine whether the given disjunction serves as a base of +/// another member reference i.e. `x.y` where `x` could be overloaded. +static bool isPartOfMemberChain(ConstraintSystem &CS, Constraint *disjunction) { + if (isOperatorDisjunction(disjunction)) + return false; + + auto &CG = CS.getConstraintGraph(); + + TypeVariableType *typeVar = nullptr; + + // If disjunction is applied, the member is chained on the result. + if (auto appliedFn = CS.getAppliedDisjunctionArgumentFunction(disjunction)) { + typeVar = appliedFn->getResult()->getAs(); + } else { + typeVar = disjunction->getNestedConstraints()[0] + ->getFirstType() + ->getAs(); + } + + if (!typeVar) + return false; + + return llvm::any_of( + CG[typeVar].getConstraints(), [&typeVar](Constraint *constraint) { + if (constraint->getKind() != ConstraintKind::ValueMember) + return false; + + return constraint->getFirstType()->isEqual(typeVar); + }); +} + } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -287,26 +318,11 @@ static void determineBestChoicesInContext( getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); if (applicableFn.isNull()) { - auto *locator = disjunction->getLocator(); - if (auto expr = getAsExpr(locator->getAnchor())) { - if (auto *parentExpr = cs.getParentExpr(expr)) { - // If this is a chained member reference or a direct operator - // argument it could be prioritized since it helps to establish - // context for other calls i.e. `(a.)b + 2` if `a` and/or `b` - // are disjunctions they should be preferred over `+`. - switch (parentExpr->getKind()) { - case ExprKind::Binary: - case ExprKind::PrefixUnary: - case ExprKind::PostfixUnary: - case ExprKind::UnresolvedDot: - recordResult(disjunction, {/*score=*/1.0}); - continue; - - default: - break; - } - } - } + // If this is a chained member reference it could be prioritized since + // it helps to establish context for other calls i.e. `a.b + 2` if + // `a` is a disjunction it should be preferred over `+`. + if (isPartOfMemberChain(cs, disjunction)) + recordResult(disjunction, {/*score=*/1.0}); continue; } diff --git a/test/Constraints/operator.swift b/test/Constraints/operator.swift index d12bcbbc20a04..77d07f5dd3078 100644 --- a/test/Constraints/operator.swift +++ b/test/Constraints/operator.swift @@ -375,20 +375,3 @@ do { col <<<>>> val // Ok } } - -// Make sure that ?? selects an overload that doesn't produce an optional. -do { - class Obj { - var x: String! - } - - class Child : Obj { - func x() -> String? { nil } - static func x(_: Int) -> String { "" } - } - - func test(arr: [Child], v: String, defaultV: Child) -> Child { - let result = arr.first { $0.x == v } ?? defaultV - return result // Ok - } -} From e7aeef87f53b00333551ae4a66ceb21a453039e2 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:18:59 -0800 Subject: [PATCH 14/72] Revert "[ConstraintSystem] Fix `getEffectiveOverloadType` handling of `mutating` methods" This reverts commit c767f7aff7ec02d0d772455df56bb8cc8cdd8ff3. --- lib/Sema/TypeOfReference.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp index d60bac95fbc6a..44110d5927e6d 100644 --- a/lib/Sema/TypeOfReference.cpp +++ b/lib/Sema/TypeOfReference.cpp @@ -1834,15 +1834,11 @@ Type ConstraintSystem::getEffectiveOverloadType(ConstraintLocator *locator, type, var, useDC, GetClosureType{*this}, ClosureIsolatedByPreconcurrency{*this}); } else if (isa(decl) || isa(decl)) { - if (decl->isInstanceMember()) { - auto baseTy = overload.getBaseType(); - if (!baseTy) - return Type(); - - baseTy = baseTy->getRValueType(); - if (!baseTy->getAnyNominal() && !baseTy->is()) - return Type(); - } + if (decl->isInstanceMember() && + (!overload.getBaseType() || + (!overload.getBaseType()->getAnyNominal() && + !overload.getBaseType()->is()))) + return Type(); // Cope with 'Self' returns. if (!decl->getDeclContext()->getSelfProtocolDecl()) { From 8dd7f7ab9e7774d7071bd2b74bed1fcfd16a663c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:19:07 -0800 Subject: [PATCH 15/72] Revert "[CSOptimizer] Reduce overload types before ranking" This reverts commit 95b47aead6e7a7a933e3c764e3c918ba7507465e. --- lib/Sema/CSOptimizer.cpp | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 460c892a41f81..51ee22a4cbf36 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -780,14 +780,6 @@ static void determineBestChoicesInContext( favorExactMatchesOnly = true; } - // This is important for SIMD operators in particular because - // a lot of their overloads have same-type requires to a concrete - // type: `(_: SIMD*, ...) -> ...`. - if (genericSig) { - overloadType = overloadType->getReducedType(genericSig) - ->castTo(); - } - double score = 0.0; unsigned numDefaulted = 0; for (unsigned paramIdx = 0, n = overloadType->getNumParams(); From 5f59fbc1698fbf964e85f1ea67c1196ac31f89da Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:19:09 -0800 Subject: [PATCH 16/72] Revert "[CSOptimizer] Implement special prioritization rules for result builder contexts" This reverts commit 56d6635e46e9c2b88c40f2fabad3a452d6748f9d. --- lib/Sema/CSOptimizer.cpp | 78 ------------------- .../complex_swiftui_padding_conditions.swift | 17 ---- 2 files changed, 95 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 51ee22a4cbf36..6e7d9e097f5e7 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1097,79 +1097,6 @@ selectBestBindingDisjunction(ConstraintSystem &cs, return firstBindDisjunction; } -/// Prioritize `build{Block, Expression, ...}` and any chained -/// members that are connected to individual builder elements -/// i.e. `ForEach(...) { ... }.padding(...)`, once `ForEach` -/// is resolved, `padding` should be prioritized because its -/// requirements can help prune the solution space before the -/// body is checked. -static Constraint * -selectDisjunctionInResultBuilderContext(ConstraintSystem &cs, - ArrayRef disjunctions) { - auto context = AnyFunctionRef::fromDeclContext(cs.DC); - if (!context) - return nullptr; - - if (!cs.getAppliedResultBuilderTransform(context.value())) - return nullptr; - - std::pair best{nullptr, 0}; - for (auto *disjunction : disjunctions) { - auto *member = - getAsExpr(disjunction->getLocator()->getAnchor()); - if (!member) - continue; - - // Attempt `build{Block, Expression, ...} first because they - // provide contextual information for the inner calls. - if (isResultBuilderMethodReference(cs.getASTContext(), member)) - return disjunction; - - Expr *curr = member; - bool disqualified = false; - // Walk up the parent expression chain and check whether this - // disjunction represents one of the members in a chain that - // leads up to `buildExpression` (if defined by the builder) - // or to a pattern binding for `$__builderN` (the walk won't - // find any argument position locations in that case). - while (auto parent = cs.getParentExpr(curr)) { - if (!(isExpr(parent) || isExpr(parent))) { - disqualified = true; - break; - } - - if (auto *call = getAsExpr(parent)) { - // The current parent appears in an argument position. - if (call->getFn() != curr) { - // Allow expressions that appear in a argument position to - // `build{Expression, Block, ...} methods. - if (auto *UDE = getAsExpr(call->getFn())) { - disqualified = - !isResultBuilderMethodReference(cs.getASTContext(), UDE); - } else { - disqualified = true; - } - } - } - - if (disqualified) - break; - - curr = parent; - } - - if (disqualified) - continue; - - if (auto depth = cs.getExprDepth(member)) { - if (!best.first || best.second > depth) - best = std::make_pair(disjunction, depth.value()); - } - } - - return best.first; -} - std::optional>> ConstraintSystem::selectDisjunction() { SmallVector disjunctions; @@ -1184,11 +1111,6 @@ ConstraintSystem::selectDisjunction() { llvm::DenseMap favorings; determineBestChoicesInContext(*this, disjunctions, favorings); - if (auto *disjunction = - selectDisjunctionInResultBuilderContext(*this, disjunctions)) { - return std::make_pair(disjunction, favorings[disjunction].FavoredChoices); - } - // Pick the disjunction with the smallest number of favored, then active // choices. auto bestDisjunction = std::min_element( diff --git a/validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift b/validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift deleted file mode 100644 index df038a16ff4cc..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift +++ /dev/null @@ -1,17 +0,0 @@ -// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -swift-version 5 -// REQUIRES: OS=macosx - -import SwiftUI - -func test(a: [(offset: Int, element: Double)], - c: Color, - x: CGFloat, - n: Int -) -> some View { - ForEach(a, id: \.offset) { i, r in - RoundedRectangle(cornerRadius: r, style: .continuous) - .stroke(c, lineWidth: 1) - .padding(.horizontal, x / Double(n) * Double(n - 1 - i) / 2) - .padding(.vertical, x / Double(n) * Double(n - 1 - i) / 2) - } -} From 6635a472ca1b5a505208ed03527a6a3450614b89 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:19:12 -0800 Subject: [PATCH 17/72] Revert "[CSOptimizer] Allow only widening CGFloat->Double conversions while matching candidate arguments" This reverts commit bf8ae3bc1b6ad57d300d8a9207283d89482bb968. --- lib/Sema/CSOptimizer.cpp | 39 ++++++------------- ...l_with_operators_and_double_literals.swift | 31 --------------- 2 files changed, 11 insertions(+), 59 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 6e7d9e097f5e7..b7904f09934f1 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -530,22 +530,18 @@ static void determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> std::optional { - auto areEqual = [&](Type a, Type b) { + auto areEqual = [&options](Type a, Type b) { + // Double<->CGFloat implicit conversion support for literals + // only since in this case the conversion might not result in + // score penalty. + if (options.contains(MatchFlag::Literal) && + ((a->isDouble() && b->isCGFloat()) || + (a->isCGFloat() && b->isDouble()))) + return true; + return a->getDesugaredType()->isEqual(b->getDesugaredType()); }; - // Allow CGFloat -> Double widening conversions between - // candidate argument types and parameter types. This would - // make sure that Double is always preferred over CGFloat - // when using literals and ranking supported disjunction - // choices. Narrowing conversion (Double -> CGFloat) should - // be delayed as much as possible. - if (options.contains(MatchFlag::OnParam)) { - if (candidateType->isCGFloat() && paramType->isDouble()) { - return options.contains(MatchFlag::Literal) ? 0.2 : 0.9; - } - } - if (options.contains(MatchFlag::ExactOnly)) return areEqual(candidateType, paramType) ? 1 : 0; @@ -563,12 +559,12 @@ static void determineBestChoicesInContext( if (candidateType->isInt() && TypeChecker::conformsToKnownProtocol( paramType, KnownProtocolKind::ExpressibleByIntegerLiteral)) - return paramType->isDouble() ? 0.2 : 0.3; + return 0.2; if (candidateType->isDouble() && TypeChecker::conformsToKnownProtocol( paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) - return 0.3; + return 0.2; } return 0; @@ -920,19 +916,6 @@ static void determineBestChoicesInContext( // with a lot of favored overloads because on the result type alone. if (decl->isOperator() && !isStandardComparisonOperator(decl)) { if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) { - // Avoid increasing weight based on CGFloat result type - // match because that could require narrowing conversion - // in the arguments and that is always detrimental. - // - // For example, `has_CGFloat_param(1.0 + 2.0)` should use - // `+(_: Double, _: Double) -> Double` instead of - // `+(_: CGFloat, _: CGFloat) -> CGFloat` which would match - // parameter of `has_CGFloat_param` exactly but use a - // narrowing conversion for both literals. - if (candidateResultTy->lookThroughAllOptionalTypes() - ->isCGFloat()) - return false; - return scoreCandidateMatch(genericSig, overloadType->getResult(), candidateResultTy, diff --git a/validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift b/validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift deleted file mode 100644 index 49ce8b4d6abac..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift +++ /dev/null @@ -1,31 +0,0 @@ -// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) %s -typecheck -solver-expression-time-threshold=1 - -// REQUIRES: asserts,no_asan -// REQUIRES: objc_interop - -// FIXME: This should be a scale-test but it doesn't allow passing `%clang-importer-sdk` - -// This import is important because it brings CGFloat and -// enables Double<->CGFloat implicit conversion that affects -// literals below. -import Foundation - -let p/*: [(String, Bool, Bool, Double)]*/ = [ - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0), - ("", true, true, 0 * 0.0 * 0.0) -] From fa12735c7f752f227d47048cb13d07684c595b96 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:19:17 -0800 Subject: [PATCH 18/72] Revert "[CSSimplify] CGFloat-Double: Rank narrowing correctly when result is injected into an optional" This reverts commit cb876cbd9e4123cc6ba9a989b0cd91e5f79dc36a. --- lib/Sema/CSSimplify.cpp | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index ff7da127505bc..7e4d212b372b0 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -14841,19 +14841,9 @@ ConstraintSystem::simplifyRestrictedConstraintImpl( restriction == ConversionRestrictionKind::CGFloatToDouble ? 2 : 10; if (restriction == ConversionRestrictionKind::DoubleToCGFloat) { - SmallVector originalPath; - auto anchor = locator.getLocatorParts(originalPath); - - SourceRange range; - ArrayRef path(originalPath); - simplifyLocator(anchor, path, range); - - if (path.empty() || llvm::all_of(path, [](const LocatorPathElt &elt) { - return elt.is(); - })) { - if (auto *expr = getAsExpr(anchor)) - if (auto depth = getExprDepth(expr)) - impact = (*depth + 1) * impact; + if (auto *anchor = locator.trySimplifyToExpr()) { + if (auto depth = getExprDepth(anchor)) + impact = (*depth + 1) * impact; } } else if (locator.directlyAt() || locator.endsWith()) { From c65c27307cb378b6f76665d95279ef7cb1f6e28e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:27 -0800 Subject: [PATCH 19/72] Revert "[CSBindings] Prevent `BindingSet::isViable` from dropping viable bindings (v2)" This reverts commit b7e749307644bd2f5cc5daf243a0edfdf67741e0. --- lib/Sema/CSBindings.cpp | 33 ++------------------------------- 1 file changed, 2 insertions(+), 31 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 94cec862b8bad..f8d6759e304fe 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -31,6 +31,7 @@ using namespace swift; using namespace constraints; using namespace inference; + void ConstraintGraphNode::initBindingSet() { ASSERT(!hasBindingSet()); ASSERT(forRepresentativeVar()); @@ -38,12 +39,6 @@ void ConstraintGraphNode::initBindingSet() { Set.emplace(CG.getConstraintSystem(), TypeVar, Potential); } -/// Check whether there exists a type that could be implicitly converted -/// to a given type i.e. is the given type is Double or Optional<..> this -/// function is going to return true because CGFloat could be converted -/// to a Double and non-optional value could be injected into an optional. -static bool hasConversions(Type); - static std::optional checkTypeOfBinding(TypeVariableType *typeVar, Type type); @@ -1310,31 +1305,7 @@ bool BindingSet::isViable(PotentialBinding &binding, bool isTransitive) { if (!existingNTD || NTD != existingNTD) continue; - // What is going on in this method needs to be thoroughly re-evaluated! - // - // This logic aims to skip dropping bindings if - // collection type has conversions i.e. in situations like: - // - // [$T1] conv $T2 - // $T2 conv [(Int, String)] - // $T2.Element equal $T5.Element - // - // `$T1` could be bound to `(i: Int, v: String)` after - // `$T2` is bound to `[(Int, String)]` which is is a problem - // because it means that `$T2` was attempted to early - // before the solver had a chance to discover all viable - // bindings. - // - // Let's say existing binding is `[(Int, String)]` and - // relation is "exact", in this case there is no point - // tracking `[$T1]` because upcasts are only allowed for - // subtype and other conversions. - if (existing->Kind != AllowedBindingKind::Exact) { - if (existingType->isKnownStdlibCollectionType() && - hasConversions(existingType)) { - continue; - } - } + // FIXME: What is going on here needs to be thoroughly re-evaluated. // If new type has a type variable it shouldn't // be considered viable. From 52355838a5dc43da32ad299c7e6485bbe4cd113a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:29 -0800 Subject: [PATCH 20/72] Revert "[CSOptimizer] Add support for chained members without arguments" This reverts commit 87cd5f873364890ac76a6ef115a3283d2cde8363. --- lib/Sema/CSOptimizer.cpp | 40 +------------------ ...ns_and_operators_with_iou_base_types.swift | 35 ---------------- 2 files changed, 1 insertion(+), 74 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index b7904f09934f1..16fc051b58c80 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -253,37 +253,6 @@ static void findFavoredChoicesBasedOnArity( favoredChoice(choice); } -/// Determine whether the given disjunction serves as a base of -/// another member reference i.e. `x.y` where `x` could be overloaded. -static bool isPartOfMemberChain(ConstraintSystem &CS, Constraint *disjunction) { - if (isOperatorDisjunction(disjunction)) - return false; - - auto &CG = CS.getConstraintGraph(); - - TypeVariableType *typeVar = nullptr; - - // If disjunction is applied, the member is chained on the result. - if (auto appliedFn = CS.getAppliedDisjunctionArgumentFunction(disjunction)) { - typeVar = appliedFn->getResult()->getAs(); - } else { - typeVar = disjunction->getNestedConstraints()[0] - ->getFirstType() - ->getAs(); - } - - if (!typeVar) - return false; - - return llvm::any_of( - CG[typeVar].getConstraints(), [&typeVar](Constraint *constraint) { - if (constraint->getKind() != ConstraintKind::ValueMember) - return false; - - return constraint->getFirstType()->isEqual(typeVar); - }); -} - } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -317,15 +286,8 @@ static void determineBestChoicesInContext( auto applicableFn = getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); - if (applicableFn.isNull()) { - // If this is a chained member reference it could be prioritized since - // it helps to establish context for other calls i.e. `a.b + 2` if - // `a` is a disjunction it should be preferred over `+`. - if (isPartOfMemberChain(cs, disjunction)) - recordResult(disjunction, {/*score=*/1.0}); - + if (applicableFn.isNull()) continue; - } auto argFuncType = applicableFn.get()->getFirstType()->getAs(); diff --git a/validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift b/validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift deleted file mode 100644 index d00b81af40f38..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift +++ /dev/null @@ -1,35 +0,0 @@ -// RUN: %target-swift-frontend(mock-sdk: %clang-importer-sdk) %s -typecheck -solver-expression-time-threshold=1 - -// REQUIRES: OS=macosx,no_asan -// REQUIRES: objc_interop - -import Foundation - -struct CGRect { - var x: CGFloat - - init(x: CGFloat, y: CGFloat, width: CGFloat, height: CGFloat) { } - init(x: Double, y: Double, width: Double, height: Double) { } -} - -protocol View {} - -extension Optional: View where Wrapped: View {} - -extension View { - func frame() -> some View { self } - func frame(x: Int, y: Int, w: Int, z: Int) -> some View { self } - func frame(y: Bool) -> some View { self } -} - -struct NSView { - var frame: CGRect -} - -func test(margin: CGFloat, view: NSView!) -> CGRect { - // `view` is first attempted as `NSView?` and only if that fails is force unwrapped - return CGRect(x: view.frame.x + margin, - y: view.frame.x + margin, - width: view.frame.x - view.frame.x - view.frame.x - (margin * 2), - height: margin) -} From 6fc9f0bbd0a5485a6299f94dcd80a326e1f434cf Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:31 -0800 Subject: [PATCH 21/72] Revert "[CSOptimizer] Mark compiler synthesized disjunctions as optimized" This reverts commit 867e64182f3dc847ca8cb1ba865a81d1d20ece8f. --- lib/Sema/CSOptimizer.cpp | 111 ++++++++++++++++++--------------------- 1 file changed, 51 insertions(+), 60 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 16fc051b58c80..bc8c9bd0b4af9 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -34,19 +34,6 @@ using namespace constraints; namespace { -struct DisjunctionInfo { - /// The score of the disjunction is the highest score from its choices. - /// If the score is nullopt it means that the disjunction is not optimizable. - std::optional Score; - /// The highest scoring choices that could be favored when disjunction - /// is attempted. - llvm::TinyPtrVector FavoredChoices; - - DisjunctionInfo() = default; - DisjunctionInfo(double score, ArrayRef favoredChoices = {}) - : Score(score), FavoredChoices(favoredChoices) {} -}; - // TODO: both `isIntegerType` and `isFloatType` should be available on Type // as `isStdlib{Integer, Float}Type`. @@ -259,30 +246,16 @@ static void findFavoredChoicesBasedOnArity( /// favored choices in the current context. static void determineBestChoicesInContext( ConstraintSystem &cs, SmallVectorImpl &disjunctions, - llvm::DenseMap &result) { + llvm::DenseMap>> + &favorings) { double bestOverallScore = 0.0; - - auto recordResult = [&bestOverallScore, &result](Constraint *disjunction, - DisjunctionInfo &&info) { - bestOverallScore = std::max(bestOverallScore, info.Score.value_or(0)); - result.try_emplace(disjunction, info); - }; + // Tops scores across all of the disjunctions. + llvm::DenseMap disjunctionScores; + llvm::DenseMap> + favoredChoicesPerDisjunction; for (auto *disjunction : disjunctions) { - // If this is a compiler synthesized disjunction, mark it as supported - // and record all of the previously favored choices. Such disjunctions - // include - explicit coercions, IUO references,injected implicit - // initializers for CGFloat<->Double conversions and restrictions with - // multiple choices. - if (disjunction->countFavoredNestedConstraints() > 0) { - DisjunctionInfo info(/*score=*/2.0); - llvm::copy_if(disjunction->getNestedConstraints(), - std::back_inserter(info.FavoredChoices), - [](Constraint *choice) { return choice->isFavored(); }); - recordResult(disjunction, std::move(info)); - continue; - } - auto applicableFn = getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); @@ -309,14 +282,14 @@ static void determineBestChoicesInContext( // of `OverloadedDeclRef` calls were favored purely // based on arity of arguments and parameters matching. { - llvm::TinyPtrVector favoredChoices; - findFavoredChoicesBasedOnArity(cs, disjunction, argumentList, - [&favoredChoices](Constraint *choice) { - favoredChoices.push_back(choice); - }); - - if (!favoredChoices.empty()) { - recordResult(disjunction, {/*score=*/0.01, favoredChoices}); + findFavoredChoicesBasedOnArity( + cs, disjunction, argumentList, [&](Constraint *choice) { + favoredChoicesPerDisjunction[disjunction].push_back(choice); + }); + + if (!favoredChoicesPerDisjunction[disjunction].empty()) { + disjunctionScores[disjunction] = 0.01; + bestOverallScore = std::max(bestOverallScore, 0.01); continue; } } @@ -921,16 +894,17 @@ static void determineBestChoicesInContext( << " with score " << bestScore << "\n"; } - bestOverallScore = std::max(bestOverallScore, bestScore); + // No matching overload choices to favor. + if (bestScore == 0.0) + continue; - DisjunctionInfo info(/*score=*/bestScore); + bestOverallScore = std::max(bestOverallScore, bestScore); + disjunctionScores[disjunction] = bestScore; for (const auto &choice : favoredChoices) { if (choice.second == bestScore) - info.FavoredChoices.push_back(choice.first); + favoredChoicesPerDisjunction[disjunction].push_back(choice.first); } - - recordResult(disjunction, std::move(info)); } if (cs.isDebugMode() && bestOverallScore > 0) { @@ -961,15 +935,14 @@ static void determineBestChoicesInContext( getLogger(/*extraIndent=*/4) << "Best overall score = " << bestOverallScore << '\n'; - for (auto *disjunction : disjunctions) { - auto &entry = result[disjunction]; + for (const auto &entry : disjunctionScores) { getLogger(/*extraIndent=*/4) << "[Disjunction '" - << disjunction->getNestedConstraints()[0]->getFirstType()->getString( + << entry.first->getNestedConstraints()[0]->getFirstType()->getString( PO) - << "' with score = " << entry.Score.value_or(0) << '\n'; + << "' with score = " << entry.second << '\n'; - for (const auto *choice : entry.FavoredChoices) { + for (const auto *choice : favoredChoicesPerDisjunction[entry.first]) { auto &log = getLogger(/*extraIndent=*/6); log << "- "; @@ -982,6 +955,16 @@ static void determineBestChoicesInContext( getLogger() << ")\n"; } + + if (bestOverallScore == 0) + return; + + for (auto &entry : disjunctionScores) { + TinyPtrVector favoredChoices; + for (auto *choice : favoredChoicesPerDisjunction[entry.first]) + favoredChoices.push_back(choice); + favorings[entry.first] = std::make_pair(entry.second, favoredChoices); + } } // Attempt to find a disjunction of bind constraints where all options @@ -1053,7 +1036,9 @@ ConstraintSystem::selectDisjunction() { if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return std::make_pair(disjunction, llvm::TinyPtrVector()); - llvm::DenseMap favorings; + llvm::DenseMap>> + favorings; determineBestChoicesInContext(*this, disjunctions, favorings); // Pick the disjunction with the smallest number of favored, then active @@ -1067,16 +1052,23 @@ ConstraintSystem::selectDisjunction() { auto &[firstScore, firstFavoredChoices] = favorings[first]; auto &[secondScore, secondFavoredChoices] = favorings[second]; + bool isFirstSupported = isSupportedDisjunction(first); + bool isSecondSupported = isSupportedDisjunction(second); + // Rank based on scores only if both disjunctions are supported. - if (firstScore && secondScore) { + if (isFirstSupported && isSecondSupported) { // If both disjunctions have the same score they should be ranked // based on number of favored/active choices. - if (*firstScore != *secondScore) - return *firstScore > *secondScore; + if (firstScore != secondScore) + return firstScore > secondScore; } - unsigned numFirstFavored = firstFavoredChoices.size(); - unsigned numSecondFavored = secondFavoredChoices.size(); + unsigned numFirstFavored = isFirstSupported + ? firstFavoredChoices.size() + : first->countFavoredNestedConstraints(); + unsigned numSecondFavored = + isSecondSupported ? secondFavoredChoices.size() + : second->countFavoredNestedConstraints(); if (numFirstFavored == numSecondFavored) { if (firstActive != secondActive) @@ -1090,8 +1082,7 @@ ConstraintSystem::selectDisjunction() { }); if (bestDisjunction != disjunctions.end()) - return std::make_pair(*bestDisjunction, - favorings[*bestDisjunction].FavoredChoices); + return std::make_pair(*bestDisjunction, favorings[*bestDisjunction].second); return std::nullopt; } From 7f7bbcb34c3a366380314beeaf7bcce1b8f85837 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:33 -0800 Subject: [PATCH 22/72] Revert "[CSOptimizer] Make a light-weight generic overload check if some requirements are unsatisfiable" This reverts commit 15c773b9d7765467e562e03ca1a0d8b8b57b6f62. --- lib/Sema/CSOptimizer.cpp | 67 +++++-------------- .../collections_chained_with_plus.swift.gyb | 18 ----- 2 files changed, 17 insertions(+), 68 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/slow/collections_chained_with_plus.swift.gyb diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index bc8c9bd0b4af9..e6fe100513547 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -555,9 +555,11 @@ static void determineBestChoicesInContext( // Check protocol requirement(s) if this parameter is a // generic parameter type. if (genericSig && paramType->isTypeParameter()) { - // Light-weight check if cases where `checkRequirements` is not - // applicable. - auto checkProtocolRequirementsOnly = [&]() -> double { + // If candidate is not fully resolved or is matched against a + // dependent member type (i.e. `Self.T`), let's check conformances + // only and lower the score. + if (candidateType->hasTypeVariable() || + paramType->is()) { auto protocolRequirements = genericSig->getRequiredProtocols(paramType); if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { @@ -572,64 +574,29 @@ static void determineBestChoicesInContext( } return 0; - }; - - // If candidate is not fully resolved or is matched against a - // dependent member type (i.e. `Self.T`), let's check conformances - // only and lower the score. - if (candidateType->hasTypeVariable() || - paramType->is()) { - return checkProtocolRequirementsOnly(); } // Cannot match anything but generic type parameters here. if (!paramType->is()) return std::nullopt; - bool hasUnsatisfiableRequirements = false; - SmallVector requirements; - - for (const auto &requirement : genericSig.getRequirements()) { - if (hasUnsatisfiableRequirements) - break; - - llvm::SmallPtrSet toExamine; - - auto recordReferencesGenericParams = [&toExamine](Type type) { - type.visit([&toExamine](Type innerTy) { - if (auto *GP = innerTy->getAs()) - toExamine.insert(GP); - }); - }; - - recordReferencesGenericParams(requirement.getFirstType()); + // If the candidate type is fully resolved, let's check all of + // the requirements that are associated with the corresponding + // parameter, if all of them are satisfied this candidate is + // an exact match. - if (requirement.getKind() != RequirementKind::Layout) - recordReferencesGenericParams(requirement.getSecondType()); + auto isParameterType = [¶mType](Type type) { + return type->isEqual(paramType); + }; - if (llvm::any_of(toExamine, [&](GenericTypeParamType *GP) { - return paramType->isEqual(GP); - })) { + SmallVector requirements; + for (const auto &requirement : genericSig.getRequirements()) { + if (requirement.getFirstType().findIf(isParameterType) || + (requirement.getKind() != RequirementKind::Layout && + requirement.getSecondType().findIf(isParameterType))) requirements.push_back(requirement); - // If requirement mentions other generic parameters - // `checkRequirements` would because we don't have - // candidate substitutions for anything but the current - // parameter type. - hasUnsatisfiableRequirements |= toExamine.size() > 1; - } } - // If some of the requirements cannot be satisfied, because - // they reference other generic parameters, for example: - // ``, let's perform a - // light-weight check instead of skipping this overload choice. - if (hasUnsatisfiableRequirements) - return checkProtocolRequirementsOnly(); - - // If the candidate type is fully resolved, let's check all of - // the requirements that are associated with the corresponding - // parameter, if all of them are satisfied this candidate is - // an exact match. auto result = checkRequirements( requirements, [¶mType, &candidateType](SubstitutableType *type) -> Type { diff --git a/validation-test/Sema/type_checker_perf/slow/collections_chained_with_plus.swift.gyb b/validation-test/Sema/type_checker_perf/slow/collections_chained_with_plus.swift.gyb deleted file mode 100644 index 82e558a5a73fb..0000000000000 --- a/validation-test/Sema/type_checker_perf/slow/collections_chained_with_plus.swift.gyb +++ /dev/null @@ -1,18 +0,0 @@ -// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select NumLeafScopes %s -Xfrontend=-typecheck -// REQUIRES: asserts, no_asan - -struct Value: RandomAccessCollection, RangeReplaceableCollection { - let startIndex = 0 - let endIndex = 0 - - subscript(_: Int) -> Int { 0 } - - func replaceSubrange(_: Range, with: C) {} -} - -func f(v: Value) { - let _ = v -%for i in range(0, N): - + v -%end -} From a34b715f131e0d1d3398987b7a043b3fcc53df68 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:35 -0800 Subject: [PATCH 23/72] Revert "[CSOptimizer] Fix `selectDisjunction` to use favored choices even if disjunction was not optimized" This reverts commit c2a55886f05da8c2b5e422d497ce46fe0dde7259. --- lib/Sema/CSOptimizer.cpp | 38 +++++++++++-------- test/Constraints/casts_swift6.swift | 6 +-- ...rals_and_operators_cast_to_float.swift.gyb | 10 ----- 3 files changed, 26 insertions(+), 28 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/array_of_literals_and_operators_cast_to_float.swift.gyb diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index e6fe100513547..93218317a5abd 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -244,7 +244,7 @@ static void findFavoredChoicesBasedOnArity( /// Given a set of disjunctions, attempt to determine /// favored choices in the current context. -static void determineBestChoicesInContext( +static Constraint *determineBestChoicesInContext( ConstraintSystem &cs, SmallVectorImpl &disjunctions, llvm::DenseMap>> @@ -267,14 +267,14 @@ static void determineBestChoicesInContext( auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); if (!argumentList) - return; + return nullptr; for (const auto &argument : *argumentList) { if (auto *expr = argument.getExpr()) { // Directly `<#...#>` or has one inside. if (isa(expr) || cs.containsIDEInspectionTarget(expr)) - return; + return nullptr; } } @@ -924,7 +924,7 @@ static void determineBestChoicesInContext( } if (bestOverallScore == 0) - return; + return nullptr; for (auto &entry : disjunctionScores) { TinyPtrVector favoredChoices; @@ -932,6 +932,19 @@ static void determineBestChoicesInContext( favoredChoices.push_back(choice); favorings[entry.first] = std::make_pair(entry.second, favoredChoices); } + + Constraint *bestDisjunction = nullptr; + for (auto *disjunction : disjunctions) { + if (disjunctionScores[disjunction] != bestOverallScore) + continue; + + if (!bestDisjunction) + bestDisjunction = disjunction; + else // Multiple disjunctions with the same score. + return nullptr; + } + + return bestDisjunction; } // Attempt to find a disjunction of bind constraints where all options @@ -1006,7 +1019,9 @@ ConstraintSystem::selectDisjunction() { llvm::DenseMap>> favorings; - determineBestChoicesInContext(*this, disjunctions, favorings); + if (auto *bestDisjunction = + determineBestChoicesInContext(*this, disjunctions, favorings)) + return std::make_pair(bestDisjunction, favorings[bestDisjunction].second); // Pick the disjunction with the smallest number of favored, then active // choices. @@ -1019,23 +1034,16 @@ ConstraintSystem::selectDisjunction() { auto &[firstScore, firstFavoredChoices] = favorings[first]; auto &[secondScore, secondFavoredChoices] = favorings[second]; - bool isFirstSupported = isSupportedDisjunction(first); - bool isSecondSupported = isSupportedDisjunction(second); - // Rank based on scores only if both disjunctions are supported. - if (isFirstSupported && isSecondSupported) { + if (isSupportedDisjunction(first) && isSupportedDisjunction(second)) { // If both disjunctions have the same score they should be ranked // based on number of favored/active choices. if (firstScore != secondScore) return firstScore > secondScore; } - unsigned numFirstFavored = isFirstSupported - ? firstFavoredChoices.size() - : first->countFavoredNestedConstraints(); - unsigned numSecondFavored = - isSecondSupported ? secondFavoredChoices.size() - : second->countFavoredNestedConstraints(); + unsigned numFirstFavored = firstFavoredChoices.size(); + unsigned numSecondFavored = secondFavoredChoices.size(); if (numFirstFavored == numSecondFavored) { if (firstActive != secondActive) diff --git a/test/Constraints/casts_swift6.swift b/test/Constraints/casts_swift6.swift index 17cbd89100741..5161a493e9dbd 100644 --- a/test/Constraints/casts_swift6.swift +++ b/test/Constraints/casts_swift6.swift @@ -25,9 +25,9 @@ func test_compatibility_coercions(_ arr: [Int], _ optArr: [Int]?, _ dict: [Strin // Make sure we error on the following in Swift 6 mode. _ = id(arr) as [String] // expected-error {{conflicting arguments to generic parameter 'T' ('[Int]' vs. '[String]')}} - _ = (arr ?? []) as [String] // expected-error {{conflicting arguments to generic parameter 'T' ('[Int]' vs. '[String]')}} - _ = (arr ?? [] ?? []) as [String] // expected-error {{conflicting arguments to generic parameter 'T' ('[Int]' vs. '[String]')}} - // expected-error@-1{{conflicting arguments to generic parameter 'T' ('[Int]' vs. '[String]')}} + _ = (arr ?? []) as [String] // expected-error {{conflicting arguments to generic parameter 'T' ('[String]' vs. '[Int]')}} + _ = (arr ?? [] ?? []) as [String] // expected-error {{conflicting arguments to generic parameter 'T' ('[String]' vs. '[Int]')}} + // expected-error@-1{{conflicting arguments to generic parameter 'T' ('[String]' vs. '[Int]')}} _ = (optArr ?? []) as [String] // expected-error {{conflicting arguments to generic parameter 'T' ('[Int]' vs. '[String]'}} _ = (arr ?? []) as [String]? // expected-error {{'[Int]' is not convertible to '[String]?'}} diff --git a/validation-test/Sema/type_checker_perf/fast/array_of_literals_and_operators_cast_to_float.swift.gyb b/validation-test/Sema/type_checker_perf/fast/array_of_literals_and_operators_cast_to_float.swift.gyb deleted file mode 100644 index 2cd48795d50c5..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/array_of_literals_and_operators_cast_to_float.swift.gyb +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %scale-test --begin 1 --end 10 --step 1 --select NumLeafScopes %s -// REQUIRES: asserts,no_asan - -let _ = [ - 0, -%for i in range(2, N+2): - 1/${i}, -%end - 1 -] as [Float] From 21bb23612ba0bbaa8ddf1da109c79d56d965b154 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:37 -0800 Subject: [PATCH 24/72] Revert "[CSOptimizer] Limit "old" behavior compatibility to unlabeled unary arguments" This reverts commit 9fb73143f6e6462bc9664a00fc00c72e9f70a241. --- lib/Sema/CSOptimizer.cpp | 33 +++++++------------ .../old_hack_related_ambiguities.swift | 11 ------- 2 files changed, 11 insertions(+), 33 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 93218317a5abd..a1d768d76a97a 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -181,15 +181,6 @@ static OverloadedDeclRefExpr *isOverloadedDeclRef(Constraint *disjunction) { return nullptr; } -static unsigned numOverloadChoicesMatchingOnArity(OverloadedDeclRefExpr *ODRE, - ArgumentList *arguments) { - return llvm::count_if(ODRE->getDecls(), [&arguments](auto *choice) { - if (auto *paramList = getParameterList(choice)) - return arguments->size() == paramList->size(); - return false; - }); -} - /// This maintains an "old hack" behavior where overloads of some /// `OverloadedDeclRef` calls were favored purely based on number of /// argument and (non-defaulted) parameters matching. @@ -200,7 +191,11 @@ static void findFavoredChoicesBasedOnArity( if (!ODRE) return; - if (numOverloadChoicesMatchingOnArity(ODRE, argumentList) > 1) + if (llvm::count_if(ODRE->getDecls(), [&argumentList](auto *choice) { + if (auto *paramList = getParameterList(choice)) + return argumentList->size() == paramList->size(); + return false; + }) > 1) return; auto isVariadicGenericOverload = [&](ValueDecl *choice) { @@ -636,16 +631,7 @@ static Constraint *determineBestChoicesInContext( double bestScore = 0.0; SmallVector, 2> favoredChoices; - // Preserves old behavior where, for unary calls, the solver - // would not consider choices that didn't match on the number - // of parameters (regardless of defaults) and only exact - // matches were favored. - bool preserveFavoringOfUnlabeledUnaryArgument = false; - if (argumentList->isUnlabeledUnary()) { - auto ODRE = isOverloadedDeclRef(disjunction); - preserveFavoringOfUnlabeledUnaryArgument = - !ODRE || numOverloadChoicesMatchingOnArity(ODRE, argumentList) < 2; - } + bool isOverloadedDeclRefDisjunction = isOverloadedDeclRef(disjunction); forEachDisjunctionChoice( cs, disjunction, @@ -668,8 +654,11 @@ static Constraint *determineBestChoicesInContext( // matches to filter out non-default literal bindings which otherwise // could cause "over-favoring". bool favorExactMatchesOnly = onlyLiteralCandidates; - - if (preserveFavoringOfUnlabeledUnaryArgument) { + // Preserves old behavior where for unary calls to members + // the solver would not consider choices that didn't match on + // the number of parameters (regardless of defaults) and only + // exact matches were favored. + if (!isOverloadedDeclRefDisjunction && argumentList->size() == 1) { // Old behavior completely disregarded the fact that some of // the parameters could be defaulted. if (overloadType->getNumParams() != 1) diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index da8da45cc76dc..36c1c99c3b1ed 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -43,17 +43,6 @@ do { do { struct S { let n: Int - - func test(v: String) -> Int { } - func test(v: String, flag: Bool = false) -> Int? { } - - - func verify(v: String) -> Int? { - guard let _ = test(v: v) else { // Ok - return nil - } - return 0 - } } func f(_: String, _ p: Bool = false) -> S? { From 18008ce32d8aa21064666d632a297f8335224ddc Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:39 -0800 Subject: [PATCH 25/72] Revert "[Tests] NFC: Update a couple of type-checker tests" This reverts commit ff8663ff161cc96cc92e897618dd56c529915038. --- .../{slow => fast}/borderline_flat_map_operator_mix.swift | 7 ++----- .../{fast => slow}/nil_coalescing.swift.gyb | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/borderline_flat_map_operator_mix.swift (50%) rename validation-test/Sema/type_checker_perf/{fast => slow}/nil_coalescing.swift.gyb (58%) diff --git a/validation-test/Sema/type_checker_perf/slow/borderline_flat_map_operator_mix.swift b/validation-test/Sema/type_checker_perf/fast/borderline_flat_map_operator_mix.swift similarity index 50% rename from validation-test/Sema/type_checker_perf/slow/borderline_flat_map_operator_mix.swift rename to validation-test/Sema/type_checker_perf/fast/borderline_flat_map_operator_mix.swift index b88a73d90121f..cca07954da499 100644 --- a/validation-test/Sema/type_checker_perf/slow/borderline_flat_map_operator_mix.swift +++ b/validation-test/Sema/type_checker_perf/fast/borderline_flat_map_operator_mix.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 +// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=10 // REQUIRES: no_asan @@ -10,11 +10,8 @@ struct S { } } -// Note: One possible approach to this issue would be to determine when the array literal inside of the inner closure -// doesn't have any other possible bindings but Array and attempt it at that point. That would fail overload of flatMap -// that returns an optional value. func f(x: Array, y: Range) -> [S] { - return x.flatMap { z in // expected-error {{the compiler is unable to type-check this expression in reasonable time}} + return x.flatMap { z in return ((y.lowerBound / 1)...(y.upperBound + 1) / 1).flatMap { w in return [S(1 * Double(w) + 1.0 + z.t), S(1 * Double(w) + 1.0 - z.t)] diff --git a/validation-test/Sema/type_checker_perf/fast/nil_coalescing.swift.gyb b/validation-test/Sema/type_checker_perf/slow/nil_coalescing.swift.gyb similarity index 58% rename from validation-test/Sema/type_checker_perf/fast/nil_coalescing.swift.gyb rename to validation-test/Sema/type_checker_perf/slow/nil_coalescing.swift.gyb index af8639d9bc159..2ef5c8f16a3d7 100644 --- a/validation-test/Sema/type_checker_perf/fast/nil_coalescing.swift.gyb +++ b/validation-test/Sema/type_checker_perf/slow/nil_coalescing.swift.gyb @@ -1,4 +1,4 @@ -// RUN: %scale-test --begin 1 --end 10 --step 1 --select NumLeafScopes %s +// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select NumLeafScopes %s // REQUIRES: asserts,no_asan func t(_ x: Int?) -> Int { From 202d15bc0f60928220897e93ffd383b989629116 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:41 -0800 Subject: [PATCH 26/72] Revert "[Tests] NFC: Move simd related test-case from `slow` to `fast`" This reverts commit 28396a6dce2eaff21eb6eaffb34275dfff6e4d35. --- .../{fast => slow}/simd_scalar_multiple.swift.gyb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename validation-test/Sema/type_checker_perf/{fast => slow}/simd_scalar_multiple.swift.gyb (70%) diff --git a/validation-test/Sema/type_checker_perf/fast/simd_scalar_multiple.swift.gyb b/validation-test/Sema/type_checker_perf/slow/simd_scalar_multiple.swift.gyb similarity index 70% rename from validation-test/Sema/type_checker_perf/fast/simd_scalar_multiple.swift.gyb rename to validation-test/Sema/type_checker_perf/slow/simd_scalar_multiple.swift.gyb index 1d5e4f9e0ea6f..497afd10785f5 100644 --- a/validation-test/Sema/type_checker_perf/fast/simd_scalar_multiple.swift.gyb +++ b/validation-test/Sema/type_checker_perf/slow/simd_scalar_multiple.swift.gyb @@ -1,4 +1,4 @@ -// RUN: %scale-test --begin 1 --end 5 --step 1 --select NumLeafScopes %s +// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select NumLeafScopes %s // REQUIRES: asserts,no_asan From 14d4469d52350a0433acb0a2a462481265a2f165 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:42 -0800 Subject: [PATCH 27/72] Revert "[CSGen] NFC: Remove obsolete `ConstraintSystem::{get, set}FavoredType`" This reverts commit 8bd288447f378034d68168f3eccd3433c206940c. --- include/swift/Sema/ConstraintSystem.h | 13 +++++++++++++ lib/Sema/CSGen.cpp | 10 ++++++++++ 2 files changed, 23 insertions(+) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index e6c93009e6d82..2114b5f2edc92 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2250,6 +2250,10 @@ class ConstraintSystem { llvm::SetVector TypeVariables; + /// Maps expressions to types for choosing a favored overload + /// type in a disjunction constraint. + llvm::DenseMap FavoredTypes; + /// Maps discovered closures to their types inferred /// from declared parameters/result and body. /// @@ -2985,6 +2989,15 @@ class ConstraintSystem { return nullptr; } + TypeBase* getFavoredType(Expr *E) { + assert(E != nullptr); + return this->FavoredTypes[E]; + } + void setFavoredType(Expr *E, TypeBase *T) { + assert(E != nullptr); + this->FavoredTypes[E] = T; + } + /// Set the type in our type map for the given node, and record the change /// in the trail. /// diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index b94d96491b54c..0e40d0bbacb5d 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -403,6 +403,7 @@ namespace { Type fixedOutputType = CS.getFixedTypeRecursive(outputTy, /*wantRValue=*/false); if (!fixedOutputType->isTypeVariableOrMember()) { + CS.setFavoredType(anchor, fixedOutputType.getPointer()); outputTy = fixedOutputType; } @@ -803,6 +804,11 @@ namespace { getParentPackExpansionExpr(E)); } } + + if (!knownType->hasPlaceholder()) { + // Set the favored type for this expression to the known type. + CS.setFavoredType(E, knownType.getPointer()); + } } } @@ -1281,6 +1287,9 @@ namespace { CS.getASTContext()); } + if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) { + CS.setFavoredType(expr, favoredTy); + } return CS.getType(expr->getSubExpr()); } @@ -2555,6 +2564,7 @@ namespace { Type fixedType = CS.getFixedTypeRecursive(resultType, /*wantRvalue=*/true); if (!fixedType->isTypeVariableOrMember()) { + CS.setFavoredType(expr, fixedType.getPointer()); resultType = fixedType; } From c3a23599ca06015bdd69f010195325694a494b8c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:44 -0800 Subject: [PATCH 28/72] Revert "[CSOptimizer] Allow literal arguments to match parameters that conform to `ExpressibleBy{Integer, Float}Literal`" This reverts commit 2fdd4b6c35ae5c7763307cd16b335030ae537d84. --- lib/Sema/CSOptimizer.cpp | 23 ++----------------- .../old_hack_related_ambiguities.swift | 9 -------- 2 files changed, 2 insertions(+), 30 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index a1d768d76a97a..cd71f1c14645b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -481,22 +481,6 @@ static Constraint *determineBestChoicesInContext( } if (options.contains(MatchFlag::Literal)) { - // Integer and floating-point literals can match any parameter - // type that conforms to `ExpressibleBy{Integer, Float}Literal` - // protocol but since that would constitute a non-default binding - // the score has to be slightly lowered. - if (!paramType->hasTypeParameter()) { - if (candidateType->isInt() && - TypeChecker::conformsToKnownProtocol( - paramType, KnownProtocolKind::ExpressibleByIntegerLiteral)) - return 0.2; - - if (candidateType->isDouble() && - TypeChecker::conformsToKnownProtocol( - paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) - return 0.2; - } - return 0; } @@ -650,10 +634,7 @@ static Constraint *determineBestChoicesInContext( if (!matchings) return; - // If all of the arguments are literals, let's prioritize exact - // matches to filter out non-default literal bindings which otherwise - // could cause "over-favoring". - bool favorExactMatchesOnly = onlyLiteralCandidates; + bool favorExactMatchesOnly = false; // Preserves old behavior where for unary calls to members // the solver would not consider choices that didn't match on // the number of parameters (regardless of defaults) and only @@ -831,7 +812,7 @@ static Constraint *determineBestChoicesInContext( [&resultTy](const auto ¶m) { return param.getPlainType()->isEqual(resultTy); })) - score += 0.1; + score += 0.001; } favoredChoices.push_back({choice, score}); diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index 36c1c99c3b1ed..d1dc81b514fab 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -237,12 +237,3 @@ do { assert(s.isEmpty, "") // Ok } } - -extension Double { - public static func * (left: Float, right: Double) -> Double { 0 } -} - -func test_non_default_literal_use(arg: Float) { - let v = arg * 2.0 // shouldn't use `(Float, Double) -> Double` overload - let _: Float = v // Ok -} From 722fe0bd9a48e17d48c6ee0aacdfe3656d4deb84 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:46 -0800 Subject: [PATCH 29/72] Revert "[CSOptimizer] Adjust `scoreCandidateMatch` to indicate when match cannot be decided" This reverts commit 9b62c84a4f44b9ecdd9bbb8df642b04b9d2d4ddb. --- lib/Sema/CSOptimizer.cpp | 41 ++++++++++++---------------------------- 1 file changed, 12 insertions(+), 29 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index cd71f1c14645b..39f58378686df 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -450,16 +450,10 @@ static Constraint *determineBestChoicesInContext( // - Array-to-pointer conversion // - Value to existential conversion // - Exact match on top-level types - // - // In situations when it's not possible to determine whether a candidate - // type matches a parameter type (i.e. when partially resolved generic - // types are matched) this function is going to produce \c std::nullopt - // instead of `0` that indicates "not a match". - std::function(GenericSignature, Type, Type, - MatchOptions)> - scoreCandidateMatch = - [&](GenericSignature genericSig, Type candidateType, Type paramType, - MatchOptions options) -> std::optional { + std::function + scoreCandidateMatch = [&](GenericSignature genericSig, + Type candidateType, Type paramType, + MatchOptions options) -> double { auto areEqual = [&options](Type a, Type b) { // Double<->CGFloat implicit conversion support for literals // only since in this case the conversion might not result in @@ -533,12 +527,10 @@ static Constraint *determineBestChoicesInContext( // Check protocol requirement(s) if this parameter is a // generic parameter type. - if (genericSig && paramType->isTypeParameter()) { - // If candidate is not fully resolved or is matched against a - // dependent member type (i.e. `Self.T`), let's check conformances - // only and lower the score. - if (candidateType->hasTypeVariable() || - paramType->is()) { + if (genericSig && paramType->is()) { + // If candidate is not fully resolved, check conformances only + // and lower the score. + if (candidateType->hasTypeVariable()) { auto protocolRequirements = genericSig->getRequiredProtocols(paramType); if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { @@ -555,10 +547,6 @@ static Constraint *determineBestChoicesInContext( return 0; } - // Cannot match anything but generic type parameters here. - if (!paramType->is()) - return std::nullopt; - // If the candidate type is fully resolved, let's check all of // the requirements that are associated with the corresponding // parameter, if all of them are satisfied this candidate is @@ -730,15 +718,10 @@ static Constraint *determineBestChoicesInContext( if (favorExactMatchesOnly) options |= MatchFlag::ExactOnly; - auto candidateScore = scoreCandidateMatch( - genericSig, candidateType, paramType, options); - - if (!candidateScore) - continue; - - if (candidateScore > 0) { - bestCandidateScore = - std::max(bestCandidateScore, candidateScore.value()); + auto score = scoreCandidateMatch(genericSig, candidateType, + paramType, options); + if (score > 0) { + bestCandidateScore = std::max(bestCandidateScore, score); continue; } From b0a27707cdf667076b6322f2105cda02aefdc9e4 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:48 -0800 Subject: [PATCH 30/72] Revert "[CSOptimizer] Fix Double<->CGFloat implicit conversion support when arguments are literals" This reverts commit 6caf1ccbb2671dbd628ef81877e7fd6ad1def4f7. --- lib/Sema/CSOptimizer.cpp | 16 +++------------- .../implicit_double_cgfloat_conversion.swift | 5 ----- 2 files changed, 3 insertions(+), 18 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 39f58378686df..ae589d9289401 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -454,15 +454,7 @@ static Constraint *determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> double { - auto areEqual = [&options](Type a, Type b) { - // Double<->CGFloat implicit conversion support for literals - // only since in this case the conversion might not result in - // score penalty. - if (options.contains(MatchFlag::Literal) && - ((a->isDouble() && b->isCGFloat()) || - (a->isCGFloat() && b->isDouble()))) - return true; - + auto areEqual = [](Type a, Type b) { return a->getDesugaredType()->isEqual(b->getDesugaredType()); }; @@ -470,13 +462,11 @@ static Constraint *determineBestChoicesInContext( return areEqual(candidateType, paramType) ? 1 : 0; // Exact match between candidate and parameter types. - if (areEqual(candidateType, paramType)) { + if (areEqual(candidateType, paramType)) return options.contains(MatchFlag::Literal) ? 0.3 : 1; - } - if (options.contains(MatchFlag::Literal)) { + if (options.contains(MatchFlag::Literal)) return 0; - } // Check whether match would require optional injection. { diff --git a/test/Constraints/implicit_double_cgfloat_conversion.swift b/test/Constraints/implicit_double_cgfloat_conversion.swift index 83c6955851b58..22011f0019214 100644 --- a/test/Constraints/implicit_double_cgfloat_conversion.swift +++ b/test/Constraints/implicit_double_cgfloat_conversion.swift @@ -361,8 +361,3 @@ do { return v // Ok } } - -func test_cgfloat_operator_is_attempted_with_literal_arguments(v: CGFloat?) { - let ratio = v ?? (2.0 / 16.0) - let _: CGFloat = ratio // Ok -} From d69f64aac02a1ccf9a0a2d1a5e8088854d9a5064 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:50 -0800 Subject: [PATCH 31/72] Revert "[CSOptimizer] A more comprehensive generic overload checking when candidates are fully resolved" This reverts commit e30587bda4c24afa56cd1e91f0d1407c41253a0a. --- lib/Sema/CSOptimizer.cpp | 79 ++++++++++------------------------------ 1 file changed, 20 insertions(+), 59 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index ae589d9289401..d15397991876c 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -479,13 +479,8 @@ static Constraint *determineBestChoicesInContext( if (!candidateOptionals.empty() || !paramOptionals.empty()) { if (paramOptionals.size() >= candidateOptionals.size()) { - auto score = scoreCandidateMatch(genericSig, candidateType, - paramType, options); - // Injection lowers the score slightly to comply with - // old behavior where exact matches on operator parameter - // types were always preferred. - return score == 1 && isOperatorDisjunction(disjunction) ? 0.9 - : score; + return scoreCandidateMatch(genericSig, candidateType, paramType, + options); } // Optionality mismatch. @@ -517,60 +512,26 @@ static Constraint *determineBestChoicesInContext( // Check protocol requirement(s) if this parameter is a // generic parameter type. - if (genericSig && paramType->is()) { - // If candidate is not fully resolved, check conformances only - // and lower the score. - if (candidateType->hasTypeVariable()) { - auto protocolRequirements = - genericSig->getRequiredProtocols(paramType); - if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { - return bool(cs.lookupConformance(candidateType, protocol)); - })) { - if (auto *GP = paramType->getAs()) { - auto *paramDecl = GP->getDecl(); - if (paramDecl && paramDecl->isOpaqueType()) - return 1.0; - } - return 0.7; + if (genericSig && paramType->isTypeParameter()) { + auto protocolRequirements = genericSig->getRequiredProtocols(paramType); + // It's a generic parameter or dependent member which might + // be connected via ame-type constraints to other generic + // parameters or dependent member but we cannot check that here, + // so let's add a tiny score just to acknowledge that it could + // possibly match. + if (protocolRequirements.empty()) + return 0.01; + + if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { + return bool(cs.lookupConformance(candidateType, protocol)); + })) { + if (auto *GP = paramType->getAs()) { + auto *paramDecl = GP->getDecl(); + if (paramDecl && paramDecl->isOpaqueType()) + return 1.0; } - - return 0; - } - - // If the candidate type is fully resolved, let's check all of - // the requirements that are associated with the corresponding - // parameter, if all of them are satisfied this candidate is - // an exact match. - - auto isParameterType = [¶mType](Type type) { - return type->isEqual(paramType); - }; - - SmallVector requirements; - for (const auto &requirement : genericSig.getRequirements()) { - if (requirement.getFirstType().findIf(isParameterType) || - (requirement.getKind() != RequirementKind::Layout && - requirement.getSecondType().findIf(isParameterType))) - requirements.push_back(requirement); + return 0.7; } - - auto result = checkRequirements( - requirements, - [¶mType, &candidateType](SubstitutableType *type) -> Type { - if (type->isEqual(paramType)) - return candidateType; - return ErrorType::get(type); - }, - SubstOptions(std::nullopt)); - - // Concrete operator overloads are always more preferable to - // generic ones if there are exact or subtype matches, for - // everything else the solver should try both concrete and - // generic and disambiguate during ranking. - if (result == CheckRequirementsResult::Success) - return isOperatorDisjunction(disjunction) ? 0.9 : 1.0; - - return 0; } // Parameter is generic, let's check whether top-level From 24d3f2a6cf797b3585466a474c582b7b18fc935a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:52 -0800 Subject: [PATCH 32/72] Revert "[CSOptimizer] Restore old hack behavior which used to favor overloads based on arity matches" This reverts commit a3a3ec4fe054c07e3d004b62fb4d0733a8bbd97d. --- lib/Sema/CSOptimizer.cpp | 84 ++----------------- .../old_hack_related_ambiguities.swift | 50 ----------- test/Constraints/ranking.swift | 8 ++ 3 files changed, 13 insertions(+), 129 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index d15397991876c..39ccdc7588dd7 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -172,67 +172,9 @@ void forEachDisjunctionChoice( } } -static OverloadedDeclRefExpr *isOverloadedDeclRef(Constraint *disjunction) { +static bool isOverloadedDeclRef(Constraint *disjunction) { assert(disjunction->getKind() == ConstraintKind::Disjunction); - - auto *locator = disjunction->getLocator(); - if (locator->getPath().empty()) - return getAsExpr(locator->getAnchor()); - return nullptr; -} - -/// This maintains an "old hack" behavior where overloads of some -/// `OverloadedDeclRef` calls were favored purely based on number of -/// argument and (non-defaulted) parameters matching. -static void findFavoredChoicesBasedOnArity( - ConstraintSystem &cs, Constraint *disjunction, ArgumentList *argumentList, - llvm::function_ref favoredChoice) { - auto *ODRE = isOverloadedDeclRef(disjunction); - if (!ODRE) - return; - - if (llvm::count_if(ODRE->getDecls(), [&argumentList](auto *choice) { - if (auto *paramList = getParameterList(choice)) - return argumentList->size() == paramList->size(); - return false; - }) > 1) - return; - - auto isVariadicGenericOverload = [&](ValueDecl *choice) { - auto genericContext = choice->getAsGenericContext(); - if (!genericContext) - return false; - - auto *GPL = genericContext->getGenericParams(); - if (!GPL) - return false; - - return llvm::any_of(GPL->getParams(), [&](const GenericTypeParamDecl *GP) { - return GP->isParameterPack(); - }); - }; - - bool hasVariadicGenerics = false; - SmallVector favored; - - forEachDisjunctionChoice( - cs, disjunction, - [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { - if (isVariadicGenericOverload(decl)) - hasVariadicGenerics = true; - - if (overloadType->getNumParams() == argumentList->size() || - llvm::count_if(*getParameterList(decl), [](auto *param) { - return !param->isDefaultArgument(); - }) == argumentList->size()) - favored.push_back(choice); - }); - - if (hasVariadicGenerics) - return; - - for (auto *choice : favored) - favoredChoice(choice); + return disjunction->getLocator()->directlyAt(); } } // end anonymous namespace @@ -251,6 +193,9 @@ static Constraint *determineBestChoicesInContext( favoredChoicesPerDisjunction; for (auto *disjunction : disjunctions) { + if (!isSupportedDisjunction(disjunction)) + continue; + auto applicableFn = getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); @@ -273,25 +218,6 @@ static Constraint *determineBestChoicesInContext( } } - // This maintains an "old hack" behavior where overloads - // of `OverloadedDeclRef` calls were favored purely - // based on arity of arguments and parameters matching. - { - findFavoredChoicesBasedOnArity( - cs, disjunction, argumentList, [&](Constraint *choice) { - favoredChoicesPerDisjunction[disjunction].push_back(choice); - }); - - if (!favoredChoicesPerDisjunction[disjunction].empty()) { - disjunctionScores[disjunction] = 0.01; - bestOverallScore = std::max(bestOverallScore, 0.01); - continue; - } - } - - if (!isSupportedDisjunction(disjunction)) - continue; - SmallVector argsWithLabels; { argsWithLabels.append(argFuncType->getParams().begin(), diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index d1dc81b514fab..f30bfddfaf313 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -187,53 +187,3 @@ do { let result = test(10, accuracy: 1) let _: Int = result } - -// swift-distributed-tracing snippet that relies on old hack behavior. -protocol TracerInstant { -} - -extension Int: TracerInstant {} - -do { - enum SpanKind { - case `internal` - } - - func withSpan( - _ operationName: String, - at instant: @autoclosure () -> Instant, - context: @autoclosure () -> Int = 0, - ofKind kind: SpanKind = .internal - ) {} - - func withSpan( - _ operationName: String, - context: @autoclosure () -> Int = 0, - ofKind kind: SpanKind = .internal, - at instant: @autoclosure () -> some TracerInstant = 42 - ) {} - - withSpan("", at: 0) // Ok -} - -protocol ForAssert { - var isEmpty: Bool { get } -} - -extension ForAssert { - var isEmpty: Bool { false } -} - -do { - func assert(_ condition: @autoclosure () -> Bool, _ message: @autoclosure () -> String = String(), file: StaticString = #file, line: UInt = #line) {} - func assert(_ condition: Bool, _ message: @autoclosure () -> String, file: StaticString = #file, line: UInt = #line) {} - func assert(_ condition: Bool, file: StaticString = #fileID, line: UInt = #line) {} - - struct S : ForAssert { - var isEmpty: Bool { false } - } - - func test(s: S) { - assert(s.isEmpty, "") // Ok - } -} diff --git a/test/Constraints/ranking.swift b/test/Constraints/ranking.swift index 82d29791e6f14..74e3b6842f02e 100644 --- a/test/Constraints/ranking.swift +++ b/test/Constraints/ranking.swift @@ -450,3 +450,11 @@ struct HasIntInit { func compare_solutions_with_bindings(x: UInt8, y: UInt8) -> HasIntInit { return .init(Int(x / numericCast(y))) } + +// Test to make sure that previous favoring behavior is maintained and @autoclosure makes a difference. +func test_no_ambiguity_with_autoclosure(x: Int) { + func test(_ condition: Bool, file: StaticString = #file, line: UInt = #line) {} + func test(_ condition: @autoclosure () -> Bool, file: StaticString = #file, line: UInt = #line) {} + + test(x >= 0) // Ok +} From 3c1e2b408c9db8460142696bf4f7c3d8fb235917 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:53 -0800 Subject: [PATCH 33/72] Revert "[CSOptimizer] Desugar types before checking for equality" This reverts commit 802f5cd105f3d9ab3132056ecd48a458dfbe7547. --- lib/Sema/CSOptimizer.cpp | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 39ccdc7588dd7..acc31e43bd3a2 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -380,15 +380,11 @@ static Constraint *determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> double { - auto areEqual = [](Type a, Type b) { - return a->getDesugaredType()->isEqual(b->getDesugaredType()); - }; - if (options.contains(MatchFlag::ExactOnly)) - return areEqual(candidateType, paramType) ? 1 : 0; + return candidateType->isEqual(paramType) ? 1 : 0; // Exact match between candidate and parameter types. - if (areEqual(candidateType, paramType)) + if (candidateType->isEqual(paramType)) return options.contains(MatchFlag::Literal) ? 0.3 : 1; if (options.contains(MatchFlag::Literal)) From 169e71fd5638a295bb5a0e82ebb18746bec81ccd Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:55 -0800 Subject: [PATCH 34/72] Revert "[ConstraintSystem] Narrowly disable `tryOptimizeGenericDisjunction` when some of the arguments are number literals" This reverts commit 8d5cb112efdc86d736f8cc7fda9b3e43a23d2fdb. --- include/swift/Sema/ConstraintSystem.h | 4 ---- lib/Sema/CSSolver.cpp | 21 ------------------- lib/Sema/TypeCheckConstraints.cpp | 4 ---- .../old_hack_related_ambiguities.swift | 9 -------- 4 files changed, 38 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 2114b5f2edc92..dc93a1568870b 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -488,10 +488,6 @@ class TypeVariableType::Implementation { /// literal (represented by `ArrayExpr` and `DictionaryExpr` in AST). bool isCollectionLiteralType() const; - /// Determine whether this type variable represents a literal such - /// as an integer value, a floating-point value with and without a sign. - bool isNumberLiteralType() const; - /// Determine whether this type variable represents a result type of a /// function call. bool isFunctionResult() const; diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 1e9d4e9bb4a25..8ba8d4e3586b2 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1390,27 +1390,6 @@ tryOptimizeGenericDisjunction(ConstraintSystem &cs, Constraint *disjunction, return nullptr; } - // Don't attempt this optimization if call has number literals. - // This is intended to narrowly fix situations like: - // - // func test(_: T) { ... } - // func test(_: T) { ... } - // - // test(42) - // - // The call should use `` overload even though the - // `` is a more specialized version because - // selecting `` doesn't introduce non-default literal - // types. - if (auto *argFnType = cs.getAppliedDisjunctionArgumentFunction(disjunction)) { - if (llvm::any_of( - argFnType->getParams(), [](const AnyFunctionType::Param ¶m) { - auto *typeVar = param.getPlainType()->getAs(); - return typeVar && typeVar->getImpl().isNumberLiteralType(); - })) - return nullptr; - } - llvm::SmallVector choices; for (auto *choice : constraints) { if (choices.size() > 2) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index cb44d7ef92fcb..ff597da3036ac 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -204,10 +204,6 @@ bool TypeVariableType::Implementation::isCollectionLiteralType() const { locator->directlyAt()); } -bool TypeVariableType::Implementation::isNumberLiteralType() const { - return locator && locator->directlyAt(); -} - bool TypeVariableType::Implementation::isFunctionResult() const { return locator && locator->isLastElement(); } diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index f30bfddfaf313..67feaabab5836 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -178,12 +178,3 @@ do { } } -// `tryOptimizeGenericDisjunction` is too aggressive sometimes, make sure that `` -// overload is _not_ selected in this case. -do { - func test(_ expression1: @autoclosure () throws -> T, accuracy: T) -> T {} - func test(_ expression1: @autoclosure () throws -> T, accuracy: T) -> T {} - - let result = test(10, accuracy: 1) - let _: Int = result -} From 3c85f81580fe5e378d605661fc537d8316fad08e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:57 -0800 Subject: [PATCH 35/72] Revert "[CSOptimizer] Infer argument candidates from calls to `Double` and CGFloat constructors" This reverts commit f2a6677a6d42dbda51d70e54c38a3c101985b4e6. --- include/swift/Sema/ConstraintSystem.h | 4 ---- lib/Sema/CSOptimizer.cpp | 17 ----------------- lib/Sema/TypeCheckConstraints.cpp | 4 ---- .../implicit_double_cgfloat_conversion.swift | 19 ------------------- 4 files changed, 44 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index dc93a1568870b..9675ffe85a304 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -488,10 +488,6 @@ class TypeVariableType::Implementation { /// literal (represented by `ArrayExpr` and `DictionaryExpr` in AST). bool isCollectionLiteralType() const; - /// Determine whether this type variable represents a result type of a - /// function call. - bool isFunctionResult() const; - /// Retrieve the representative of the equivalence class to which this /// type variable belongs. /// diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index acc31e43bd3a2..232acd0f9ea6d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -250,23 +250,6 @@ static Constraint *determineBestChoicesInContext( {literal.second.getDefaultType(), /*fromLiteral=*/true}); } } - - // Helps situations like `1 + {Double, CGFloat}(...)` by inferring - // a type for the second operand of `+` based on a type being constructed. - // - // Currently limited to Double and CGFloat only since we need to - // support implicit `Double<->CGFloat` conversion. - if (typeVar->getImpl().isFunctionResult() && - isOperatorDisjunction(disjunction)) { - auto resultLoc = typeVar->getImpl().getLocator(); - if (auto *call = getAsExpr(resultLoc->getAnchor())) { - if (auto *typeExpr = dyn_cast(call->getFn())) { - auto instanceTy = cs.getType(typeExpr)->getMetatypeInstanceType(); - if (instanceTy->isDouble() || instanceTy->isCGFloat()) - types.push_back({instanceTy, /*fromLiteral=*/false}); - } - } - } } else { types.push_back({argType, /*fromLiteral=*/false}); } diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index ff597da3036ac..016a1c90a162f 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -204,10 +204,6 @@ bool TypeVariableType::Implementation::isCollectionLiteralType() const { locator->directlyAt()); } -bool TypeVariableType::Implementation::isFunctionResult() const { - return locator && locator->isLastElement(); -} - void *operator new(size_t bytes, ConstraintSystem& cs, size_t alignment) { return cs.getAllocator().Allocate(bytes, alignment); diff --git a/test/Constraints/implicit_double_cgfloat_conversion.swift b/test/Constraints/implicit_double_cgfloat_conversion.swift index 22011f0019214..32126db0864d0 100644 --- a/test/Constraints/implicit_double_cgfloat_conversion.swift +++ b/test/Constraints/implicit_double_cgfloat_conversion.swift @@ -342,22 +342,3 @@ func test_init_validation() { } } } - -do { - struct G { - init(_: T) {} - } - - func round(_: Double) -> Double {} - func round(_: T) -> T {} - - func test_cgfloat_over_double(withColors colors: Int, size: CGSize) -> G { - let g = G(1.0 / CGFloat(colors)) - return g // Ok - } - - func test_no_ambiguity(width: Int, height: Int) -> CGFloat { - let v = round(CGFloat(width / height) * 10) / 10.0 - return v // Ok - } -} From a316a444ffa0ae6f332b06f2c07be6ef3550d62d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:20:59 -0800 Subject: [PATCH 36/72] Revert "[CSOptimizer] Score all of the overload choices matching on literals uniformly" This reverts commit 59109c2d602e7ea48d0c41cb9918d961dba16211. --- lib/Sema/CSOptimizer.cpp | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 232acd0f9ea6d..ad393a47d1b5d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -268,19 +268,6 @@ static Constraint *determineBestChoicesInContext( resultTypes.push_back(resultType); } - // Determine whether all of the argument candidates are inferred from literals. - // This information is going to be used later on when we need to decide how to - // score a matching choice. - bool onlyLiteralCandidates = - argFuncType->getNumParams() > 0 && - llvm::none_of( - indices(argFuncType->getParams()), [&](const unsigned argIdx) { - auto &candidates = candidateArgumentTypes[argIdx]; - return llvm::any_of(candidates, [&](const auto &candidate) { - return !candidate.second; - }); - }); - // Match arguments to the given overload choice. auto matchArguments = [&](OverloadChoice choice, FunctionType *overloadType) -> std::optional { @@ -605,18 +592,6 @@ static Constraint *determineBestChoicesInContext( // parameters. score /= (overloadType->getNumParams() - numDefaulted); - // Make sure that the score is uniform for all disjunction - // choices that match on literals only, this would make sure that - // in operator chains that consist purely of literals we'd - // always prefer outermost disjunction instead of innermost - // one. - // - // Preferring outer disjunction first works better in situations - // when contextual type for the whole chain becomes available at - // some point during solving at it would allow for faster pruning. - if (score > 0 && onlyLiteralCandidates) - score = 0.1; - // If one of the result types matches exactly, that's a good // indication that overload choice should be favored. // From 6278adf82d4b0049b573dc021ee7fca09d0c49ef Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:02 -0800 Subject: [PATCH 37/72] Revert "[CSOptimizer] Enable ranking of `Int*`, `Float{80}` and `Double` initializers" This reverts commit 6fb6d1cf90c94865df46fc1557990e0640a1164e. --- lib/Sema/CSOptimizer.cpp | 30 ------------------------------ 1 file changed, 30 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index ad393a47d1b5d..04215455d3d54 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -34,20 +34,6 @@ using namespace constraints; namespace { -// TODO: both `isIntegerType` and `isFloatType` should be available on Type -// as `isStdlib{Integer, Float}Type`. - -static bool isIntegerType(Type type) { - return type->isInt() || type->isInt8() || type->isInt16() || - type->isInt32() || type->isInt64() || type->isUInt() || - type->isUInt8() || type->isUInt16() || type->isUInt32() || - type->isUInt64(); -} - -static bool isFloatType(Type type) { - return type->isFloat() || type->isDouble() || type->isFloat80(); -} - static bool isSupportedOperator(Constraint *disjunction) { if (!isOperatorDisjunction(disjunction)) return false; @@ -71,16 +57,6 @@ static bool isSupportedOperator(Constraint *disjunction) { return false; } -static bool isSupportedSpecialConstructor(ConstructorDecl *ctor) { - if (auto *selfDecl = ctor->getImplicitSelfDecl()) { - auto selfTy = selfDecl->getInterfaceType(); - /// Support `Int*`, `Float*` and `Double` initializers since their generic - /// overloads are not too complicated. - return selfTy && (isIntegerType(selfTy) || isFloatType(selfTy)); - } - return false; -} - static bool isStandardComparisonOperator(ValueDecl *decl) { return decl->isOperator() && decl->getBaseIdentifier().isStandardComparisonOperator(); @@ -96,12 +72,6 @@ static bool isSupportedDisjunction(Constraint *disjunction) { if (isSupportedOperator(disjunction)) return true; - if (auto *ctor = dyn_cast_or_null( - getOverloadChoiceDecl(choices.front()))) { - if (isSupportedSpecialConstructor(ctor)) - return true; - } - // Non-operator disjunctions are supported only if they don't // have any generic choices. return llvm::all_of(choices, [&](Constraint *choice) { From 3b6ddcb11e000a049d893ef3c77c5fc1a8014921 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:04 -0800 Subject: [PATCH 38/72] Revert "[CSOptimizer] Rank disjunctions based on score only if both sides are supported" This reverts commit 8818d399f9b14dd0fd8bc325f4427246a660c824. --- lib/Sema/CSOptimizer.cpp | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 04215455d3d54..99ea31b244c40 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -677,9 +677,6 @@ static Constraint *determineBestChoicesInContext( getLogger() << ")\n"; } - if (bestOverallScore == 0) - return nullptr; - for (auto &entry : disjunctionScores) { TinyPtrVector favoredChoices; for (auto *choice : favoredChoicesPerDisjunction[entry.first]) @@ -687,6 +684,9 @@ static Constraint *determineBestChoicesInContext( favorings[entry.first] = std::make_pair(entry.second, favoredChoices); } + if (bestOverallScore == 0) + return nullptr; + Constraint *bestDisjunction = nullptr; for (auto *disjunction : disjunctions) { if (disjunctionScores[disjunction] != bestOverallScore) @@ -788,13 +788,8 @@ ConstraintSystem::selectDisjunction() { auto &[firstScore, firstFavoredChoices] = favorings[first]; auto &[secondScore, secondFavoredChoices] = favorings[second]; - // Rank based on scores only if both disjunctions are supported. - if (isSupportedDisjunction(first) && isSupportedDisjunction(second)) { - // If both disjunctions have the same score they should be ranked - // based on number of favored/active choices. - if (firstScore != secondScore) - return firstScore > secondScore; - } + if (firstScore > secondScore) + return true; unsigned numFirstFavored = firstFavoredChoices.size(); unsigned numSecondFavored = secondFavoredChoices.size(); From c90dd9991051a50c3af35f30d556fb514f866f6c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:06 -0800 Subject: [PATCH 39/72] Revert "[CSOptimizer] Rank results of operators regardless of whether anything is known about parameters" This reverts commit 3996b25fbdc70566990c33a6f431ca645336cac7. --- lib/Sema/CSOptimizer.cpp | 28 ++++++++----------- ...or_chain_with_hetergeneous_arguments.swift | 1 + 2 files changed, 12 insertions(+), 17 deletions(-) rename validation-test/Sema/type_checker_perf/{fast => slow}/operator_chain_with_hetergeneous_arguments.swift (90%) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 99ea31b244c40..ca2e5de42eaed 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -57,15 +57,6 @@ static bool isSupportedOperator(Constraint *disjunction) { return false; } -static bool isStandardComparisonOperator(ValueDecl *decl) { - return decl->isOperator() && - decl->getBaseIdentifier().isStandardComparisonOperator(); -} - -static bool isArithmeticOperator(ValueDecl *decl) { - return decl->isOperator() && decl->getBaseIdentifier().isArithmeticOperator(); -} - static bool isSupportedDisjunction(Constraint *disjunction) { auto choices = disjunction->getNestedConstraints(); @@ -570,7 +561,9 @@ static Constraint *determineBestChoicesInContext( // ones that all have the same result type), regular // functions/methods and especially initializers could end up // with a lot of favored overloads because on the result type alone. - if (decl->isOperator() && !isStandardComparisonOperator(decl)) { + if (score > 0 || + (decl->isOperator() && + !decl->getBaseIdentifier().isStandardComparisonOperator())) { if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) { return scoreCandidateMatch(genericSig, overloadType->getResult(), @@ -582,14 +575,15 @@ static Constraint *determineBestChoicesInContext( } if (score > 0) { - // Nudge the score slightly to prefer concrete homogeneous - // arithmetic operators. - // - // This is an opportunistic optimization based on the operator - // use patterns where homogeneous operators are the most - // heavily used ones. - if (isArithmeticOperator(decl) && + if (decl->isOperator() && + decl->getBaseIdentifier().isArithmeticOperator() && overloadType->getNumParams() == 2) { + // Nudge the score slightly to prefer concrete homogeneous + // operators. + // + // This is an opportunistic optimization based on the operator + // use patterns where homogeneous operators are the most + // heavily used ones. auto resultTy = overloadType->getResult(); if (!resultTy->hasTypeParameter() && llvm::all_of(overloadType->getParams(), diff --git a/validation-test/Sema/type_checker_perf/fast/operator_chain_with_hetergeneous_arguments.swift b/validation-test/Sema/type_checker_perf/slow/operator_chain_with_hetergeneous_arguments.swift similarity index 90% rename from validation-test/Sema/type_checker_perf/fast/operator_chain_with_hetergeneous_arguments.swift rename to validation-test/Sema/type_checker_perf/slow/operator_chain_with_hetergeneous_arguments.swift index 787509fb565b2..5d6c8c4440ece 100644 --- a/validation-test/Sema/type_checker_perf/fast/operator_chain_with_hetergeneous_arguments.swift +++ b/validation-test/Sema/type_checker_perf/slow/operator_chain_with_hetergeneous_arguments.swift @@ -5,4 +5,5 @@ func test(bytes: Int, length: UInt32) { // left-hand side of `>=` is `Int` and right-hand side is a chain of `UInt32` inferred from `length` _ = bytes >= 1 + 2 + 3 + 4 + 5 + 6 + 7 + 8 + 9 + 10 + length + // expected-error@-1 {{reasonable time}} } From d75bd3364fcba393f5ee896005ac1111930e1ba9 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:08 -0800 Subject: [PATCH 40/72] Revert "[Tests] NFC: Add more test-cases that were previously solved due to old hacks behavior" This reverts commit d0ff6c81b818a76e78463bec2613354498a4af76. --- .../old_hack_related_ambiguities.swift | 117 ------------------ 1 file changed, 117 deletions(-) diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index 67feaabab5836..d2db5a2e36632 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -61,120 +61,3 @@ do { } } -// ambiguities related to ~= -protocol _Error: Error {} - -extension _Error { - public static func ~=(lhs: Self, rhs: Self) -> Bool { - false - } - - public static func ~=(lhs: Error, rhs: Self) -> Bool { - false - } - - public static func ~=(lhs: Self, rhs: Error) -> Bool { - false - } -} - -enum CustomError { - case A -} - -extension CustomError: _Error {} - -func f(e: CustomError) { - if e ~= CustomError.A {} -} - -// Generic overload should be preferred over concrete one because the latter is non-default literal -struct Pattern {} - -func ~= (pattern: Pattern, value: String) -> Bool { - return false -} - -extension Pattern: ExpressibleByStringLiteral { - init(stringLiteral value: String) {} -} - -func test_default_tilda(v: String) { - _ = "hi" ~= v // Ok -} - -struct UUID {} - -struct LogKey { - init(base: some CustomStringConvertible, foo: Int = 0) { - } - - init(base: UUID, foo: Int = 0) { - } -} - -@available(swift 99) -extension LogKey { - init(base: String?) { - } - - init(base: UUID?) { - } -} - -func test_that_unavailable_init_is_not_used(x: String?) { - _ = LogKey(base: x ?? "??") -} - -// error: value of optional type 'UID?' must be unwrapped to a value of type 'UID' -struct S: Comparable { - static func <(lhs: Self, rhs: Self) -> Bool { - false - } -} - -func max(_ a: S?, _ b: S?) -> S? { - nil -} - -func test_stdlib_max_selection(s: S) -> S { - let new = max(s, s) - return new // Ok -} - -// error: initializer for conditional binding must have Optional type, not 'UnsafeMutablePointer' -do { - struct TestPointerConversions { - var p: UnsafeMutableRawPointer { get { fatalError() } } - - func f(_ p: UnsafeMutableRawPointer) { - guard let x = UnsafeMutablePointer(OpaquePointer(self.p)) else { - return - } - _ = x - - guard let x = UnsafeMutablePointer(OpaquePointer(p)) else { - return - } - _ = x - } - } -} - -// error: initializer 'init(_:)' requires that 'T' conform to 'BinaryInteger' -do { - struct Config { - subscript(_ key: String) -> T? { nil } - subscript(_ key: String) -> Any? { nil } - } - - struct S { - init(maxQueueDepth: UInt) {} - } - - func f(config: Config) { - let maxQueueDepth = config["hi"] ?? 256 - _ = S(maxQueueDepth: UInt(maxQueueDepth)) - } -} - From 3145a02c1605aa134c4f218c4bb3abda996ea4c8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:10 -0800 Subject: [PATCH 41/72] Revert "[CSOptimizer] Average score should reflect number of defaulted parameters" This reverts commit 23589add740de550c0de8a9cd9c887d66ad1ca72. --- lib/Sema/CSOptimizer.cpp | 11 ++-------- .../old_hack_related_ambiguities.swift | 22 ------------------- 2 files changed, 2 insertions(+), 31 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index ca2e5de42eaed..8d3217db645dd 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -441,7 +441,6 @@ static Constraint *determineBestChoicesInContext( } double score = 0.0; - unsigned numDefaulted = 0; for (unsigned paramIdx = 0, n = overloadType->getNumParams(); paramIdx != n; ++paramIdx) { const auto ¶m = overloadType->getParams()[paramIdx]; @@ -449,8 +448,7 @@ static Constraint *determineBestChoicesInContext( auto argIndices = matchings->parameterBindings[paramIdx]; switch (argIndices.size()) { case 0: - // Current parameter is defaulted, mark and continue. - ++numDefaulted; + // Current parameter is defaulted. continue; case 1: @@ -544,14 +542,9 @@ static Constraint *determineBestChoicesInContext( score += bestCandidateScore; } - // An overload whether all of the parameters are defaulted - // that's called without arguments. - if (numDefaulted == overloadType->getNumParams()) - return; - // Average the score to avoid disfavoring disjunctions with fewer // parameters. - score /= (overloadType->getNumParams() - numDefaulted); + score /= overloadType->getNumParams(); // If one of the result types matches exactly, that's a good // indication that overload choice should be favored. diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index d2db5a2e36632..30743800d8642 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -39,25 +39,3 @@ do { } } -// error: initializer for conditional binding must have Optional type, not 'S' -do { - struct S { - let n: Int - } - - func f(_: String, _ p: Bool = false) -> S? { - nil - } - - func f(_ x: String) -> S { - fatalError() - } - - func g(_ x: String) -> Int? { - guard let y = f(x) else { - return nil - } - return y.n - } -} - From 11af1bb518fabb9af5ed2413bb7d26698557c883 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:12 -0800 Subject: [PATCH 42/72] Revert "[Tests] NFC: Adjust a couple of improved tests" This reverts commit 66981364fedcefde9018a83c3ae5b4913d930e85. --- validation-test/stdlib/FixedPointDiagnostics.swift.gyb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validation-test/stdlib/FixedPointDiagnostics.swift.gyb b/validation-test/stdlib/FixedPointDiagnostics.swift.gyb index 2b2df2812c0d2..e091805af22b7 100644 --- a/validation-test/stdlib/FixedPointDiagnostics.swift.gyb +++ b/validation-test/stdlib/FixedPointDiagnostics.swift.gyb @@ -53,8 +53,8 @@ func testMixedSignArithmetic() { Stride(1) - ${T}(0) // expected-error {{}} expected-note {{}} var y: Stride = 0 - y += ${T}(1) // expected-error {{cannot convert value of type '${T}' to expected argument type 'Stride' (aka 'Int')}} {{10-10=Stride(}} - y -= ${T}(1) // expected-error {{cannot convert value of type '${T}' to expected argument type 'Stride' (aka 'Int')}} {{10-10=Stride(}} + y += ${T}(1) // expected-error {{cannot convert value of type '${T}' to expected argument type 'Int'}} {{10-10=Int(}} + y -= ${T}(1) // expected-error {{cannot convert value of type '${T}' to expected argument type 'Int'}} {{10-10=Int(}} } % end } From 213428ba673314c7e05d44bb1ab50c109fcfa1a8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:13 -0800 Subject: [PATCH 43/72] Revert "[CSOptimizer] Don't optimize (implicit) calls with code completion arguments" This reverts commit 8a918e23692c5a0dd4dde3da20e77f8c9165541b. --- lib/Sema/CSOptimizer.cpp | 11 +---------- test/IDE/complete_operators.swift | 2 +- 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 8d3217db645dd..e3106043155fc 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -167,18 +167,9 @@ static Constraint *determineBestChoicesInContext( applicableFn.get()->getFirstType()->getAs(); auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); - if (!argumentList) + if (!argumentList || cs.containsIDEInspectionTarget(argumentList)) return nullptr; - for (const auto &argument : *argumentList) { - if (auto *expr = argument.getExpr()) { - // Directly `<#...#>` or has one inside. - if (isa(expr) || - cs.containsIDEInspectionTarget(expr)) - return nullptr; - } - } - SmallVector argsWithLabels; { argsWithLabels.append(argFuncType->getParams().begin(), diff --git a/test/IDE/complete_operators.swift b/test/IDE/complete_operators.swift index 483d3cc6225d8..a349fd4ecf8da 100644 --- a/test/IDE/complete_operators.swift +++ b/test/IDE/complete_operators.swift @@ -52,7 +52,7 @@ func testPostfix6() { func testPostfix7() { 1 + 2 * 3.0#^POSTFIX_7^# } -// POSTFIX_7: Decl[PostfixOperatorFunction]/CurrModule/TypeRelation[Convertible]: ***[#Double#] +// POSTFIX_7: Decl[PostfixOperatorFunction]/CurrModule: ***[#Double#] func testPostfix8(x: S) { x#^POSTFIX_8^# From b893e8bd5d2e0de17e160b86fca3804b410b24d2 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:15 -0800 Subject: [PATCH 44/72] Revert "[CSOptimizer] attempt to rank only standard/simd operators and fully concrete overload sets" This reverts commit deca9b61c549dabcef8cf20dfcaa4ea9f33ae5cb. --- lib/Sema/CSOptimizer.cpp | 54 ++++++---------------------------------- 1 file changed, 8 insertions(+), 46 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index e3106043155fc..f0236407244ad 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -34,48 +34,6 @@ using namespace constraints; namespace { -static bool isSupportedOperator(Constraint *disjunction) { - if (!isOperatorDisjunction(disjunction)) - return false; - - auto choices = disjunction->getNestedConstraints(); - auto *decl = getOverloadChoiceDecl(choices.front()); - - auto name = decl->getBaseIdentifier(); - if (name.isArithmeticOperator() || name.isStandardComparisonOperator() || - name.is("^")) { - return true; - } - - // Operators like &<<, &>>, &+, .== etc. - if (llvm::any_of(choices, [](Constraint *choice) { - return isSIMDOperator(getOverloadChoiceDecl(choice)); - })) { - return true; - } - - return false; -} - -static bool isSupportedDisjunction(Constraint *disjunction) { - auto choices = disjunction->getNestedConstraints(); - - if (isSupportedOperator(disjunction)) - return true; - - // Non-operator disjunctions are supported only if they don't - // have any generic choices. - return llvm::all_of(choices, [&](Constraint *choice) { - if (choice->getKind() != ConstraintKind::BindOverload) - return false; - - if (auto *decl = getOverloadChoiceDecl(choice)) - return decl->getInterfaceType()->is(); - - return false; - }); -} - NullablePtr getApplicableFnConstraint(ConstraintGraph &CG, Constraint *disjunction) { auto *boundVar = disjunction->getNestedConstraints()[0] @@ -154,9 +112,6 @@ static Constraint *determineBestChoicesInContext( favoredChoicesPerDisjunction; for (auto *disjunction : disjunctions) { - if (!isSupportedDisjunction(disjunction)) - continue; - auto applicableFn = getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); @@ -382,7 +337,7 @@ static Constraint *determineBestChoicesInContext( // types match i.e. Array as a parameter. // // This is slightly better than all of the conformances matching - // because the parameter is concrete and could split the graph. + // because the parameter is concrete and could split the graph. if (paramType->hasTypeParameter()) { auto *candidateDecl = candidateType->getAnyNominal(); auto *paramDecl = paramType->getAnyNominal(); @@ -410,6 +365,13 @@ static Constraint *determineBestChoicesInContext( } else if (auto *SD = dyn_cast(decl)) { genericSig = SD->getGenericSignature(); } + + // Let's not consider non-operator generic overloads because we + // need conformance checking functionality to determine best + // favoring, preferring such overloads based on concrete types + // alone leads to subpar choices due to missed information. + if (genericSig && !decl->isOperator()) + return; } auto matchings = From 4a1ca5962e55ac872cdf83e6abd4942c001c1368 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:17 -0800 Subject: [PATCH 45/72] Revert "[CSOptimizer] Record best scores for each disjunction and use them in `selectDisjunction`" This reverts commit 3819ddfb4010ac0eaa9c8ba68e8f4d82d3ed9344. --- lib/Sema/CSOptimizer.cpp | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index f0236407244ad..d4402eb2dcabd 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -102,8 +102,7 @@ static bool isOverloadedDeclRef(Constraint *disjunction) { /// favored choices in the current context. static Constraint *determineBestChoicesInContext( ConstraintSystem &cs, SmallVectorImpl &disjunctions, - llvm::DenseMap>> + llvm::DenseMap> &favorings) { double bestOverallScore = 0.0; // Tops scores across all of the disjunctions. @@ -618,10 +617,11 @@ static Constraint *determineBestChoicesInContext( } for (auto &entry : disjunctionScores) { - TinyPtrVector favoredChoices; + if (entry.second != bestOverallScore) + continue; + for (auto *choice : favoredChoicesPerDisjunction[entry.first]) - favoredChoices.push_back(choice); - favorings[entry.first] = std::make_pair(entry.second, favoredChoices); + favorings[entry.first].push_back(choice); } if (bestOverallScore == 0) @@ -710,12 +710,10 @@ ConstraintSystem::selectDisjunction() { if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return std::make_pair(disjunction, llvm::TinyPtrVector()); - llvm::DenseMap>> - favorings; + llvm::DenseMap> favorings; if (auto *bestDisjunction = determineBestChoicesInContext(*this, disjunctions, favorings)) - return std::make_pair(bestDisjunction, favorings[bestDisjunction].second); + return std::make_pair(bestDisjunction, favorings[bestDisjunction]); // Pick the disjunction with the smallest number of favored, then active // choices. @@ -724,29 +722,21 @@ ConstraintSystem::selectDisjunction() { [&](Constraint *first, Constraint *second) -> bool { unsigned firstActive = first->countActiveNestedConstraints(); unsigned secondActive = second->countActiveNestedConstraints(); + unsigned firstFavored = favorings[first].size(); + unsigned secondFavored = favorings[second].size(); - auto &[firstScore, firstFavoredChoices] = favorings[first]; - auto &[secondScore, secondFavoredChoices] = favorings[second]; - - if (firstScore > secondScore) - return true; - - unsigned numFirstFavored = firstFavoredChoices.size(); - unsigned numSecondFavored = secondFavoredChoices.size(); - - if (numFirstFavored == numSecondFavored) { + if (firstFavored == secondFavored) { if (firstActive != secondActive) return firstActive < secondActive; } - numFirstFavored = numFirstFavored ? numFirstFavored : firstActive; - numSecondFavored = numSecondFavored ? numSecondFavored : secondActive; - - return numFirstFavored < numSecondFavored; + firstFavored = firstFavored ? firstFavored : firstActive; + secondFavored = secondFavored ? secondFavored : secondActive; + return firstFavored < secondFavored; }); if (bestDisjunction != disjunctions.end()) - return std::make_pair(*bestDisjunction, favorings[*bestDisjunction].second); + return std::make_pair(*bestDisjunction, favorings[*bestDisjunction]); return std::nullopt; } From ac4b6a485fd79cc196016afb88302de51be9dddf Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:18 -0800 Subject: [PATCH 46/72] Revert "[CSOptimizer] Let `determineBestChoicesInContext` return the best disjunction if one is available" This reverts commit cf05405eae56384f6021eaec6facfcc938b1edc0. --- lib/Sema/CSOptimizer.cpp | 24 +++--------------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index d4402eb2dcabd..73bb0880b9a8f 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -100,7 +100,7 @@ static bool isOverloadedDeclRef(Constraint *disjunction) { /// Given a set of disjunctions, attempt to determine /// favored choices in the current context. -static Constraint *determineBestChoicesInContext( +static void determineBestChoicesInContext( ConstraintSystem &cs, SmallVectorImpl &disjunctions, llvm::DenseMap> &favorings) { @@ -122,7 +122,7 @@ static Constraint *determineBestChoicesInContext( auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); if (!argumentList || cs.containsIDEInspectionTarget(argumentList)) - return nullptr; + return; SmallVector argsWithLabels; { @@ -623,22 +623,6 @@ static Constraint *determineBestChoicesInContext( for (auto *choice : favoredChoicesPerDisjunction[entry.first]) favorings[entry.first].push_back(choice); } - - if (bestOverallScore == 0) - return nullptr; - - Constraint *bestDisjunction = nullptr; - for (auto *disjunction : disjunctions) { - if (disjunctionScores[disjunction] != bestOverallScore) - continue; - - if (!bestDisjunction) - bestDisjunction = disjunction; - else // Multiple disjunctions with the same score. - return nullptr; - } - - return bestDisjunction; } // Attempt to find a disjunction of bind constraints where all options @@ -711,9 +695,7 @@ ConstraintSystem::selectDisjunction() { return std::make_pair(disjunction, llvm::TinyPtrVector()); llvm::DenseMap> favorings; - if (auto *bestDisjunction = - determineBestChoicesInContext(*this, disjunctions, favorings)) - return std::make_pair(bestDisjunction, favorings[bestDisjunction]); + determineBestChoicesInContext(*this, disjunctions, favorings); // Pick the disjunction with the smallest number of favored, then active // choices. From 81fea7f4da6cd6aa4ad221f4087ce2e1f510d8c4 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:20 -0800 Subject: [PATCH 47/72] Revert "[CSOptimizer] Emulate old behavior related to favoring of unary calls to members" This reverts commit 527de22bec2b28308f8f9d292d098f923bb15c08. --- lib/Sema/CSOptimizer.cpp | 33 ++------------- .../old_hack_related_ambiguities.swift | 41 ------------------- 2 files changed, 3 insertions(+), 71 deletions(-) delete mode 100644 test/Constraints/old_hack_related_ambiguities.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 73bb0880b9a8f..7b2b35270f27f 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -91,11 +91,6 @@ void forEachDisjunctionChoice( } } -static bool isOverloadedDeclRef(Constraint *disjunction) { - assert(disjunction->getKind() == ConstraintKind::Disjunction); - return disjunction->getLocator()->directlyAt(); -} - } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -237,7 +232,6 @@ static void determineBestChoicesInContext( enum class MatchFlag { OnParam = 0x01, Literal = 0x02, - ExactOnly = 0x04, }; using MatchOptions = OptionSet; @@ -256,9 +250,6 @@ static void determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> double { - if (options.contains(MatchFlag::ExactOnly)) - return candidateType->isEqual(paramType) ? 1 : 0; - // Exact match between candidate and parameter types. if (candidateType->isEqual(paramType)) return options.contains(MatchFlag::Literal) ? 0.3 : 1; @@ -276,17 +267,17 @@ static void determineBestChoicesInContext( paramType = paramType->lookThroughAllOptionalTypes(paramOptionals); if (!candidateOptionals.empty() || !paramOptionals.empty()) { - if (paramOptionals.size() >= candidateOptionals.size()) { + if (paramOptionals.size() >= candidateOptionals.size()) return scoreCandidateMatch(genericSig, candidateType, paramType, options); - } // Optionality mismatch. return 0; } } - // Candidate could be converted to a superclass. + // Candidate could be injected into optional parameter type + // or converted to a superclass. if (isSubclassOf(candidateType, paramType)) return 1; @@ -352,8 +343,6 @@ static void determineBestChoicesInContext( double bestScore = 0.0; SmallVector, 2> favoredChoices; - bool isOverloadedDeclRefDisjunction = isOverloadedDeclRef(disjunction); - forEachDisjunctionChoice( cs, disjunction, [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { @@ -378,20 +367,6 @@ static void determineBestChoicesInContext( if (!matchings) return; - bool favorExactMatchesOnly = false; - // Preserves old behavior where for unary calls to members - // the solver would not consider choices that didn't match on - // the number of parameters (regardless of defaults) and only - // exact matches were favored. - if (!isOverloadedDeclRefDisjunction && argumentList->size() == 1) { - // Old behavior completely disregarded the fact that some of - // the parameters could be defaulted. - if (overloadType->getNumParams() != 1) - return; - - favorExactMatchesOnly = true; - } - double score = 0.0; for (unsigned paramIdx = 0, n = overloadType->getNumParams(); paramIdx != n; ++paramIdx) { @@ -469,8 +444,6 @@ static void determineBestChoicesInContext( MatchOptions options(MatchFlag::OnParam); if (isLiteralDefault) options |= MatchFlag::Literal; - if (favorExactMatchesOnly) - options |= MatchFlag::ExactOnly; auto score = scoreCandidateMatch(genericSig, candidateType, paramType, options); diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift deleted file mode 100644 index 30743800d8642..0000000000000 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ /dev/null @@ -1,41 +0,0 @@ -// RUN: %target-typecheck-verify-swift - -func entity(_: Int) -> Int { - 0 -} - -struct Test { - func test(_ v: Int) -> Int { v } - func test(_ v: Int?) -> Int? { v } -} - -func test_ternary_literal(v: Test) -> Int? { - true ? v.test(0) : nil // Ok -} - -func test_ternary(v: Test) -> Int? { - true ? v.test(entity(0)) : nil // Ok -} - -do { - struct TestFloat { - func test(_ v: Float) -> Float { v } // expected-note {{found this candidate}} - func test(_ v: Float?) -> Float? { v } // expected-note {{found this candidate}} - } - - func test_ternary_non_default_literal(v: TestFloat) -> Float? { - true ? v.test(1.0) : nil // expected-error {{ambiguous use of 'test'}} - } -} - -do { - struct Test { - init(a: Int, b: Int = 0) throws {} - init?(a: Int?) {} - } - - func test(v: Int) -> Test? { - return Test(a: v) // Ok - } -} - From e601140052721cb4724ae34b00406340aefbe1d5 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:22 -0800 Subject: [PATCH 48/72] Revert "[Tests] NFC: Add a test-case for rdar://133340307 which is now fast" This reverts commit 670127abd65fdbbee985d46051701761d35e2983. --- .../fast/rdar133340307.swift | 134 ------------------ 1 file changed, 134 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/rdar133340307.swift diff --git a/validation-test/Sema/type_checker_perf/fast/rdar133340307.swift b/validation-test/Sema/type_checker_perf/fast/rdar133340307.swift deleted file mode 100644 index 9740b4d15f06c..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/rdar133340307.swift +++ /dev/null @@ -1,134 +0,0 @@ -// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -// REQUIRES: tools-release,no_asan - -public protocol Applicative {} - -public struct Kind {} - -public extension Applicative { - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } - - static func product (_ fa:Kind, _ fb:Kind) -> Kind { - fatalError() - } -} - -public extension Applicative { - static func zip (_ fa0:Kind, _ fa1:Kind, _ fa2:Kind, _ fa3:Kind, _ fa4:Kind, _ fa5:Kind, _ fa6:Kind, _ fa7:Kind, _ fa8:Kind, _ fa9:Kind, _ fa10:Kind, _ fa11:Kind, _ fa12:Kind, _ fa13:Kind, _ fa14:Kind, _ fa15:Kind, _ fa16:Kind, _ fa17:Kind, _ fa18:Kind, _ fa19:Kind, _ fa20:Kind, _ fa21:Kind, _ fa22:Kind, _ fa23:Kind, _ fa24:Kind, _ fa25:Kind, _ fa26:Kind, _ fa27:Kind, _ fa28:Kind, _ fa29:Kind) -> Kind { - product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(product(fa0, fa1), fa2), fa3), fa4), fa5), fa6), fa7), fa8), fa9), fa10), fa11), fa12), fa13), fa14), fa15), fa16), fa17), fa18), fa19), fa20), fa21), fa22), fa23), fa24), fa25), fa26), fa27), fa28), fa29) // Ok - } -} From 48d635bcad02275d8face8abfc1513b0fef2126d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:23 -0800 Subject: [PATCH 49/72] Revert "[CSOptimizer] Prefer homogeneous arithmetic operator overloads when argument(s) or result match" This reverts commit d69b6a059494dafc3ce917fa4f34959f2cf05589. --- lib/Sema/CSOptimizer.cpp | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 7b2b35270f27f..2b74e03dbb8f9 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -493,24 +493,6 @@ static void determineBestChoicesInContext( } if (score > 0) { - if (decl->isOperator() && - decl->getBaseIdentifier().isArithmeticOperator() && - overloadType->getNumParams() == 2) { - // Nudge the score slightly to prefer concrete homogeneous - // operators. - // - // This is an opportunistic optimization based on the operator - // use patterns where homogeneous operators are the most - // heavily used ones. - auto resultTy = overloadType->getResult(); - if (!resultTy->hasTypeParameter() && - llvm::all_of(overloadType->getParams(), - [&resultTy](const auto ¶m) { - return param.getPlainType()->isEqual(resultTy); - })) - score += 0.001; - } - favoredChoices.push_back({choice, score}); bestScore = std::max(bestScore, score); } From 90bcaeb73f1bba293cf5408f4fb42b311e8fa037 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:25 -0800 Subject: [PATCH 50/72] Revert "[CSOptimizer] Remove an outdated optimization to compare resolved argument types with all else equal" This reverts commit 1760bd1f1ec002885195bd154ce9d0a26c9d7879. --- include/swift/Sema/Constraint.h | 5 +++++ lib/Sema/CSOptimizer.cpp | 7 +++++++ lib/Sema/Constraint.cpp | 11 +++++++++++ 3 files changed, 23 insertions(+) diff --git a/include/swift/Sema/Constraint.h b/include/swift/Sema/Constraint.h index 41e6aebe4e52a..382a213c54d1a 100644 --- a/include/swift/Sema/Constraint.h +++ b/include/swift/Sema/Constraint.h @@ -818,6 +818,11 @@ class Constraint final : public llvm::ilist_node, }); } + /// Returns the number of resolved argument types for an applied disjunction + /// constraint. This is always zero for disjunctions that do not represent + /// an applied overload. + unsigned countResolvedArgumentTypes(ConstraintSystem &cs) const; + /// Determine if this constraint represents explicit conversion, /// e.g. coercion constraint "as X" which forms a disjunction. bool isExplicitConversion() const; diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 2b74e03dbb8f9..f1656dc30c15d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -662,9 +662,16 @@ ConstraintSystem::selectDisjunction() { unsigned firstFavored = favorings[first].size(); unsigned secondFavored = favorings[second].size(); + // Everything else equal, choose the disjunction with the greatest + // number of resolved argument types. The number of resolved argument + // types is always zero for disjunctions that don't represent applied + // overloads. if (firstFavored == secondFavored) { if (firstActive != secondActive) return firstActive < secondActive; + + return first->countResolvedArgumentTypes(*this) > + second->countResolvedArgumentTypes(*this); } firstFavored = firstFavored ? firstFavored : firstActive; diff --git a/lib/Sema/Constraint.cpp b/lib/Sema/Constraint.cpp index e44f4873a1f8f..be1a062bc5c5b 100644 --- a/lib/Sema/Constraint.cpp +++ b/lib/Sema/Constraint.cpp @@ -703,6 +703,17 @@ gatherReferencedTypeVars(Constraint *constraint, } } +unsigned Constraint::countResolvedArgumentTypes(ConstraintSystem &cs) const { + auto *argumentFuncType = cs.getAppliedDisjunctionArgumentFunction(this); + if (!argumentFuncType) + return 0; + + return llvm::count_if(argumentFuncType->getParams(), [&](const AnyFunctionType::Param arg) { + auto argType = cs.getFixedTypeRecursive(arg.getPlainType(), /*wantRValue=*/true); + return !argType->isTypeVariableOrMember(); + }); +} + bool Constraint::isExplicitConversion() const { assert(Kind == ConstraintKind::Disjunction); From f6a6d19d526bb1fe9dff62d3c611a50e01774442 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:27 -0800 Subject: [PATCH 51/72] Revert "[CSOptimizer] NFC: Switch from llvm::Optional to std::optional post-rebase" This reverts commit c429f5b9ec30adbbfd0eef1edafe1e09503d071d. --- include/swift/Sema/ConstraintSystem.h | 2 +- lib/Sema/CSOptimizer.cpp | 21 +++++++++++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 9675ffe85a304..63fe82222ae6f 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5247,7 +5247,7 @@ class ConstraintSystem { /// Pick a disjunction from the InactiveConstraints list. /// /// \returns The selected disjunction and a set of it's favored choices. - std::optional>> + llvm::Optional>> selectDisjunction(); /// Pick a conjunction from the InactiveConstraints list. diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index f1656dc30c15d..bab3703d1027e 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -171,7 +171,7 @@ static void determineBestChoicesInContext( // Match arguments to the given overload choice. auto matchArguments = [&](OverloadChoice choice, FunctionType *overloadType) - -> std::optional { + -> llvm::Optional { auto *decl = choice.getDeclOrNull(); assert(decl); @@ -186,7 +186,7 @@ static void determineBestChoicesInContext( return matchCallArguments(argsWithLabels, overloadType->getParams(), paramListInfo, argumentList->getFirstTrailingClosureIndex(), - /*allow fixes*/ false, listener, std::nullopt); + /*allow fixes*/ false, listener, llvm::None); }; // Determine whether the candidate type is a subclass of the superclass @@ -209,14 +209,15 @@ static void determineBestChoicesInContext( return false; return llvm::all_of(layout.getProtocols(), [&](ProtocolDecl *P) { - if (auto superclass = P->getSuperclassDecl()) { - if (!isSubclassOf(candidateType, - superclass->getDeclaredInterfaceType())) + if (auto superclass = P->getSuperclass()) { + if (!isSubclassOf(candidateType, superclass)) return false; } - return bool(TypeChecker::containsProtocol(candidateType, P, - /*allowMissing=*/false)); + return bool(TypeChecker::containsProtocol( + candidateType, P, cs.DC->getParentModule(), + /*skipConditionalRequirements=*/true, + /*allowMissing=*/false)); }); } @@ -638,13 +639,13 @@ selectBestBindingDisjunction(ConstraintSystem &cs, return firstBindDisjunction; } -std::optional>> +llvm::Optional>> ConstraintSystem::selectDisjunction() { SmallVector disjunctions; collectDisjunctions(disjunctions); if (disjunctions.empty()) - return std::nullopt; + return llvm::None; if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return std::make_pair(disjunction, llvm::TinyPtrVector()); @@ -682,5 +683,5 @@ ConstraintSystem::selectDisjunction() { if (bestDisjunction != disjunctions.end()) return std::make_pair(*bestDisjunction, favorings[*bestDisjunction]); - return std::nullopt; + return llvm::None; } From fadf2856650f0dfa1619f915c8b1f590a28404ac Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:29 -0800 Subject: [PATCH 52/72] Revert "[CSOptimizer] Increase score when type matches opaque type" This reverts commit 2869dff995ca5301ce33f2116ca370749dca77f6. --- lib/Sema/CSOptimizer.cpp | 8 +----- test/Constraints/operator.swift | 46 --------------------------------- 2 files changed, 1 insertion(+), 53 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index bab3703d1027e..9b4d2ca033ffe 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -314,14 +314,8 @@ static void determineBestChoicesInContext( if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { return bool(cs.lookupConformance(candidateType, protocol)); - })) { - if (auto *GP = paramType->getAs()) { - auto *paramDecl = GP->getDecl(); - if (paramDecl && paramDecl->isOpaqueType()) - return 1.0; - } + })) return 0.7; - } } // Parameter is generic, let's check whether top-level diff --git a/test/Constraints/operator.swift b/test/Constraints/operator.swift index 77d07f5dd3078..4770481c95e30 100644 --- a/test/Constraints/operator.swift +++ b/test/Constraints/operator.swift @@ -329,49 +329,3 @@ enum I60954 { } init?(_ string: S) where S: StringProtocol {} // expected-note{{where 'S' = 'I60954'}} } - -infix operator <<<>>> : DefaultPrecedence - -protocol P5 { -} - -struct Expr : P6 {} - -protocol P6: P5 { -} - -extension P6 { - public static func <<<>>> (lhs: Self, rhs: (any P5)?) -> Expr { Expr() } - public static func <<<>>> (lhs: (any P5)?, rhs: Self) -> Expr { Expr() } - public static func <<<>>> (lhs: Self, rhs: some P6) -> Expr { Expr() } - - public static prefix func ! (value: Self) -> Expr { - Expr() - } -} - -extension P6 { - public static func != (lhs: Self, rhs: some P6) -> Expr { - !(lhs <<<>>> rhs) // Ok - } -} - -do { - struct Value : P6 { - } - - struct Column: P6 { - } - - func test(col: Column, val: Value) -> Expr { - col <<<>>> val // Ok - } - - func test(col: Column, val: some P6) -> Expr { - col <<<>>> val // Ok - } - - func test(col: some P6, val: Value) -> Expr { - col <<<>>> val // Ok - } -} From b380264f6f3bf3dcfd3d22e56514c39914d01c01 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:31 -0800 Subject: [PATCH 53/72] Revert "[CSOptimizer] NFC: Switch to llvm::Optional" This reverts commit 0fc68069221dfba4dc93f479ed7941665f7dd3fa. --- include/swift/Sema/ConstraintSystem.h | 2 +- lib/Sema/CSOptimizer.cpp | 13 +++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 63fe82222ae6f..ac2b72e916917 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5247,7 +5247,7 @@ class ConstraintSystem { /// Pick a disjunction from the InactiveConstraints list. /// /// \returns The selected disjunction and a set of it's favored choices. - llvm::Optional>> + Optional>> selectDisjunction(); /// Pick a conjunction from the InactiveConstraints list. diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 9b4d2ca033ffe..a2c452ce78a9b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -170,8 +170,9 @@ static void determineBestChoicesInContext( } // Match arguments to the given overload choice. - auto matchArguments = [&](OverloadChoice choice, FunctionType *overloadType) - -> llvm::Optional { + auto matchArguments = + [&](OverloadChoice choice, + FunctionType *overloadType) -> Optional { auto *decl = choice.getDeclOrNull(); assert(decl); @@ -186,7 +187,7 @@ static void determineBestChoicesInContext( return matchCallArguments(argsWithLabels, overloadType->getParams(), paramListInfo, argumentList->getFirstTrailingClosureIndex(), - /*allow fixes*/ false, listener, llvm::None); + /*allow fixes*/ false, listener, None); }; // Determine whether the candidate type is a subclass of the superclass @@ -633,13 +634,13 @@ selectBestBindingDisjunction(ConstraintSystem &cs, return firstBindDisjunction; } -llvm::Optional>> +Optional>> ConstraintSystem::selectDisjunction() { SmallVector disjunctions; collectDisjunctions(disjunctions); if (disjunctions.empty()) - return llvm::None; + return None; if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return std::make_pair(disjunction, llvm::TinyPtrVector()); @@ -677,5 +678,5 @@ ConstraintSystem::selectDisjunction() { if (bestDisjunction != disjunctions.end()) return std::make_pair(*bestDisjunction, favorings[*bestDisjunction]); - return llvm::None; + return None; } From 06451303b4462bbd2939bfc524ff2dfbda7603bd Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:32 -0800 Subject: [PATCH 54/72] Revert "[CSOptimizer] NFC: Adjust conformance check to use `ConstraintSystem::lookupConformance`" This reverts commit da65333d41d35601c9d3dc29d3441b527a516cea. --- lib/Sema/CSOptimizer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index a2c452ce78a9b..bc2dd200d7618 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -314,7 +314,9 @@ static void determineBestChoicesInContext( return 0.01; if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { - return bool(cs.lookupConformance(candidateType, protocol)); + return TypeChecker::conformsToProtocol(candidateType, protocol, + cs.DC->getParentModule(), + /*allowMissing=*/false); })) return 0.7; } From bab81347a5225a4c4bbb8969ebf0bf86054fe49c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:34 -0800 Subject: [PATCH 55/72] Revert "[CSOptimizer] Treat all type parameters equally" This reverts commit 957a5f4270ce9820f8be8d03a0c4e2f01db37d0e. --- lib/Sema/CSOptimizer.cpp | 41 ++++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index bc2dd200d7618..5e53d97b93c24 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -252,6 +252,12 @@ static void determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> double { + // Dependent members cannot be handled here because + // they require substitution of the base type which + // could come from a different argument. + if (paramType->getAs()) + return 0; + // Exact match between candidate and parameter types. if (candidateType->isEqual(paramType)) return options.contains(MatchFlag::Literal) ? 0.3 : 1; @@ -303,22 +309,25 @@ static void determineBestChoicesInContext( // Check protocol requirement(s) if this parameter is a // generic parameter type. - if (genericSig && paramType->isTypeParameter()) { - auto protocolRequirements = genericSig->getRequiredProtocols(paramType); - // It's a generic parameter or dependent member which might - // be connected via ame-type constraints to other generic - // parameters or dependent member but we cannot check that here, - // so let's add a tiny score just to acknowledge that it could - // possibly match. - if (protocolRequirements.empty()) - return 0.01; - - if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { - return TypeChecker::conformsToProtocol(candidateType, protocol, - cs.DC->getParentModule(), - /*allowMissing=*/false); - })) - return 0.7; + GenericSignature::RequiredProtocols protocolRequirements; + if (genericSig) { + if (auto *GP = paramType->getAs()) { + protocolRequirements = genericSig->getRequiredProtocols(GP); + // It's a generic parameter which might be connected via + // same-type constraints to other generic parameters but + // we cannot check that here, so let's add a tiny score + // just to acknowledge that it could possibly match. + if (protocolRequirements.empty()) { + return 0.01; + } + + if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { + return TypeChecker::conformsToProtocol(candidateType, protocol, + cs.DC->getParentModule(), + /*allowMissing=*/false); + })) + return 0.7; + } } // Parameter is generic, let's check whether top-level From e91817f3d3b19aac56e71b5a0d7c8989890535af Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:36 -0800 Subject: [PATCH 56/72] Revert "[CSStep] Remove disjunction pruning logic from DisjunctionStep" This reverts commit 2c44e379488ef1476d4aa5c0f86f58ec040e6de9. --- lib/Sema/CSStep.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index a3e75b93f0136..e446b07b5dd29 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -671,6 +671,8 @@ class TypeVariableStep final : public BindingStep { class DisjunctionStep final : public BindingStep { Constraint *Disjunction; + SmallVector DisabledChoices; + std::optional BestNonGenericScore; std::optional> LastSolvedChoice; @@ -687,12 +689,16 @@ class DisjunctionStep final : public BindingStep { : BindingStep(cs, {cs, disjunction, favoredChoices}, solutions), Disjunction(disjunction) { assert(Disjunction->getKind() == ConstraintKind::Disjunction); + pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; } ~DisjunctionStep() override { // Rewind back any changes left after attempting last choice. ActiveChoice.reset(); + // Re-enable previously disabled overload choices. + for (auto *choice : DisabledChoices) + choice->setEnabled(); } StepResult resume(bool prevFailed) override; @@ -745,6 +751,46 @@ class DisjunctionStep final : public BindingStep { /// simplified further, false otherwise. bool attempt(const DisjunctionChoice &choice) override; + // Check if selected disjunction has a representative + // this might happen when there are multiple binary operators + // chained together. If so, disable choices which differ + // from currently selected representative. + void pruneOverloadSet(Constraint *disjunction) { + auto *choice = disjunction->getNestedConstraints().front(); + if (choice->getKind() != ConstraintKind::BindOverload) + return; + + auto *typeVar = choice->getFirstType()->getAs(); + if (!typeVar) + return; + + auto *repr = typeVar->getImpl().getRepresentative(nullptr); + if (!repr || repr == typeVar) + return; + + for (auto overload : CS.getResolvedOverloads()) { + auto resolved = overload.second; + if (!resolved.boundType->isEqual(repr)) + continue; + + auto &representative = resolved.choice; + if (!representative.isDecl()) + return; + + // Disable all of the overload choices which are different from + // the one which is currently picked for representative. + for (auto *constraint : disjunction->getNestedConstraints()) { + auto choice = constraint->getOverloadChoice(); + if (!choice.isDecl() || choice.getDecl() == representative.getDecl()) + continue; + + constraint->setDisabled(); + DisabledChoices.push_back(constraint); + } + break; + } + }; + // Figure out which of the solutions has the smallest score. static std::optional getBestScore(SmallVectorImpl &solutions) { From 9d812c29918206c37af74fa0f44e9ada871ab21e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:37 -0800 Subject: [PATCH 57/72] Revert "[CSOptimizer] Relax candidate type requirements from equality to set of no-impact conversions" This reverts commit 11b897b32f92ab735915910782e2c9bab00fe3f2. --- lib/Sema/CSOptimizer.cpp | 221 +++++------------------- test/Constraints/diag_ambiguities.swift | 6 +- test/expr/expressions.swift | 8 +- 3 files changed, 55 insertions(+), 180 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 5e53d97b93c24..0f9a78b1e3ef9 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -15,16 +15,13 @@ //===----------------------------------------------------------------------===// #include "TypeChecker.h" -#include "swift/AST/ExistentialLayout.h" #include "swift/AST/GenericSignature.h" -#include "swift/Basic/OptionSet.h" #include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" #include "llvm/ADT/BitVector.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/TinyPtrVector.h" -#include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -190,162 +187,6 @@ static void determineBestChoicesInContext( /*allow fixes*/ false, listener, None); }; - // Determine whether the candidate type is a subclass of the superclass - // type. - std::function isSubclassOf = [&](Type candidateType, - Type superclassType) { - // Conversion from a concrete type to its existential value. - if (superclassType->isExistentialType() && !superclassType->isAny()) { - auto layout = superclassType->getExistentialLayout(); - - if (auto layoutConstraint = layout.getLayoutConstraint()) { - if (layoutConstraint->isClass() && - !(candidateType->isClassExistentialType() || - candidateType->mayHaveSuperclass())) - return false; - } - - if (layout.explicitSuperclass && - !isSubclassOf(candidateType, layout.explicitSuperclass)) - return false; - - return llvm::all_of(layout.getProtocols(), [&](ProtocolDecl *P) { - if (auto superclass = P->getSuperclass()) { - if (!isSubclassOf(candidateType, superclass)) - return false; - } - - return bool(TypeChecker::containsProtocol( - candidateType, P, cs.DC->getParentModule(), - /*skipConditionalRequirements=*/true, - /*allowMissing=*/false)); - }); - } - - auto *subclassDecl = candidateType->getClassOrBoundGenericClass(); - auto *superclassDecl = superclassType->getClassOrBoundGenericClass(); - - if (!(subclassDecl && superclassDecl)) - return false; - - return superclassDecl->isSuperclassOf(subclassDecl); - }; - - enum class MatchFlag { - OnParam = 0x01, - Literal = 0x02, - }; - - using MatchOptions = OptionSet; - - // Perform a limited set of checks to determine whether the candidate - // could possibly match the parameter type: - // - // - Equality - // - Protocol conformance(s) - // - Optional injection - // - Superclass conversion - // - Array-to-pointer conversion - // - Value to existential conversion - // - Exact match on top-level types - std::function - scoreCandidateMatch = [&](GenericSignature genericSig, - Type candidateType, Type paramType, - MatchOptions options) -> double { - // Dependent members cannot be handled here because - // they require substitution of the base type which - // could come from a different argument. - if (paramType->getAs()) - return 0; - - // Exact match between candidate and parameter types. - if (candidateType->isEqual(paramType)) - return options.contains(MatchFlag::Literal) ? 0.3 : 1; - - if (options.contains(MatchFlag::Literal)) - return 0; - - // Check whether match would require optional injection. - { - SmallVector candidateOptionals; - SmallVector paramOptionals; - - candidateType = - candidateType->lookThroughAllOptionalTypes(candidateOptionals); - paramType = paramType->lookThroughAllOptionalTypes(paramOptionals); - - if (!candidateOptionals.empty() || !paramOptionals.empty()) { - if (paramOptionals.size() >= candidateOptionals.size()) - return scoreCandidateMatch(genericSig, candidateType, paramType, - options); - - // Optionality mismatch. - return 0; - } - } - - // Candidate could be injected into optional parameter type - // or converted to a superclass. - if (isSubclassOf(candidateType, paramType)) - return 1; - - // Possible Array -> Unsafe*Pointer conversion. - if (options.contains(MatchFlag::OnParam)) { - if (candidateType->isArrayType() && - paramType->getAnyPointerElementType()) - return 1; - } - - // If both argument and parameter are tuples of the same arity, - // it's a match. - { - if (auto *candidateTuple = candidateType->getAs()) { - auto *paramTuple = paramType->getAs(); - if (paramTuple && - candidateTuple->getNumElements() == paramTuple->getNumElements()) - return 1; - } - } - - // Check protocol requirement(s) if this parameter is a - // generic parameter type. - GenericSignature::RequiredProtocols protocolRequirements; - if (genericSig) { - if (auto *GP = paramType->getAs()) { - protocolRequirements = genericSig->getRequiredProtocols(GP); - // It's a generic parameter which might be connected via - // same-type constraints to other generic parameters but - // we cannot check that here, so let's add a tiny score - // just to acknowledge that it could possibly match. - if (protocolRequirements.empty()) { - return 0.01; - } - - if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { - return TypeChecker::conformsToProtocol(candidateType, protocol, - cs.DC->getParentModule(), - /*allowMissing=*/false); - })) - return 0.7; - } - } - - // Parameter is generic, let's check whether top-level - // types match i.e. Array as a parameter. - // - // This is slightly better than all of the conformances matching - // because the parameter is concrete and could split the graph. - if (paramType->hasTypeParameter()) { - auto *candidateDecl = candidateType->getAnyNominal(); - auto *paramDecl = paramType->getAnyNominal(); - - if (candidateDecl && paramDecl && candidateDecl == paramDecl) - return 0.8; - } - - return 0; - }; - // The choice with the best score. double bestScore = 0.0; SmallVector, 2> favoredChoices; @@ -415,6 +256,23 @@ static void determineBestChoicesInContext( if (paramType->is()) continue; + // Check protocol requirement(s) if this parameter is a + // generic parameter type. + GenericSignature::RequiredProtocols protocolRequirements; + if (genericSig) { + if (auto *GP = paramType->getAs()) { + protocolRequirements = genericSig->getRequiredProtocols(GP); + // It's a generic parameter which might be connected via + // same-type constraints to other generic parameters but + // we cannot check that here, so let's ignore it. + if (protocolRequirements.empty()) + continue; + } + + if (paramType->getAs()) + return; + } + // The idea here is to match the parameter type against // all of the argument candidate types and pick the best // match (i.e. exact equality one). @@ -448,14 +306,32 @@ static void determineBestChoicesInContext( // The specifier only matters for `inout` check. candidateType = candidateType->getWithoutSpecifierType(); - MatchOptions options(MatchFlag::OnParam); - if (isLiteralDefault) - options |= MatchFlag::Literal; - - auto score = scoreCandidateMatch(genericSig, candidateType, - paramType, options); - if (score > 0) { - bestCandidateScore = std::max(bestCandidateScore, score); + // We don't check generic requirements against literal default + // types because it creates more noise than signal for operators. + if (!protocolRequirements.empty() && !isLiteralDefault) { + if (llvm::all_of( + protocolRequirements, [&](ProtocolDecl *protocol) { + return TypeChecker::conformsToProtocol( + candidateType, protocol, cs.DC->getParentModule(), + /*allowMissing=*/false); + })) { + // Score is lower here because we still prefer concrete + // overloads over the generic ones when possible. + bestCandidateScore = std::max(bestCandidateScore, 0.7); + continue; + } + } else if (paramType->hasTypeParameter()) { + // i.e. Array or Optional as a parameter. + // This is slightly better than all of the conformances matching + // because the parameter is concrete and could split the graph. + if (paramType->getAnyNominal() == candidateType->getAnyNominal()) { + bestCandidateScore = std::max(bestCandidateScore, 0.8); + continue; + } + } else if (candidateType->isEqual(paramType)) { + // Exact match on one of the candidate bindings. + bestCandidateScore = + std::max(bestCandidateScore, isLiteralDefault ? 0.3 : 1.0); continue; } @@ -489,12 +365,11 @@ static void determineBestChoicesInContext( if (score > 0 || (decl->isOperator() && !decl->getBaseIdentifier().isStandardComparisonOperator())) { - if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) { - return scoreCandidateMatch(genericSig, - overloadType->getResult(), - candidateResultTy, - /*options=*/{}) > 0; - })) { + if (llvm::any_of( + resultTypes, [&overloadType](const Type candidateResultTy) { + auto overloadResultTy = overloadType->getResult(); + return candidateResultTy->isEqual(overloadResultTy); + })) { score += 1.0; } } diff --git a/test/Constraints/diag_ambiguities.swift b/test/Constraints/diag_ambiguities.swift index cb328fb0d5ed4..d6b358609c771 100644 --- a/test/Constraints/diag_ambiguities.swift +++ b/test/Constraints/diag_ambiguities.swift @@ -29,11 +29,11 @@ C(g) // expected-error{{ambiguous use of 'g'}} func h(_ x: T) -> () {} _ = C(h) // OK - init(_: (Int) -> ()) -func rdar29691909_callee(_ o: AnyObject?) -> Any? { return o } // expected-note {{found this candidate}} -func rdar29691909_callee(_ o: AnyObject) -> Any { return o } // expected-note {{found this candidate}} +func rdar29691909_callee(_ o: AnyObject?) -> Any? { return o } +func rdar29691909_callee(_ o: AnyObject) -> Any { return o } func rdar29691909(o: AnyObject) -> Any? { - return rdar29691909_callee(o) // expected-error{{ambiguous use of 'rdar29691909_callee'}} + return rdar29691909_callee(o) } func rdar29907555(_ value: Any!) -> String { diff --git a/test/expr/expressions.swift b/test/expr/expressions.swift index 4a4d8c3232ce8..d7ddc5fc0c765 100644 --- a/test/expr/expressions.swift +++ b/test/expr/expressions.swift @@ -758,10 +758,10 @@ func invalidDictionaryLiteral() { //===----------------------------------------------------------------------===// // nil/metatype comparisons //===----------------------------------------------------------------------===// -_ = Int.self == nil // expected-warning {{comparing non-optional value of type 'Int.Type' to 'nil' always returns false}} -_ = nil == Int.self // expected-warning {{comparing non-optional value of type 'Int.Type' to 'nil' always returns false}} -_ = Int.self != nil // expected-warning {{comparing non-optional value of type 'Int.Type' to 'nil' always returns true}} -_ = nil != Int.self // expected-warning {{comparing non-optional value of type 'Int.Type' to 'nil' always returns true}} +_ = Int.self == nil // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns false}} +_ = nil == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns false}} +_ = Int.self != nil // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns true}} +_ = nil != Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns true}} _ = Int.self == .none // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns false}} _ = .none == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns false}} From 83946b80a478a02394a632d9f0b815db5ec5dd33 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:40 -0800 Subject: [PATCH 58/72] Revert "[CSOptimizer] Use `matchCallArguments` to establish argument-to-parameter relationships" This reverts commit cb1cb2018dc7662cd8af08633c17b876b187b377. --- lib/Sema/CSOptimizer.cpp | 77 ++++++++++++---------------------------- 1 file changed, 22 insertions(+), 55 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 0f9a78b1e3ef9..5ab7c377e31a3 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -116,13 +116,6 @@ static void determineBestChoicesInContext( if (!argumentList || cs.containsIDEInspectionTarget(argumentList)) return; - SmallVector argsWithLabels; - { - argsWithLabels.append(argFuncType->getParams().begin(), - argFuncType->getParams().end()); - FunctionType::relabelParams(argsWithLabels, argumentList); - } - SmallVector, 2>, 2> candidateArgumentTypes; candidateArgumentTypes.resize(argFuncType->getNumParams()); @@ -166,27 +159,6 @@ static void determineBestChoicesInContext( resultTypes.push_back(resultType); } - // Match arguments to the given overload choice. - auto matchArguments = - [&](OverloadChoice choice, - FunctionType *overloadType) -> Optional { - auto *decl = choice.getDeclOrNull(); - assert(decl); - - auto hasAppliedSelf = - decl->hasCurriedSelf() && - doesMemberRefApplyCurriedSelf(choice.getBaseType(), decl); - - ParameterListInfo paramListInfo(overloadType->getParams(), decl, - hasAppliedSelf); - - MatchCallArgumentListener listener; - return matchCallArguments(argsWithLabels, overloadType->getParams(), - paramListInfo, - argumentList->getFirstTrailingClosureIndex(), - /*allow fixes*/ false, listener, None); - }; - // The choice with the best score. double bestScore = 0.0; SmallVector, 2> favoredChoices; @@ -210,35 +182,27 @@ static void determineBestChoicesInContext( return; } - auto matchings = - matchArguments(choice->getOverloadChoice(), overloadType); - if (!matchings) - return; + ParameterListInfo paramListInfo( + overloadType->getParams(), decl, + hasAppliedSelf(cs, choice->getOverloadChoice())); double score = 0.0; - for (unsigned paramIdx = 0, n = overloadType->getNumParams(); - paramIdx != n; ++paramIdx) { - const auto ¶m = overloadType->getParams()[paramIdx]; - - auto argIndices = matchings->parameterBindings[paramIdx]; - switch (argIndices.size()) { - case 0: - // Current parameter is defaulted. - continue; - - case 1: - // One-to-one match between argument and parameter. - break; + for (unsigned i = 0, n = overloadType->getNumParams(); i != n; ++i) { + const auto ¶m = overloadType->getParams()[i]; + + if (i >= candidateArgumentTypes.size()) { + // If parameter has a default - continue matching, + // all of the subsequence parameters should have defaults + // as well, if they don't the overload choice in not viable. + if (paramListInfo.hasDefaultArgument(i)) + continue; - default: - // Cannot deal with multiple possible matchings at the moment. + // Early return because this overload choice is not viable + // without default value for the current parameter. return; } - auto argIdx = argIndices.front(); - - // Looks like there is nothing know about the argument. - if (candidateArgumentTypes[argIdx].empty()) + if (candidateArgumentTypes[i].empty()) continue; const auto paramFlags = param.getParameterFlags(); @@ -256,6 +220,9 @@ static void determineBestChoicesInContext( if (paramType->is()) continue; + if (candidateArgumentTypes[i].empty()) + continue; + // Check protocol requirement(s) if this parameter is a // generic parameter type. GenericSignature::RequiredProtocols protocolRequirements; @@ -281,11 +248,11 @@ static void determineBestChoicesInContext( // all bound concrete types, we consider this is mismatch // at this parameter position and remove the overload choice // from consideration. + double bestCandidateScore = 0; - llvm::BitVector mismatches(candidateArgumentTypes[argIdx].size()); + llvm::BitVector mismatches(candidateArgumentTypes[i].size()); - for (unsigned candidateIdx : - indices(candidateArgumentTypes[argIdx])) { + for (unsigned candidateIdx : indices(candidateArgumentTypes[i])) { // If one of the candidates matched exactly there is no reason // to continue checking. if (bestCandidateScore == 1) @@ -295,7 +262,7 @@ static void determineBestChoicesInContext( bool isLiteralDefault; std::tie(candidateType, isLiteralDefault) = - candidateArgumentTypes[argIdx][candidateIdx]; + candidateArgumentTypes[i][candidateIdx]; // `inout` parameter accepts only l-value argument. if (paramFlags.isInOut() && !candidateType->is()) { From a170c76449076bd9a409efc94eb9aa7607a074e6 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:41 -0800 Subject: [PATCH 59/72] Revert "[CSOptimizer] Don't attempt to optimize calls with code completion token(s) in argument position" This reverts commit 14e2a16fced65b6e4b2130af91957556b0efbfb8. --- lib/Sema/CSOptimizer.cpp | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 5ab7c377e31a3..f74ab82aff1ce 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -112,10 +112,6 @@ static void determineBestChoicesInContext( auto argFuncType = applicableFn.get()->getFirstType()->getAs(); - auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); - if (!argumentList || cs.containsIDEInspectionTarget(argumentList)) - return; - SmallVector, 2>, 2> candidateArgumentTypes; candidateArgumentTypes.resize(argFuncType->getNumParams()); From e8970d4ab0ef6e745c21ac5af2c031044aa9130d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:43 -0800 Subject: [PATCH 60/72] Revert "[CSOptimizer] Allow generic operator overloads without associated type parameters" This reverts commit bc5f70a9a3b2b0f3664e8a1fcb20acc693217cc0. --- lib/Sema/CSOptimizer.cpp | 117 +++++++++--------- .../{fast => slow}/rdar17170728.swift | 1 + 2 files changed, 59 insertions(+), 59 deletions(-) rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar17170728.swift (74%) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index f74ab82aff1ce..c3187b1998fb6 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -14,8 +14,6 @@ // //===----------------------------------------------------------------------===// -#include "TypeChecker.h" -#include "swift/AST/GenericSignature.h" #include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" #include "llvm/ADT/BitVector.h" @@ -88,6 +86,34 @@ void forEachDisjunctionChoice( } } +static bool isSIMDType(Type type) { + auto *NTD = dyn_cast_or_null(type->getAnyNominal()); + if (!NTD) + return false; + + auto typeName = NTD->getName().str(); + if (!typeName.startswith("SIMD")) + return false; + + return NTD->getParentModule()->getName().is("Swift"); +} + +static bool isArithmeticOperatorOnSIMDProtocol(ValueDecl *decl) { + if (!isSIMDOperator(decl)) + return false; + + if (!decl->getBaseIdentifier().isArithmeticOperator()) + return false; + + auto *DC = decl->getDeclContext(); + if (auto *P = DC->getSelfProtocolDecl()) { + if (auto knownKind = P->getKnownProtocolKind()) + return *knownKind == KnownProtocolKind::SIMD; + } + + return false; +} + } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -155,6 +181,23 @@ static void determineBestChoicesInContext( resultTypes.push_back(resultType); } + auto isViableOverload = [&](ValueDecl *decl) { + // Allow standard arithmetic operator overloads on SIMD protocol + // to be considered because we can favor them when then argument + // is a known SIMD type. + if (isArithmeticOperatorOnSIMDProtocol(decl)) + return true; + + // Don't consider generic overloads because we need conformance + // checking functionality to determine best favoring, preferring + // such overloads based only on concrete types leads to subpar + // choices due to missed information. + if (decl->getInterfaceType()->is()) + return false; + + return true; + }; + // The choice with the best score. double bestScore = 0.0; SmallVector, 2> favoredChoices; @@ -162,21 +205,8 @@ static void determineBestChoicesInContext( forEachDisjunctionChoice( cs, disjunction, [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { - GenericSignature genericSig; - { - if (auto *GF = dyn_cast(decl)) { - genericSig = GF->getGenericSignature(); - } else if (auto *SD = dyn_cast(decl)) { - genericSig = SD->getGenericSignature(); - } - - // Let's not consider non-operator generic overloads because we - // need conformance checking functionality to determine best - // favoring, preferring such overloads based on concrete types - // alone leads to subpar choices due to missed information. - if (genericSig && !decl->isOperator()) - return; - } + if (!isViableOverload(decl)) + return; ParameterListInfo paramListInfo( overloadType->getParams(), decl, @@ -219,23 +249,6 @@ static void determineBestChoicesInContext( if (candidateArgumentTypes[i].empty()) continue; - // Check protocol requirement(s) if this parameter is a - // generic parameter type. - GenericSignature::RequiredProtocols protocolRequirements; - if (genericSig) { - if (auto *GP = paramType->getAs()) { - protocolRequirements = genericSig->getRequiredProtocols(GP); - // It's a generic parameter which might be connected via - // same-type constraints to other generic parameters but - // we cannot check that here, so let's ignore it. - if (protocolRequirements.empty()) - continue; - } - - if (paramType->getAs()) - return; - } - // The idea here is to match the parameter type against // all of the argument candidate types and pick the best // match (i.e. exact equality one). @@ -268,36 +281,22 @@ static void determineBestChoicesInContext( // The specifier only matters for `inout` check. candidateType = candidateType->getWithoutSpecifierType(); - - // We don't check generic requirements against literal default - // types because it creates more noise than signal for operators. - if (!protocolRequirements.empty() && !isLiteralDefault) { - if (llvm::all_of( - protocolRequirements, [&](ProtocolDecl *protocol) { - return TypeChecker::conformsToProtocol( - candidateType, protocol, cs.DC->getParentModule(), - /*allowMissing=*/false); - })) { - // Score is lower here because we still prefer concrete - // overloads over the generic ones when possible. - bestCandidateScore = std::max(bestCandidateScore, 0.7); - continue; - } - } else if (paramType->hasTypeParameter()) { - // i.e. Array or Optional as a parameter. - // This is slightly better than all of the conformances matching - // because the parameter is concrete and could split the graph. - if (paramType->getAnyNominal() == candidateType->getAnyNominal()) { - bestCandidateScore = std::max(bestCandidateScore, 0.8); - continue; - } - } else if (candidateType->isEqual(paramType)) { - // Exact match on one of the candidate bindings. + // Exact match on one of the candidate bindings. + if (candidateType->isEqual(paramType)) { bestCandidateScore = std::max(bestCandidateScore, isLiteralDefault ? 0.3 : 1.0); continue; } + // If argument is SIMD type i.e. SIMD1<...> it's appropriate + // to favor of the overloads that are declared on SIMD protocol + // and expect a particular `Scalar` if it's known. + if (isSIMDType(candidateType) && + isArithmeticOperatorOnSIMDProtocol(decl)) { + bestCandidateScore = 1.0; + continue; + } + // Only established arguments could be considered mismatches, // literal default types should be regarded as holes if they // didn't match. diff --git a/validation-test/Sema/type_checker_perf/fast/rdar17170728.swift b/validation-test/Sema/type_checker_perf/slow/rdar17170728.swift similarity index 74% rename from validation-test/Sema/type_checker_perf/fast/rdar17170728.swift rename to validation-test/Sema/type_checker_perf/slow/rdar17170728.swift index 1e64e4194e4b6..62c2bb2ea367b 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar17170728.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar17170728.swift @@ -5,6 +5,7 @@ let i: Int? = 1 let j: Int? let k: Int? = 2 +// expected-error@+1 {{the compiler is unable to type-check this expression in reasonable time}} let _ = [i, j, k].reduce(0 as Int?) { $0 != nil && $1 != nil ? $0! + $1! : ($0 != nil ? $0! : ($1 != nil ? $1! : nil)) } From bb9081e8f41a2964af02b1988fff9a9d308af557 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:45 -0800 Subject: [PATCH 61/72] Revert "[CSOptimizer] Make sure that all parameters without arguments are defaulted" This reverts commit 7c1c46d4e4480d78c1dc3ef5447fd616411a5dfe. --- lib/Sema/CSOptimizer.cpp | 20 +++----------------- test/Constraints/ranking.swift | 8 -------- 2 files changed, 3 insertions(+), 25 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c3187b1998fb6..cd4b44eb15a8b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -208,29 +208,15 @@ static void determineBestChoicesInContext( if (!isViableOverload(decl)) return; - ParameterListInfo paramListInfo( - overloadType->getParams(), decl, - hasAppliedSelf(cs, choice->getOverloadChoice())); + if (overloadType->getNumParams() != argFuncType->getNumParams()) + return; double score = 0.0; for (unsigned i = 0, n = overloadType->getNumParams(); i != n; ++i) { - const auto ¶m = overloadType->getParams()[i]; - - if (i >= candidateArgumentTypes.size()) { - // If parameter has a default - continue matching, - // all of the subsequence parameters should have defaults - // as well, if they don't the overload choice in not viable. - if (paramListInfo.hasDefaultArgument(i)) - continue; - - // Early return because this overload choice is not viable - // without default value for the current parameter. - return; - } - if (candidateArgumentTypes[i].empty()) continue; + const auto ¶m = overloadType->getParams()[i]; const auto paramFlags = param.getParameterFlags(); // If parameter is variadic we cannot compare because we don't know diff --git a/test/Constraints/ranking.swift b/test/Constraints/ranking.swift index 74e3b6842f02e..82d29791e6f14 100644 --- a/test/Constraints/ranking.swift +++ b/test/Constraints/ranking.swift @@ -450,11 +450,3 @@ struct HasIntInit { func compare_solutions_with_bindings(x: UInt8, y: UInt8) -> HasIntInit { return .init(Int(x / numericCast(y))) } - -// Test to make sure that previous favoring behavior is maintained and @autoclosure makes a difference. -func test_no_ambiguity_with_autoclosure(x: Int) { - func test(_ condition: Bool, file: StaticString = #file, line: UInt = #line) {} - func test(_ condition: @autoclosure () -> Bool, file: StaticString = #file, line: UInt = #line) {} - - test(x >= 0) // Ok -} From f71037dec7845fd9c548219f2ac3dad3d5fd39a7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:46 -0800 Subject: [PATCH 62/72] Revert "[CSStep] Don't favor choices until the disjunction is picked" This reverts commit e404ed722a99f880e3dbcf163283ba31da06c4e5. --- include/swift/Sema/ConstraintSystem.h | 18 +++++------------- lib/Sema/CSOptimizer.cpp | 19 ++++++++++++------- lib/Sema/CSStep.cpp | 9 ++++----- lib/Sema/CSStep.h | 10 +--------- 4 files changed, 22 insertions(+), 34 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index ac2b72e916917..e4ddf2a62e836 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5246,9 +5246,8 @@ class ConstraintSystem { /// Pick a disjunction from the InactiveConstraints list. /// - /// \returns The selected disjunction and a set of it's favored choices. - Optional>> - selectDisjunction(); + /// \returns The selected disjunction. + Constraint *selectDisjunction(); /// Pick a conjunction from the InactiveConstraints list. /// @@ -6177,8 +6176,7 @@ class DisjunctionChoiceProducer : public BindingProducer { public: using Element = DisjunctionChoice; - DisjunctionChoiceProducer(ConstraintSystem &cs, Constraint *disjunction, - llvm::TinyPtrVector &favorites) + DisjunctionChoiceProducer(ConstraintSystem &cs, Constraint *disjunction) : BindingProducer(cs, disjunction->shouldRememberChoice() ? disjunction->getLocator() : nullptr), @@ -6188,11 +6186,6 @@ class DisjunctionChoiceProducer : public BindingProducer { assert(disjunction->getKind() == ConstraintKind::Disjunction); assert(!disjunction->shouldRememberChoice() || disjunction->getLocator()); - // Mark constraints as favored. This information - // is going to be used by partitioner. - for (auto *choice : favorites) - cs.favorConstraint(choice); - // Order and partition the disjunction choices. partitionDisjunction(Ordering, PartitionBeginning); } @@ -6237,9 +6230,8 @@ class DisjunctionChoiceProducer : public BindingProducer { // Partition the choices in the disjunction into groups that we will // iterate over in an order appropriate to attempt to stop before we // have to visit all of the options. - void - partitionDisjunction(SmallVectorImpl &Ordering, - SmallVectorImpl &PartitionBeginning); + 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 diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index cd4b44eb15a8b..46bda3bffba7c 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -468,16 +468,15 @@ selectBestBindingDisjunction(ConstraintSystem &cs, return firstBindDisjunction; } -Optional>> -ConstraintSystem::selectDisjunction() { +Constraint *ConstraintSystem::selectDisjunction() { SmallVector disjunctions; collectDisjunctions(disjunctions); if (disjunctions.empty()) - return None; + return nullptr; if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) - return std::make_pair(disjunction, llvm::TinyPtrVector()); + return disjunction; llvm::DenseMap> favorings; determineBestChoicesInContext(*this, disjunctions, favorings); @@ -509,8 +508,14 @@ ConstraintSystem::selectDisjunction() { return firstFavored < secondFavored; }); - if (bestDisjunction != disjunctions.end()) - return std::make_pair(*bestDisjunction, favorings[*bestDisjunction]); + if (bestDisjunction != disjunctions.end()) { + // If selected disjunction has any choices that should be favored + // let's record them now. + for (auto *choice : favorings[*bestDisjunction]) + favorConstraint(choice); + + return *bestDisjunction; + } - return None; + return nullptr; } diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index 77632d2a8549d..14ce02e330d51 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -366,7 +366,7 @@ StepResult ComponentStep::take(bool prevFailed) { } }); - auto disjunction = CS.selectDisjunction(); + auto *disjunction = CS.selectDisjunction(); auto *conjunction = CS.selectConjunction(); if (CS.isDebugMode()) { @@ -409,8 +409,7 @@ StepResult ComponentStep::take(bool prevFailed) { // Bindings usually happen first, but sometimes we want to prioritize a // disjunction or conjunction. if (bestBindings) { - if (disjunction && - !bestBindings->favoredOverDisjunction(disjunction->first)) + if (disjunction && !bestBindings->favoredOverDisjunction(disjunction)) return StepKind::Disjunction; if (conjunction && !bestBindings->favoredOverConjunction(conjunction)) @@ -433,9 +432,9 @@ StepResult ComponentStep::take(bool prevFailed) { return suspend( std::make_unique(*bestBindings, Solutions)); case StepKind::Disjunction: { - CS.retireConstraint(disjunction->first); + CS.retireConstraint(disjunction); return suspend( - std::make_unique(CS, *disjunction, Solutions)); + std::make_unique(CS, disjunction, Solutions)); } case StepKind::Conjunction: { CS.retireConstraint(conjunction); diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index e446b07b5dd29..3b8c39e4a015d 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -677,17 +677,9 @@ class DisjunctionStep final : public BindingStep { std::optional> LastSolvedChoice; public: - DisjunctionStep( - ConstraintSystem &cs, - std::pair> &disjunction, - SmallVectorImpl &solutions) - : DisjunctionStep(cs, disjunction.first, disjunction.second, solutions) {} - DisjunctionStep(ConstraintSystem &cs, Constraint *disjunction, - llvm::TinyPtrVector &favoredChoices, SmallVectorImpl &solutions) - : BindingStep(cs, {cs, disjunction, favoredChoices}, solutions), - Disjunction(disjunction) { + : BindingStep(cs, {cs, disjunction}, solutions), Disjunction(disjunction) { assert(Disjunction->getKind() == ConstraintKind::Disjunction); pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; From 71f282aa658c34b8af39b6d5c8de94afbe64ea9b Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:48 -0800 Subject: [PATCH 63/72] Revert "[CSOptimizer] Keep track of mismatches while evaluating candidates" This reverts commit a094c3ebb061ba25e501ade2e6fd010043666a39. --- lib/Sema/CSOptimizer.cpp | 53 ++++++---------------------------------- 1 file changed, 8 insertions(+), 45 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 46bda3bffba7c..2d9833ee60a15 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -16,7 +16,6 @@ #include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" -#include "llvm/ADT/BitVector.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/TinyPtrVector.h" @@ -232,45 +231,20 @@ static void determineBestChoicesInContext( if (paramType->is()) continue; - if (candidateArgumentTypes[i].empty()) - continue; - - // The idea here is to match the parameter type against - // all of the argument candidate types and pick the best - // match (i.e. exact equality one). - // - // If none of the candidates match exactly and they are - // all bound concrete types, we consider this is mismatch - // at this parameter position and remove the overload choice - // from consideration. - - double bestCandidateScore = 0; - llvm::BitVector mismatches(candidateArgumentTypes[i].size()); - - for (unsigned candidateIdx : indices(candidateArgumentTypes[i])) { - // If one of the candidates matched exactly there is no reason - // to continue checking. - if (bestCandidateScore == 1) - break; - - Type candidateType; - bool isLiteralDefault; - - std::tie(candidateType, isLiteralDefault) = - candidateArgumentTypes[i][candidateIdx]; + double argScore = 0.0; + for (auto const &candidate : candidateArgumentTypes[i]) { + auto candidateType = candidate.first; // `inout` parameter accepts only l-value argument. - if (paramFlags.isInOut() && !candidateType->is()) { - mismatches.set(candidateIdx); + if (paramFlags.isInOut() && !candidateType->is()) continue; - } // The specifier only matters for `inout` check. candidateType = candidateType->getWithoutSpecifierType(); // Exact match on one of the candidate bindings. if (candidateType->isEqual(paramType)) { - bestCandidateScore = - std::max(bestCandidateScore, isLiteralDefault ? 0.3 : 1.0); + argScore = std::max( + argScore, /*fromLiteral=*/candidate.second ? 0.3 : 1.0); continue; } @@ -279,23 +253,12 @@ static void determineBestChoicesInContext( // and expect a particular `Scalar` if it's known. if (isSIMDType(candidateType) && isArithmeticOperatorOnSIMDProtocol(decl)) { - bestCandidateScore = 1.0; + argScore = std::max(argScore, 1.0); continue; } - - // Only established arguments could be considered mismatches, - // literal default types should be regarded as holes if they - // didn't match. - if (!isLiteralDefault && !candidateType->hasTypeVariable()) - mismatches.set(candidateIdx); } - // If none of the candidates for this parameter matched, let's - // drop this overload from any further consideration. - if (mismatches.all()) - return; - - score += bestCandidateScore; + score += argScore; } // Average the score to avoid disfavoring disjunctions with fewer From ba5709934eda63c7cd9a70d7eb65a16788fb6d3d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:21:50 -0800 Subject: [PATCH 64/72] Revert "[CSOptimizer] Favor SIMD related arithmetic operator choices if argument is SIMD type" This reverts commit c2f7451c7b21dd74af34f5a0b5e081634b3e3cdb. --- lib/Sema/CSOptimizer.cpp | 61 ++++------------------------------------ 1 file changed, 5 insertions(+), 56 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 2d9833ee60a15..0f6a9932819bb 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -85,34 +85,6 @@ void forEachDisjunctionChoice( } } -static bool isSIMDType(Type type) { - auto *NTD = dyn_cast_or_null(type->getAnyNominal()); - if (!NTD) - return false; - - auto typeName = NTD->getName().str(); - if (!typeName.startswith("SIMD")) - return false; - - return NTD->getParentModule()->getName().is("Swift"); -} - -static bool isArithmeticOperatorOnSIMDProtocol(ValueDecl *decl) { - if (!isSIMDOperator(decl)) - return false; - - if (!decl->getBaseIdentifier().isArithmeticOperator()) - return false; - - auto *DC = decl->getDeclContext(); - if (auto *P = DC->getSelfProtocolDecl()) { - if (auto knownKind = P->getKnownProtocolKind()) - return *knownKind == KnownProtocolKind::SIMD; - } - - return false; -} - } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -180,23 +152,6 @@ static void determineBestChoicesInContext( resultTypes.push_back(resultType); } - auto isViableOverload = [&](ValueDecl *decl) { - // Allow standard arithmetic operator overloads on SIMD protocol - // to be considered because we can favor them when then argument - // is a known SIMD type. - if (isArithmeticOperatorOnSIMDProtocol(decl)) - return true; - - // Don't consider generic overloads because we need conformance - // checking functionality to determine best favoring, preferring - // such overloads based only on concrete types leads to subpar - // choices due to missed information. - if (decl->getInterfaceType()->is()) - return false; - - return true; - }; - // The choice with the best score. double bestScore = 0.0; SmallVector, 2> favoredChoices; @@ -204,7 +159,11 @@ static void determineBestChoicesInContext( forEachDisjunctionChoice( cs, disjunction, [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { - if (!isViableOverload(decl)) + // Don't consider generic overloads because we need conformance + // checking functionality to determine best favoring, preferring + // such overloads based only on concrete types leads to subpar + // choices due to missed information. + if (decl->getInterfaceType()->is()) return; if (overloadType->getNumParams() != argFuncType->getNumParams()) @@ -245,16 +204,6 @@ static void determineBestChoicesInContext( if (candidateType->isEqual(paramType)) { argScore = std::max( argScore, /*fromLiteral=*/candidate.second ? 0.3 : 1.0); - continue; - } - - // If argument is SIMD type i.e. SIMD1<...> it's appropriate - // to favor of the overloads that are declared on SIMD protocol - // and expect a particular `Scalar` if it's known. - if (isSIMDType(candidateType) && - isArithmeticOperatorOnSIMDProtocol(decl)) { - argScore = std::max(argScore, 1.0); - continue; } } From 16da0352581b488223753e18d52f8ef49c116bfd Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:24:13 -0800 Subject: [PATCH 65/72] Revert "[CSOptimizer] Initial implementation of disjunction choice favoring algorithm" This reverts commit 672ae3d252b6030c83a179fcc2e67704c0a758d8. --- lib/Sema/CSOptimizer.cpp | 412 +----------------- lib/Sema/CSSolver.cpp | 113 +++++ test/Constraints/common_type.swift | 2 - test/Constraints/diag_ambiguities.swift | 6 +- .../{fast => slow}/rdar22770433.swift | 1 + .../{fast => slow}/rdar23682605.swift | 1 + .../{fast => slow}/rdar31742586.swift | 1 + .../{fast => slow}/rdar35213699.swift | 2 + .../rdar46713933_literal_arg.swift | 1 + .../{fast => slow}/rdar91310777.swift | 1 + 10 files changed, 125 insertions(+), 415 deletions(-) rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar22770433.swift (74%) rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar23682605.swift (91%) rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar31742586.swift (67%) rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar35213699.swift (66%) rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar46713933_literal_arg.swift (85%) rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar91310777.swift (66%) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 0f6a9932819bb..0bac2925d1f53 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -14,420 +14,12 @@ // //===----------------------------------------------------------------------===// -#include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" -#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/TinyPtrVector.h" -#include "llvm/Support/raw_ostream.h" -#include -#include using namespace swift; using namespace constraints; -namespace { - -NullablePtr getApplicableFnConstraint(ConstraintGraph &CG, - Constraint *disjunction) { - auto *boundVar = disjunction->getNestedConstraints()[0] - ->getFirstType() - ->getAs(); - if (!boundVar) - return nullptr; - - auto constraints = CG.gatherConstraints( - boundVar, ConstraintGraph::GatheringKind::EquivalenceClass, - [](Constraint *constraint) { - return constraint->getKind() == ConstraintKind::ApplicableFunction; - }); - - if (constraints.size() != 1) - return nullptr; - - auto *applicableFn = constraints.front(); - // Unapplied disjunction could appear as a argument to applicable function, - // we are not interested in that. - return applicableFn->getSecondType()->isEqual(boundVar) ? applicableFn - : nullptr; -} - -void forEachDisjunctionChoice( - ConstraintSystem &cs, Constraint *disjunction, - llvm::function_ref - callback) { - for (auto constraint : disjunction->getNestedConstraints()) { - if (constraint->isDisabled()) - continue; - - if (constraint->getKind() != ConstraintKind::BindOverload) - continue; - - auto choice = constraint->getOverloadChoice(); - auto *decl = choice.getDeclOrNull(); - if (!decl) - continue; - - // If disjunction choice is unavailable or disfavored we cannot - // do anything with it. - if (decl->getAttrs().hasAttribute() || - cs.isDeclUnavailable(decl, disjunction->getLocator())) - continue; - - Type overloadType = - cs.getEffectiveOverloadType(disjunction->getLocator(), choice, - /*allowMembers=*/true, cs.DC); - - if (!overloadType || !overloadType->is()) - continue; - - callback(constraint, decl, overloadType->castTo()); - } -} - -} // end anonymous namespace - -/// Given a set of disjunctions, attempt to determine -/// favored choices in the current context. -static void determineBestChoicesInContext( - ConstraintSystem &cs, SmallVectorImpl &disjunctions, - llvm::DenseMap> - &favorings) { - double bestOverallScore = 0.0; - // Tops scores across all of the disjunctions. - llvm::DenseMap disjunctionScores; - llvm::DenseMap> - favoredChoicesPerDisjunction; - - for (auto *disjunction : disjunctions) { - auto applicableFn = - getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); - - if (applicableFn.isNull()) - continue; - - auto argFuncType = - applicableFn.get()->getFirstType()->getAs(); - - SmallVector, 2>, 2> - candidateArgumentTypes; - candidateArgumentTypes.resize(argFuncType->getNumParams()); - - llvm::TinyPtrVector resultTypes; - - for (unsigned i = 0, n = argFuncType->getNumParams(); i != n; ++i) { - const auto ¶m = argFuncType->getParams()[i]; - auto argType = cs.simplifyType(param.getPlainType()); - - SmallVector, 2> types; - if (auto *typeVar = argType->getAs()) { - auto bindingSet = cs.getBindingsFor(typeVar); - - for (const auto &binding : bindingSet.Bindings) { - types.push_back({binding.BindingType, /*fromLiteral=*/false}); - } - - for (const auto &literal : bindingSet.Literals) { - if (literal.second.hasDefaultType()) { - // Add primary default type - types.push_back( - {literal.second.getDefaultType(), /*fromLiteral=*/true}); - } - } - } else { - types.push_back({argType, /*fromLiteral=*/false}); - } - - candidateArgumentTypes[i].append(types); - } - - auto resultType = cs.simplifyType(argFuncType->getResult()); - if (auto *typeVar = resultType->getAs()) { - auto bindingSet = cs.getBindingsFor(typeVar); - - for (const auto &binding : bindingSet.Bindings) { - resultTypes.push_back(binding.BindingType); - } - } else { - resultTypes.push_back(resultType); - } - - // The choice with the best score. - double bestScore = 0.0; - SmallVector, 2> favoredChoices; - - forEachDisjunctionChoice( - cs, disjunction, - [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { - // Don't consider generic overloads because we need conformance - // checking functionality to determine best favoring, preferring - // such overloads based only on concrete types leads to subpar - // choices due to missed information. - if (decl->getInterfaceType()->is()) - return; - - if (overloadType->getNumParams() != argFuncType->getNumParams()) - return; - - double score = 0.0; - for (unsigned i = 0, n = overloadType->getNumParams(); i != n; ++i) { - if (candidateArgumentTypes[i].empty()) - continue; - - const auto ¶m = overloadType->getParams()[i]; - const auto paramFlags = param.getParameterFlags(); - - // If parameter is variadic we cannot compare because we don't know - // real arity. - if (paramFlags.isVariadic()) - continue; - - auto paramType = param.getPlainType(); - - // FIXME: Let's skip matching function types for now - // because they have special rules for e.g. Concurrency - // (around @Sendable) and @convention(c). - if (paramType->is()) - continue; - - double argScore = 0.0; - for (auto const &candidate : candidateArgumentTypes[i]) { - auto candidateType = candidate.first; - - // `inout` parameter accepts only l-value argument. - if (paramFlags.isInOut() && !candidateType->is()) - continue; - - // The specifier only matters for `inout` check. - candidateType = candidateType->getWithoutSpecifierType(); - // Exact match on one of the candidate bindings. - if (candidateType->isEqual(paramType)) { - argScore = std::max( - argScore, /*fromLiteral=*/candidate.second ? 0.3 : 1.0); - } - } - - score += argScore; - } - - // Average the score to avoid disfavoring disjunctions with fewer - // parameters. - score /= overloadType->getNumParams(); - - // If one of the result types matches exactly, that's a good - // indication that overload choice should be favored. - // - // If nothing is known about the arguments it's only safe to - // check result for operators (except to standard comparison - // ones that all have the same result type), regular - // functions/methods and especially initializers could end up - // with a lot of favored overloads because on the result type alone. - if (score > 0 || - (decl->isOperator() && - !decl->getBaseIdentifier().isStandardComparisonOperator())) { - if (llvm::any_of( - resultTypes, [&overloadType](const Type candidateResultTy) { - auto overloadResultTy = overloadType->getResult(); - return candidateResultTy->isEqual(overloadResultTy); - })) { - score += 1.0; - } - } - - if (score > 0) { - favoredChoices.push_back({choice, score}); - bestScore = std::max(bestScore, score); - } - }); - - if (cs.isDebugMode()) { - PrintOptions PO; - PO.PrintTypesForDebugging = true; - - llvm::errs().indent(cs.solverState->getCurrentIndent()) - << "<<< Disjunction " - << disjunction->getNestedConstraints()[0]->getFirstType()->getString( - PO) - << " with score " << bestScore << "\n"; - } - - // No matching overload choices to favor. - if (bestScore == 0.0) - continue; - - bestOverallScore = std::max(bestOverallScore, bestScore); - - disjunctionScores[disjunction] = bestScore; - for (const auto &choice : favoredChoices) { - if (choice.second == bestScore) - favoredChoicesPerDisjunction[disjunction].push_back(choice.first); - } - } - - if (cs.isDebugMode() && bestOverallScore > 0) { - PrintOptions PO; - PO.PrintTypesForDebugging = true; - - auto getLogger = [&](unsigned extraIndent = 0) -> llvm::raw_ostream & { - return llvm::errs().indent(cs.solverState->getCurrentIndent() + - extraIndent); - }; - - { - auto &log = getLogger(); - log << "(Optimizing disjunctions: ["; - - interleave( - disjunctions, - [&](const auto *disjunction) { - log << disjunction->getNestedConstraints()[0] - ->getFirstType() - ->getString(PO); - }, - [&]() { log << ", "; }); - - log << "]\n"; - } - - getLogger(/*extraIndent=*/4) - << "Best overall score = " << bestOverallScore << '\n'; - - for (const auto &entry : disjunctionScores) { - getLogger(/*extraIndent=*/4) - << "[Disjunction '" - << entry.first->getNestedConstraints()[0]->getFirstType()->getString( - PO) - << "' with score = " << entry.second << '\n'; - - for (const auto *choice : favoredChoicesPerDisjunction[entry.first]) { - auto &log = getLogger(/*extraIndent=*/6); - - log << "- "; - choice->print(log, &cs.getASTContext().SourceMgr); - log << '\n'; - } - - getLogger(/*extraIdent=*/4) << "]\n"; - } - - getLogger() << ")\n"; - } - - for (auto &entry : disjunctionScores) { - if (entry.second != bestOverallScore) - continue; - - for (auto *choice : favoredChoicesPerDisjunction[entry.first]) - favorings[entry.first].push_back(choice); - } -} - -// Attempt to find a disjunction of bind constraints where all options -// in the disjunction are binding the same type variable. -// -// Prefer disjunctions where the bound type variable is also the -// 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) { - - if (disjunctions.empty()) - return nullptr; - - auto getAsTypeVar = [&cs](Type type) { - return cs.simplifyType(type)->getRValueType()->getAs(); - }; - - Constraint *firstBindDisjunction = nullptr; - for (auto *disjunction : disjunctions) { - auto choices = disjunction->getNestedConstraints(); - assert(!choices.empty()); - - auto *choice = choices.front(); - if (choice->getKind() != ConstraintKind::Bind) - continue; - - // We can judge disjunction based on the single choice - // because all of choices (of bind overload set) should - // have the same left-hand side. - // Only do this for simple type variable bindings, not for - // bindings like: ($T1) -> $T2 bind String -> Int - auto *typeVar = getAsTypeVar(choice->getFirstType()); - if (!typeVar) - continue; - - if (!firstBindDisjunction) - firstBindDisjunction = disjunction; - - auto constraints = cs.getConstraintGraph().gatherConstraints( - typeVar, ConstraintGraph::GatheringKind::EquivalenceClass, - [](Constraint *constraint) { - return constraint->getKind() == ConstraintKind::Conversion; - }); - - for (auto *constraint : constraints) { - if (typeVar == getAsTypeVar(constraint->getSecondType())) - return disjunction; - } - } - - // If we had any binding disjunctions, return the first of - // those. These ensure that we attempt to bind types earlier than - // trying the elements of other disjunctions, which can often mean - // we fail faster. - return firstBindDisjunction; -} - -Constraint *ConstraintSystem::selectDisjunction() { - SmallVector disjunctions; - - collectDisjunctions(disjunctions); - if (disjunctions.empty()) - return nullptr; - - if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) - return disjunction; - - llvm::DenseMap> favorings; - determineBestChoicesInContext(*this, disjunctions, favorings); - - // Pick the disjunction with the smallest number of favored, then active - // choices. - auto bestDisjunction = std::min_element( - disjunctions.begin(), disjunctions.end(), - [&](Constraint *first, Constraint *second) -> bool { - unsigned firstActive = first->countActiveNestedConstraints(); - unsigned secondActive = second->countActiveNestedConstraints(); - unsigned firstFavored = favorings[first].size(); - unsigned secondFavored = favorings[second].size(); - - // Everything else equal, choose the disjunction with the greatest - // number of resolved argument types. The number of resolved argument - // types is always zero for disjunctions that don't represent applied - // overloads. - if (firstFavored == secondFavored) { - if (firstActive != secondActive) - return firstActive < secondActive; - - return first->countResolvedArgumentTypes(*this) > - second->countResolvedArgumentTypes(*this); - } - - firstFavored = firstFavored ? firstFavored : firstActive; - secondFavored = secondFavored ? secondFavored : secondActive; - return firstFavored < secondFavored; - }); - - if (bestDisjunction != disjunctions.end()) { - // If selected disjunction has any choices that should be favored - // let's record them now. - for (auto *choice : favorings[*bestDisjunction]) - favorConstraint(choice); - - return *bestDisjunction; - } - - return nullptr; +void ConstraintSystem::optimizeDisjunctions( + SmallVectorImpl &disjunctions) { } diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 8ba8d4e3586b2..ad82767eecbda 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1285,6 +1285,62 @@ ConstraintSystem::filterDisjunction( return SolutionKind::Unsolved; } +// Attempt to find a disjunction of bind constraints where all options +// in the disjunction are binding the same type variable. +// +// Prefer disjunctions where the bound type variable is also the +// 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) { + + if (disjunctions.empty()) + return nullptr; + + auto getAsTypeVar = [&cs](Type type) { + return cs.simplifyType(type)->getRValueType()->getAs(); + }; + + Constraint *firstBindDisjunction = nullptr; + for (auto *disjunction : disjunctions) { + auto choices = disjunction->getNestedConstraints(); + assert(!choices.empty()); + + auto *choice = choices.front(); + if (choice->getKind() != ConstraintKind::Bind) + continue; + + // We can judge disjunction based on the single choice + // because all of choices (of bind overload set) should + // have the same left-hand side. + // Only do this for simple type variable bindings, not for + // bindings like: ($T1) -> $T2 bind String -> Int + auto *typeVar = getAsTypeVar(choice->getFirstType()); + if (!typeVar) + continue; + + if (!firstBindDisjunction) + firstBindDisjunction = disjunction; + + auto constraints = cs.getConstraintGraph().gatherConstraints( + typeVar, ConstraintGraph::GatheringKind::EquivalenceClass, + [](Constraint *constraint) { + return constraint->getKind() == ConstraintKind::Conversion; + }); + + for (auto *constraint : constraints) { + if (typeVar == getAsTypeVar(constraint->getSecondType())) + return disjunction; + } + } + + // If we had any binding disjunctions, return the first of + // those. These ensure that we attempt to bind types earlier than + // trying the elements of other disjunctions, which can often mean + // we fail faster. + return firstBindDisjunction; +} std::optional> ConstraintSystem::findConstraintThroughOptionals( @@ -1760,6 +1816,63 @@ void DisjunctionChoiceProducer::partitionDisjunction( assert(Ordering.size() == Choices.size()); } +Constraint *ConstraintSystem::selectDisjunction() { + SmallVector disjunctions; + + collectDisjunctions(disjunctions); + if (disjunctions.empty()) + return nullptr; + + optimizeDisjunctions(disjunctions); + + if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) + return disjunction; + + // Pick the disjunction with the smallest number of favored, then active choices. + auto cs = this; + auto minDisjunction = std::min_element(disjunctions.begin(), disjunctions.end(), + [&](Constraint *first, Constraint *second) -> bool { + unsigned firstActive = first->countActiveNestedConstraints(); + unsigned secondActive = second->countActiveNestedConstraints(); + unsigned firstFavored = first->countFavoredNestedConstraints(); + unsigned secondFavored = second->countFavoredNestedConstraints(); + + if (!isOperatorDisjunction(first) || !isOperatorDisjunction(second)) + return firstActive < secondActive; + + if (firstFavored == secondFavored) { + // Look for additional choices that are "favored" + SmallVector firstExisting; + SmallVector secondExisting; + + existingOperatorBindingsForDisjunction(*cs, first->getNestedConstraints(), firstExisting); + firstFavored += firstExisting.size(); + existingOperatorBindingsForDisjunction(*cs, second->getNestedConstraints(), secondExisting); + secondFavored += secondExisting.size(); + } + + // Everything else equal, choose the disjunction with the greatest + // number of resolved argument types. The number of resolved argument + // types is always zero for disjunctions that don't represent applied + // overloads. + if (firstFavored == secondFavored) { + if (firstActive != secondActive) + return firstActive < secondActive; + + return (first->countResolvedArgumentTypes(*this) > second->countResolvedArgumentTypes(*this)); + } + + firstFavored = firstFavored ? firstFavored : firstActive; + secondFavored = secondFavored ? secondFavored : secondActive; + return firstFavored < secondFavored; + }); + + if (minDisjunction != disjunctions.end()) + return *minDisjunction; + + return nullptr; +} + Constraint *ConstraintSystem::selectConjunction() { SmallVector conjunctions; for (auto &constraint : InactiveConstraints) { diff --git a/test/Constraints/common_type.swift b/test/Constraints/common_type.swift index bad4415cf63f1..32f3bf3c8bb35 100644 --- a/test/Constraints/common_type.swift +++ b/test/Constraints/common_type.swift @@ -1,8 +1,6 @@ // RUN: %target-typecheck-verify-swift -debug-constraints 2>%t.err // RUN: %FileCheck %s < %t.err -// REQUIRES: needs_adjustment_for_new_favoring - struct X { func g(_: Int) -> Int { return 0 } func g(_: Double) -> Int { return 0 } diff --git a/test/Constraints/diag_ambiguities.swift b/test/Constraints/diag_ambiguities.swift index d6b358609c771..cb328fb0d5ed4 100644 --- a/test/Constraints/diag_ambiguities.swift +++ b/test/Constraints/diag_ambiguities.swift @@ -29,11 +29,11 @@ C(g) // expected-error{{ambiguous use of 'g'}} func h(_ x: T) -> () {} _ = C(h) // OK - init(_: (Int) -> ()) -func rdar29691909_callee(_ o: AnyObject?) -> Any? { return o } -func rdar29691909_callee(_ o: AnyObject) -> Any { return o } +func rdar29691909_callee(_ o: AnyObject?) -> Any? { return o } // expected-note {{found this candidate}} +func rdar29691909_callee(_ o: AnyObject) -> Any { return o } // expected-note {{found this candidate}} func rdar29691909(o: AnyObject) -> Any? { - return rdar29691909_callee(o) + return rdar29691909_callee(o) // expected-error{{ambiguous use of 'rdar29691909_callee'}} } func rdar29907555(_ value: Any!) -> String { diff --git a/validation-test/Sema/type_checker_perf/fast/rdar22770433.swift b/validation-test/Sema/type_checker_perf/slow/rdar22770433.swift similarity index 74% rename from validation-test/Sema/type_checker_perf/fast/rdar22770433.swift rename to validation-test/Sema/type_checker_perf/slow/rdar22770433.swift index 0c51c9346d7ac..f063e685689de 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar22770433.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar22770433.swift @@ -2,6 +2,7 @@ // REQUIRES: tools-release,no_asan func test(n: Int) -> Int { + // expected-error@+1 {{the compiler is unable to type-check this expression in reasonable time}} return n == 0 ? 0 : (0.. 0 && $1 % 2 == 0) ? ((($0 + $1) - ($0 + $1)) / ($1 - $0)) + (($0 + $1) / ($1 - $0)) : $0 } diff --git a/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift b/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift similarity index 91% rename from validation-test/Sema/type_checker_perf/fast/rdar23682605.swift rename to validation-test/Sema/type_checker_perf/slow/rdar23682605.swift index 4be53b4d2ef83..b7bc757fe1989 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift @@ -14,6 +14,7 @@ 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/fast/rdar31742586.swift b/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift similarity index 67% rename from validation-test/Sema/type_checker_perf/fast/rdar31742586.swift rename to validation-test/Sema/type_checker_perf/slow/rdar31742586.swift index 2c694c570a137..0cc33ce8253cf 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift @@ -3,4 +3,5 @@ func rdar31742586() -> Double { return -(1 + 2) + -(3 + 4) + 5 - (-(1 + 2) + -(3 + 4) + 5) + // 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/rdar35213699.swift b/validation-test/Sema/type_checker_perf/slow/rdar35213699.swift similarity index 66% rename from validation-test/Sema/type_checker_perf/fast/rdar35213699.swift rename to validation-test/Sema/type_checker_perf/slow/rdar35213699.swift index ea333f993b6a3..5d89ab1541a89 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar35213699.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar35213699.swift @@ -3,4 +3,6 @@ func test() { let _: UInt = 1 * 2 + 3 * 4 + 5 * 6 + 7 * 8 + 9 * 10 + 11 * 12 + 13 * 14 + // 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/rdar46713933_literal_arg.swift b/validation-test/Sema/type_checker_perf/slow/rdar46713933_literal_arg.swift similarity index 85% rename from validation-test/Sema/type_checker_perf/fast/rdar46713933_literal_arg.swift rename to validation-test/Sema/type_checker_perf/slow/rdar46713933_literal_arg.swift index 5256a92a787c7..a0628335b9c36 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar46713933_literal_arg.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar46713933_literal_arg.swift @@ -8,4 +8,5 @@ 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/rdar91310777.swift b/validation-test/Sema/type_checker_perf/slow/rdar91310777.swift similarity index 66% rename from validation-test/Sema/type_checker_perf/fast/rdar91310777.swift rename to validation-test/Sema/type_checker_perf/slow/rdar91310777.swift index 18450b15fe401..eb9dcbee848c8 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar91310777.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar91310777.swift @@ -9,6 +9,7 @@ func test() { compute { print(x) let v: UInt64 = UInt64((24 / UInt32(1)) + UInt32(0) - UInt32(0) - 24 / 42 - 42) + // expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time; try breaking up the expression into distinct sub-expressions}} print(v) } } From 4126a0e9bdc027e699c226a19f330a2cd308ea95 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:24:16 -0800 Subject: [PATCH 66/72] Revert "[ConstraintSystem] Add skeleton of constraint optimizer" This reverts commit b5f08a4009a094463e8fe138b33448b817272e44. --- lib/Sema/CMakeLists.txt | 1 - lib/Sema/CSOptimizer.cpp | 25 ------------------------- lib/Sema/CSSolver.cpp | 2 -- 3 files changed, 28 deletions(-) delete mode 100644 lib/Sema/CSOptimizer.cpp diff --git a/lib/Sema/CMakeLists.txt b/lib/Sema/CMakeLists.txt index 1bf3c664b4bd9..e250812244310 100644 --- a/lib/Sema/CMakeLists.txt +++ b/lib/Sema/CMakeLists.txt @@ -13,7 +13,6 @@ add_swift_host_library(swiftSema STATIC CSStep.cpp CSTrail.cpp CSFix.cpp - CSOptimizer.cpp CSDiagnostics.cpp CodeSynthesis.cpp CodeSynthesisDistributedActor.cpp diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp deleted file mode 100644 index 0bac2925d1f53..0000000000000 --- a/lib/Sema/CSOptimizer.cpp +++ /dev/null @@ -1,25 +0,0 @@ -//===--- CSOptimizer.cpp - Constraint Optimizer ---------------------------===// -// -// This source file is part of the Swift.org open source project -// -// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors -// Licensed under Apache License v2.0 with Runtime Library Exception -// -// See https://swift.org/LICENSE.txt for license information -// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors -// -//===----------------------------------------------------------------------===// -// -// This file implements disjunction and other constraint optimizations. -// -//===----------------------------------------------------------------------===// - -#include "swift/Sema/ConstraintSystem.h" -#include "llvm/ADT/SmallVector.h" - -using namespace swift; -using namespace constraints; - -void ConstraintSystem::optimizeDisjunctions( - SmallVectorImpl &disjunctions) { -} diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index ad82767eecbda..7a58921506ff0 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1823,8 +1823,6 @@ Constraint *ConstraintSystem::selectDisjunction() { if (disjunctions.empty()) return nullptr; - optimizeDisjunctions(disjunctions); - if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return disjunction; From c1bd2b2d089b064e277034eb9cac0218f6ef39ea Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:24:18 -0800 Subject: [PATCH 67/72] Revert "[CSGen] Remove ConstraintOptimizer and all favoring logic" This reverts commit 4432c51f57b30f1db7af9d05e0bbba747d02fa8b. --- include/swift/Sema/ConstraintSystem.h | 5 + lib/Sema/CSGen.cpp | 791 +++++++++++++++++++++++++- lib/Sema/CSSimplify.cpp | 36 ++ 3 files changed, 831 insertions(+), 1 deletion(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index e4ddf2a62e836..40e184dfa0abc 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5436,6 +5436,11 @@ class ConstraintSystem { bool applySolutionToBody(TapExpr *tapExpr, SyntacticElementTargetRewriter &rewriter); + /// Reorder the disjunctive clauses for a given expression to + /// increase the likelihood that a favored constraint will be successfully + /// resolved before any others. + void optimizeConstraints(Expr *e); + void startExpressionTimer(ExpressionTimer::AnchorType anchor); /// Determine if we've already explored too many paths in an diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index 0e40d0bbacb5d..d97fefd5e8e01 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -43,6 +43,10 @@ using namespace swift; using namespace swift::constraints; +static bool isArithmeticOperatorDecl(ValueDecl *vd) { + return vd && vd->getBaseIdentifier().isArithmeticOperator(); +} + static bool mergeRepresentativeEquivalenceClasses(ConstraintSystem &CS, TypeVariableType* tyvar1, TypeVariableType* tyvar2) { @@ -73,6 +77,698 @@ 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; + + public: + LinkedExprCollector(llvm::SmallVectorImpl &linkedExprs) + : LinkedExprs(linkedExprs) {} + + MacroWalking getMacroWalkingBehavior() const override { + return MacroWalking::Arguments; + } + + PreWalkResult walkToExprPre(Expr *expr) override { + if (isa(expr)) + return Action::SkipNode(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 Action::SkipNode(expr); + } + + return Action::Continue(expr); + } + + /// Ignore statements. + PreWalkResult walkToStmtPre(Stmt *stmt) override { + return Action::SkipNode(stmt); + } + + /// Ignore declarations. + PreWalkAction walkToDeclPre(Decl *decl) override { + return Action::SkipNode(); + } + + /// Ignore patterns. + PreWalkResult walkToPatternPre(Pattern *pat) override { + return Action::SkipNode(pat); + } + + /// Ignore types. + PreWalkAction walkToTypeReprPre(TypeRepr *T) override { + return Action::SkipNode(); + } + }; + + /// 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) {} + + MacroWalking getMacroWalkingBehavior() const override { + return MacroWalking::Arguments; + } + + PreWalkResult walkToExprPre(Expr *expr) override { + if (isa(expr)) { + LTI.hasLiteral = true; + return Action::SkipNode(expr); + } + + if (isa(expr)) { + return Action::Continue(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 Action::SkipNode(expr); + } + + + if (isa(expr)) { + return Action::SkipNode(expr); + } + + if (auto FVE = dyn_cast(expr)) { + LTI.collectedTypes.insert(CS.getType(FVE).getPointer()); + return Action::SkipNode(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 Action::SkipNode(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 Action::SkipNode(expr); + } + + if (auto *binaryExpr = dyn_cast(expr)) { + LTI.binaryExprs.push_back(binaryExpr); + } + + if (auto favoredType = CS.getFavoredType(expr)) { + LTI.collectedTypes.insert(favoredType); + + return Action::SkipNode(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 Action::SkipNode(expr); + } + + // For exprs of a tuple, avoid favoring. (We need to allow for cases like + // (Int, Int32).) + if (isa(expr)) { + return Action::SkipNode(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 Action::SkipNode(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 Action::SkipNode(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 Action::SkipNode(expr); + } + + return Action::Continue(expr); + } + + /// Ignore statements. + PreWalkResult walkToStmtPre(Stmt *stmt) override { + return Action::SkipNode(stmt); + } + + /// Ignore declarations. + PreWalkAction walkToDeclPre(Decl *decl) override { + return Action::SkipNode(); + } + + /// Ignore patterns. + PreWalkResult walkToPatternPre(Pattern *pat) override { + return Action::SkipNode(pat); + } + + /// Ignore types. + PreWalkAction walkToTypeReprPre(TypeRepr *T) override { + return Action::SkipNode(); + } + }; + + /// 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, + Type otherArgTy = Type()) { + // Determine the argument type. + argTy = argTy->getWithoutSpecifierType(); + + // Do the types match exactly? + if (paramTy->isEqual(argTy)) + return true; + + // Don't favor narrowing conversions. + if (argTy->isDouble() && paramTy->isCGFloat()) + return false; + + llvm::SmallSetVector literalProtos; + if (auto argTypeVar = argTy->getAs()) { + auto constraints = CS.getConstraintGraph().gatherConstraints( + argTypeVar, ConstraintGraph::GatheringKind::EquivalenceClass, + [](Constraint *constraint) { + return constraint->getKind() == ConstraintKind::LiteralConformsTo; + }); + + for (auto constraint : constraints) { + literalProtos.insert(constraint->getProtocol()); + } + } + + // Dig out the second argument type. + if (otherArgTy) + otherArgTy = otherArgTy->getWithoutSpecifierType(); + + for (auto literalProto : literalProtos) { + // If there is another, concrete argument, check whether it's type + // conforms to the literal protocol and test against it directly. + // This helps to avoid 'widening' the favored type to the default type for + // the literal. + if (otherArgTy && otherArgTy->getAnyNominal()) { + if (otherArgTy->isEqual(paramTy) && + CS.lookupConformance(otherArgTy, literalProto)) { + return true; + } + } else if (Type defaultType = + TypeChecker::getDefaultType(literalProto, CS.DC)) { + // If there is a default type for the literal protocol, check whether + // it is the same as the parameter type. + // Check whether there is a default type to compare against. + if (paramTy->isEqual(defaultType) || + (defaultType->isDouble() && paramTy->isCGFloat())) + return true; + } + } + + return false; + } + + /// Favor certain overloads in a call based on some basic analysis + /// of the overload set and call arguments. + /// + /// \param expr The application. + /// \param isFavored Determine whether the given overload is favored, passing + /// it the "effective" overload type when it's being called. + /// \param mustConsider If provided, a function to detect the presence of + /// overloads which inhibit any overload from being favored. + void favorCallOverloads(ApplyExpr *expr, + ConstraintSystem &CS, + llvm::function_ref isFavored, + std::function + mustConsider = nullptr) { + // Find the type variable associated with the function, if any. + auto tyvarType = CS.getType(expr->getFn())->getAs(); + if (!tyvarType || CS.getFixedType(tyvarType)) + return; + + // This type variable is only currently associated with the function + // being applied, and the only constraint attached to it should + // be the disjunction constraint for the overload group. + auto disjunction = CS.getUnboundBindOverloadDisjunction(tyvarType); + if (!disjunction) + return; + + // Find the favored constraints and mark them. + SmallVector newlyFavoredConstraints; + unsigned numFavoredConstraints = 0; + Constraint *firstFavored = nullptr; + for (auto constraint : disjunction->getNestedConstraints()) { + auto *decl = constraint->getOverloadChoice().getDeclOrNull(); + if (!decl) + continue; + + if (mustConsider && mustConsider(decl)) { + // Roll back any constraints we favored. + for (auto favored : newlyFavoredConstraints) + favored->setFavored(false); + + return; + } + + Type overloadType = CS.getEffectiveOverloadType( + constraint->getLocator(), constraint->getOverloadChoice(), + /*allowMembers=*/true, CS.DC); + if (!overloadType) + continue; + + if (!CS.isDeclUnavailable(decl, constraint->getLocator()) && + !decl->getAttrs().hasAttribute() && + isFavored(decl, overloadType)) { + // If we might need to roll back the favored constraints, keep + // track of those we are favoring. + if (mustConsider && !constraint->isFavored()) + newlyFavoredConstraints.push_back(constraint); + + constraint->setFavored(); + ++numFavoredConstraints; + if (!firstFavored) + firstFavored = constraint; + } + } + + // If there was one favored constraint, set the favored type based on its + // result type. + if (numFavoredConstraints == 1) { + auto overloadChoice = firstFavored->getOverloadChoice(); + auto overloadType = CS.getEffectiveOverloadType( + firstFavored->getLocator(), overloadChoice, /*allowMembers=*/true, + CS.DC); + auto resultType = overloadType->castTo()->getResult(); + if (!resultType->hasTypeParameter()) + CS.setFavoredType(expr, resultType.getPointer()); + } + } + + /// Return a pair, containing the total parameter count of a function, coupled + /// with the number of non-default parameters. + std::pair getParamCount(ValueDecl *VD) { + auto fTy = VD->getInterfaceType()->castTo(); + + size_t nOperands = fTy->getParams().size(); + size_t nNoDefault = 0; + + if (auto AFD = dyn_cast(VD)) { + assert(!AFD->hasImplicitSelfDecl()); + for (auto param : *AFD->getParameters()) { + if (!param->isDefaultArgument()) + ++nNoDefault; + } + } else { + nNoDefault = nOperands; + } + + return { nOperands, nNoDefault }; + } + + bool hasContextuallyFavorableResultType(AnyFunctionType *choice, + Type contextualTy) { + // No restrictions of what result could be. + if (!contextualTy) + return true; + + auto resultTy = choice->getResult(); + // Result type of the call matches expected contextual type. + return contextualTy->isEqual(resultTy); + } + + /// Favor unary operator constraints where we have exact matches + /// for the operand and contextual type. + void favorMatchingUnaryOperators(ApplyExpr *expr, + ConstraintSystem &CS) { + auto *unaryArg = expr->getArgs()->getUnaryExpr(); + assert(unaryArg); + + // Determine whether the given declaration is favored. + auto isFavoredDecl = [&](ValueDecl *value, Type type) -> bool { + auto fnTy = type->getAs(); + if (!fnTy) + return false; + + auto params = fnTy->getParams(); + if (params.size() != 1) + return false; + + auto paramTy = params[0].getPlainType(); + auto argTy = CS.getType(unaryArg); + + // There are no CGFloat overloads on some of the unary operators, so + // in order to preserve current behavior, let's not favor overloads + // which would result in conversion from CGFloat to Double; otherwise + // it would lead to ambiguities. + if (argTy->isCGFloat() && paramTy->isDouble()) + return false; + + return isFavoredParamAndArg(CS, paramTy, argTy) && + hasContextuallyFavorableResultType( + fnTy, + CS.getContextualType(expr, /*forConstraint=*/false)); + }; + + favorCallOverloads(expr, CS, isFavoredDecl); + } + + void favorMatchingOverloadExprs(ApplyExpr *expr, + ConstraintSystem &CS) { + // Find the argument type. + size_t nArgs = expr->getArgs()->size(); + auto fnExpr = expr->getFn(); + + auto mustConsiderVariadicGenericOverloads = [&](ValueDecl *overload) { + if (overload->getAttrs().hasAttribute()) + return false; + + auto genericContext = overload->getAsGenericContext(); + if (!genericContext) + return false; + + auto *GPL = genericContext->getGenericParams(); + if (!GPL) + return false; + + return llvm::any_of(GPL->getParams(), + [&](const GenericTypeParamDecl *GP) { + return GP->isParameterPack(); + }); + }; + + // Check to ensure that we have an OverloadedDeclRef, and that we're not + // favoring multiple overload constraints. (Otherwise, in this case + // favoring is useless. + if (auto ODR = dyn_cast(fnExpr)) { + bool haveMultipleApplicableOverloads = false; + + for (auto VD : ODR->getDecls()) { + if (VD->getInterfaceType()->is()) { + auto nParams = getParamCount(VD); + + if (nArgs == nParams.first) { + if (haveMultipleApplicableOverloads) { + return; + } else { + haveMultipleApplicableOverloads = true; + } + } + } + } + + // Determine whether the given declaration is favored. + auto isFavoredDecl = [&](ValueDecl *value, Type type) -> bool { + // We want to consider all options for calls that might contain the code + // completion location, as missing arguments after the completion + // location are valid (since it might be that they just haven't been + // written yet). + if (CS.isForCodeCompletion()) + return false; + + if (!type->is()) + return false; + + auto paramCount = getParamCount(value); + + return nArgs == paramCount.first || + nArgs == paramCount.second; + }; + + favorCallOverloads(expr, CS, isFavoredDecl, + mustConsiderVariadicGenericOverloads); + } + + // We only currently perform favoring for unary args. + auto *unaryArg = expr->getArgs()->getUnlabeledUnaryExpr(); + if (!unaryArg) + return; + + if (auto favoredTy = CS.getFavoredType(unaryArg)) { + // Determine whether the given declaration is favored. + auto isFavoredDecl = [&](ValueDecl *value, Type type) -> bool { + auto fnTy = type->getAs(); + if (!fnTy || fnTy->getParams().size() != 1) + return false; + + return favoredTy->isEqual(fnTy->getParams()[0].getPlainType()); + }; + + // This is a hack to ensure we always consider the protocol requirement + // itself when calling something that has a default implementation in an + // extension. Otherwise, the extension method might be favored if we're + // inside an extension context, since any archetypes in the parameter + // list could match exactly. + auto mustConsider = [&](ValueDecl *value) -> bool { + return isa(value->getDeclContext()) || + mustConsiderVariadicGenericOverloads(value); + }; + + favorCallOverloads(expr, CS, isFavoredDecl, mustConsider); + } + } + + /// Favor binary operator constraints where we have exact matches + /// for the operands and contextual type. + void favorMatchingBinaryOperators(ApplyExpr *expr, ConstraintSystem &CS) { + // If we're generating constraints for a binary operator application, + // there are two special situations to consider: + // 1. If the type checker has any newly created functions with the + // operator's name. If it does, the overloads were created after the + // associated overloaded id expression was created, and we'll need to + // add a new disjunction constraint for the new set of overloads. + // 2. If any component argument expressions (nested or otherwise) are + // literals, we can favor operator overloads whose argument types are + // identical to the literal type, or whose return types are identical + // to any contextual type associated with the application expression. + + // Find the argument types. + auto *args = expr->getArgs(); + auto *lhs = args->getExpr(0); + auto *rhs = args->getExpr(1); + + auto firstArgTy = CS.getType(lhs); + auto secondArgTy = CS.getType(rhs); + + auto isOptionalWithMatchingObjectType = [](Type optional, + Type object) -> bool { + if (auto objTy = optional->getRValueType()->getOptionalObjectType()) + return objTy->getRValueType()->isEqual(object->getRValueType()); + + return false; + }; + + auto isPotentialForcingOpportunity = [&](Type first, Type second) -> bool { + return isOptionalWithMatchingObjectType(first, second) || + isOptionalWithMatchingObjectType(second, first); + }; + + // Determine whether the given declaration is favored. + auto isFavoredDecl = [&](ValueDecl *value, Type type) -> bool { + auto fnTy = type->getAs(); + if (!fnTy) + return false; + + auto firstFavoredTy = CS.getFavoredType(lhs); + auto secondFavoredTy = CS.getFavoredType(rhs); + + auto favoredExprTy = CS.getFavoredType(expr); + + if (isArithmeticOperatorDecl(value)) { + // If the parent has been favored on the way down, propagate that + // information to its children. + // TODO: This is only valid for arithmetic expressions. + if (!firstFavoredTy) { + CS.setFavoredType(lhs, favoredExprTy); + firstFavoredTy = favoredExprTy; + } + + if (!secondFavoredTy) { + CS.setFavoredType(rhs, favoredExprTy); + secondFavoredTy = favoredExprTy; + } + } + + auto params = fnTy->getParams(); + if (params.size() != 2) + return false; + + auto firstParamTy = params[0].getOldType(); + auto secondParamTy = params[1].getOldType(); + + auto contextualTy = CS.getContextualType(expr, /*forConstraint=*/false); + + // Avoid favoring overloads that would require narrowing conversion + // to match the arguments. + { + if (firstArgTy->isDouble() && firstParamTy->isCGFloat()) + return false; + + if (secondArgTy->isDouble() && secondParamTy->isCGFloat()) + return false; + } + + return (isFavoredParamAndArg(CS, firstParamTy, firstArgTy, secondArgTy) || + isFavoredParamAndArg(CS, secondParamTy, secondArgTy, + firstArgTy)) && + firstParamTy->isEqual(secondParamTy) && + !isPotentialForcingOpportunity(firstArgTy, secondArgTy) && + hasContextuallyFavorableResultType(fnTy, contextualTy); + }; + + favorCallOverloads(expr, CS, isFavoredDecl); + } + /// If \p expr is a call and that call contains the code completion token, /// add the expressions of all arguments after the code completion token to /// \p ignoredArguments. @@ -103,6 +799,62 @@ namespace { } } } + + class ConstraintOptimizer : public ASTWalker { + ConstraintSystem &CS; + + public: + + ConstraintOptimizer(ConstraintSystem &cs) : + CS(cs) {} + + MacroWalking getMacroWalkingBehavior() const override { + return MacroWalking::Arguments; + } + + PreWalkResult walkToExprPre(Expr *expr) override { + if (CS.isArgumentIgnoredForCodeCompletion(expr)) { + return Action::SkipNode(expr); + } + + if (auto applyExpr = dyn_cast(expr)) { + if (isa(applyExpr) || + isa(applyExpr)) { + favorMatchingUnaryOperators(applyExpr, CS); + } else if (isa(applyExpr)) { + favorMatchingBinaryOperators(applyExpr, CS); + } else { + favorMatchingOverloadExprs(applyExpr, CS); + } + } + + // If the paren expr has a favored type, and the subExpr doesn't, + // propagate downwards. Otherwise, propagate upwards. + if (auto parenExpr = dyn_cast(expr)) { + if (!CS.getFavoredType(parenExpr->getSubExpr())) { + CS.setFavoredType(parenExpr->getSubExpr(), + CS.getFavoredType(parenExpr)); + } else if (!CS.getFavoredType(parenExpr)) { + CS.setFavoredType(parenExpr, + CS.getFavoredType(parenExpr->getSubExpr())); + } + } + + if (isa(expr)) + return Action::SkipNode(expr); + + return Action::Continue(expr); + } + /// Ignore statements. + PreWalkResult walkToStmtPre(Stmt *stmt) override { + return Action::SkipNode(stmt); + } + + /// Ignore declarations. + PreWalkAction walkToDeclPre(Decl *decl) override { + return Action::SkipNode(); + } + }; } // end anonymous namespace void TypeVarRefCollector::inferTypeVars(Decl *D) { @@ -348,6 +1100,18 @@ namespace { if (isLValueBase) outputTy = LValueType::get(outputTy); } + } else if (auto dictTy = CS.isDictionaryType(baseObjTy)) { + auto keyTy = dictTy->first; + auto valueTy = dictTy->second; + + if (argList->isUnlabeledUnary()) { + auto argTy = CS.getType(argList->getExpr(0)); + if (isFavoredParamAndArg(CS, keyTy, argTy)) { + outputTy = OptionalType::get(valueTy); + if (isLValueBase) + outputTy = LValueType::get(outputTy); + } + } } } @@ -3604,7 +4368,12 @@ static Expr *generateConstraintsFor(ConstraintSystem &cs, Expr *expr, ConstraintGenerator cg(cs, DC); ConstraintWalker cw(cg); - return expr->walk(cw); + Expr *result = expr->walk(cw); + + if (result) + cs.optimizeConstraints(result); + + return result; } bool ConstraintSystem::generateWrappedPropertyTypeConstraints( @@ -4348,6 +5117,26 @@ ConstraintSystem::applyPropertyWrapperToParameter( return getTypeMatchSuccess(); } +void ConstraintSystem::optimizeConstraints(Expr *e) { + if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks) + return; + + SmallVector linkedExprs; + + // Collect any linked expressions. + LinkedExprCollector collector(linkedExprs); + e->walk(collector); + + // Favor types, as appropriate. + for (auto linkedExpr : linkedExprs) { + computeFavoredTypeForExpr(linkedExpr, *this); + } + + // Optimize the constraints. + ConstraintOptimizer optimizer(*this); + e->walk(optimizer); +} + struct ResolvedMemberResult::Implementation { llvm::SmallVector AllDecls; unsigned ViableStartIdx; diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 7e4d212b372b0..1a8fd13908134 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -9937,6 +9937,7 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, // If this is true, we're using type construction syntax (Foo()) rather // than an explicit call to `init` (Foo.init()). bool isImplicitInit = false; + TypeBase *favoredType = nullptr; if (memberName.isSimpleName(DeclBaseName::createConstructor())) { SmallVector parts; if (auto anchor = memberLocator->getAnchor()) { @@ -9944,6 +9945,17 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, if (!path.empty()) if (path.back().getKind() == ConstraintLocator::ConstructorMember) isImplicitInit = true; + + if (auto *applyExpr = getAsExpr(anchor)) { + if (auto *argExpr = applyExpr->getArgs()->getUnlabeledUnaryExpr()) { + favoredType = getFavoredType(argExpr); + + if (!favoredType) { + optimizeConstraints(argExpr); + favoredType = getFavoredType(argExpr); + } + } + } } } @@ -10052,6 +10064,30 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, hasInstanceMethods = true; } + // If the invocation's argument expression has a favored type, + // use that information to determine whether a specific overload for + // the candidate should be favored. + if (isa(decl) && favoredType && + result.FavoredChoice == ~0U) { + auto *ctor = cast(decl); + + // Only try and favor monomorphic unary initializers. + if (!ctor->isGenericContext()) { + if (!ctor->getMethodInterfaceType()->hasError()) { + // The constructor might have an error type because we don't skip + // invalid decls for code completion + auto args = ctor->getMethodInterfaceType() + ->castTo() + ->getParams(); + if (args.size() == 1 && !args[0].hasLabel() && + args[0].getPlainType()->isEqual(favoredType)) { + if (!isDeclUnavailable(decl, memberLocator)) + result.FavoredChoice = result.ViableCandidates.size(); + } + } + } + } + const auto isUnsupportedExistentialMemberAccess = [&] { // We may not be able to derive a well defined type for an existential // member access if the member's signature references 'Self'. From 778a7df69e503b03564a84fea381e86a83203cb3 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 00:24:20 -0800 Subject: [PATCH 68/72] Revert "[ConstraintSystem] Remove `shrink`" This reverts commit 757ca24e8a9e30e14a3c26155978310b733e18ac. --- include/swift/Sema/ConstraintSystem.h | 78 ++++ lib/Sema/CSSolver.cpp | 552 ++++++++++++++++++++++++++ lib/Sema/TypeCheckConstraints.cpp | 4 + 3 files changed, 634 insertions(+) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 40e184dfa0abc..0138674030c80 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2458,6 +2458,75 @@ class ConstraintSystem { SynthesizedConformances; private: + /// Describe the candidate expression for partial solving. + /// This class used by shrink & solve methods which apply + /// variation of directional path consistency algorithm in attempt + /// to reduce scopes of the overload sets (disjunctions) in the system. + class Candidate { + Expr *E; + DeclContext *DC; + llvm::BumpPtrAllocator &Allocator; + + // Contextual Information. + Type CT; + ContextualTypePurpose CTP; + + public: + Candidate(ConstraintSystem &cs, Expr *expr, Type ct = Type(), + ContextualTypePurpose ctp = ContextualTypePurpose::CTP_Unused) + : E(expr), DC(cs.DC), Allocator(cs.Allocator), CT(ct), CTP(ctp) {} + + /// Return underlying expression. + Expr *getExpr() const { return E; } + + /// Try to solve this candidate sub-expression + /// and re-write it's OSR domains afterwards. + /// + /// \param shrunkExprs The set of expressions which + /// domains have been successfully shrunk so far. + /// + /// \returns true on solver failure, false otherwise. + bool solve(llvm::SmallSetVector &shrunkExprs); + + /// Apply solutions found by solver as reduced OSR sets for + /// for current and all of it's sub-expressions. + /// + /// \param solutions The solutions found by running solver on the + /// this candidate expression. + /// + /// \param shrunkExprs The set of expressions which + /// domains have been successfully shrunk so far. + void applySolutions( + llvm::SmallVectorImpl &solutions, + llvm::SmallSetVector &shrunkExprs) const; + + /// Check if attempt at solving of the candidate makes sense given + /// the current conditions - number of shrunk domains which is related + /// to the given candidate over the total number of disjunctions present. + static bool + isTooComplexGiven(ConstraintSystem *const cs, + llvm::SmallSetVector &shrunkExprs) { + SmallVector disjunctions; + cs->collectDisjunctions(disjunctions); + + unsigned unsolvedDisjunctions = disjunctions.size(); + for (auto *disjunction : disjunctions) { + auto *locator = disjunction->getLocator(); + if (!locator) + continue; + + if (auto *OSR = getAsExpr(locator->getAnchor())) { + if (shrunkExprs.count(OSR) > 0) + --unsolvedDisjunctions; + } + } + + unsigned threshold = + cs->getASTContext().TypeCheckerOpts.SolverShrinkUnsolvedThreshold; + return unsolvedDisjunctions >= threshold; + } + }; + /// Describes the current solver state. struct SolverState { SolverState(ConstraintSystem &cs, @@ -5244,6 +5313,15 @@ class ConstraintSystem { /// \returns true if an error occurred, false otherwise. bool solveSimplified(SmallVectorImpl &solutions); + /// Find reduced domains of disjunction constraints for given + /// expression, this is achieved to solving individual sub-expressions + /// and combining resolving types. Such algorithm is called directional + /// path consistency because it goes from children to parents for all + /// related sub-expressions taking union of their domains. + /// + /// \param expr The expression to find reductions for. + void shrink(Expr *expr); + /// Pick a disjunction from the InactiveConstraints list. /// /// \returns The selected disjunction. diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 7a58921506ff0..7df858000490b 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -787,6 +787,556 @@ ConstraintSystem::solveSingle(FreeTypeVariableBinding allowFreeTypeVariables, return std::move(solutions[0]); } +bool ConstraintSystem::Candidate::solve( + llvm::SmallSetVector &shrunkExprs) { + // Don't attempt to solve candidate if there is closure + // expression involved, because it's handled specially + // by parent constraint system (e.g. parameter lists). + bool containsClosure = false; + E->forEachChildExpr([&](Expr *childExpr) -> Expr * { + if (isa(childExpr)) { + containsClosure = true; + return nullptr; + } + return childExpr; + }); + + if (containsClosure) + return false; + + auto cleanupImplicitExprs = [&](Expr *expr) { + expr->forEachChildExpr([&](Expr *childExpr) -> Expr * { + Type type = childExpr->getType(); + if (childExpr->isImplicit() && type && type->hasTypeVariable()) + childExpr->setType(Type()); + return childExpr; + }); + }; + + // Allocate new constraint system for sub-expression. + ConstraintSystem cs(DC, std::nullopt); + + // Set up expression type checker timer for the candidate. + cs.startExpressionTimer(E); + + // Generate constraints for the new system. + if (auto generatedExpr = cs.generateConstraints(E, DC)) { + E = generatedExpr; + } else { + // Failure to generate constraint system for sub-expression + // means we can't continue solving sub-expressions. + cleanupImplicitExprs(E); + return true; + } + + // If this candidate is too complex given the number + // of the domains we have reduced so far, let's bail out early. + if (isTooComplexGiven(&cs, shrunkExprs)) + return false; + + auto &ctx = cs.getASTContext(); + if (cs.isDebugMode()) { + auto &log = llvm::errs(); + auto indent = cs.solverState ? cs.solverState->getCurrentIndent() : 0; + log.indent(indent) << "--- Solving candidate for shrinking at "; + auto R = E->getSourceRange(); + if (R.isValid()) { + R.print(log, ctx.SourceMgr, /*PrintText=*/ false); + } else { + log << ""; + } + log << " ---\n"; + + E->dump(log, indent); + log << '\n'; + cs.print(log); + } + + // If there is contextual type present, add an explicit "conversion" + // constraint to the system. + if (!CT.isNull()) { + auto constraintKind = ConstraintKind::Conversion; + if (CTP == CTP_CallArgument) + constraintKind = ConstraintKind::ArgumentConversion; + + cs.addConstraint(constraintKind, cs.getType(E), CT, + cs.getConstraintLocator(E), /*isFavored=*/true); + } + + // Try to solve the system and record all available solutions. + llvm::SmallVector solutions; + { + SolverState state(cs, FreeTypeVariableBinding::Allow); + + // Use solve which doesn't try to filter solution list. + // Because we want the whole set of possible domain choices. + cs.solveImpl(solutions); + } + + if (cs.isDebugMode()) { + auto &log = llvm::errs(); + auto indent = cs.solverState ? cs.solverState->getCurrentIndent() : 0; + if (solutions.empty()) { + log << "\n"; + log.indent(indent) << "--- No Solutions ---\n"; + } else { + log << "\n"; + log.indent(indent) << "--- Solutions ---\n"; + for (unsigned i = 0, n = solutions.size(); i != n; ++i) { + auto &solution = solutions[i]; + log << "\n"; + log.indent(indent) << "--- Solution #" << i << " ---\n"; + solution.dump(log, indent); + } + } + } + + // Record found solutions as suggestions. + this->applySolutions(solutions, shrunkExprs); + + // Let's double-check if we have any implicit expressions + // with type variables and nullify their types. + cleanupImplicitExprs(E); + + // No solutions for the sub-expression means that either main expression + // needs salvaging or it's inconsistent (read: doesn't have solutions). + return solutions.empty(); +} + +void ConstraintSystem::Candidate::applySolutions( + llvm::SmallVectorImpl &solutions, + llvm::SmallSetVector &shrunkExprs) const { + // A collection of OSRs with their newly reduced domains, + // it's domains are sets because multiple solutions can have the same + // choice for one of the type variables, and we want no duplication. + llvm::SmallDenseMap> + domains; + for (auto &solution : solutions) { + auto &score = solution.getFixedScore(); + + // Avoid any solutions with implicit value conversions + // because they might get reverted later when more context + // becomes available. + if (score.Data[SK_ImplicitValueConversion] > 0) + continue; + + for (auto choice : solution.overloadChoices) { + // Some of the choices might not have locators. + if (!choice.getFirst()) + continue; + + auto anchor = choice.getFirst()->getAnchor(); + auto *OSR = getAsExpr(anchor); + // Anchor is not available or expression is not an overload set. + if (!OSR) + continue; + + auto overload = choice.getSecond().choice; + auto type = overload.getDecl()->getInterfaceType(); + + // One of the solutions has polymorphic type associated with one of its + // type variables. Such functions can only be properly resolved + // via complete expression, so we'll have to forget solutions + // we have already recorded. They might not include all viable overload + // choices. + if (type->is()) { + return; + } + + domains[OSR].insert(overload.getDecl()); + } + } + + // Reduce the domains. + for (auto &domain : domains) { + auto OSR = domain.getFirst(); + auto &choices = domain.getSecond(); + + // If the domain wasn't reduced, skip it. + if (OSR->getDecls().size() == choices.size()) continue; + + // Update the expression with the reduced domain. + MutableArrayRef decls( + Allocator.Allocate(choices.size()), + choices.size()); + + std::uninitialized_copy(choices.begin(), choices.end(), decls.begin()); + OSR->setDecls(decls); + + // Record successfully shrunk expression. + shrunkExprs.insert(OSR); + } +} + +void ConstraintSystem::shrink(Expr *expr) { + if (getASTContext().TypeCheckerOpts.SolverDisableShrink) + return; + + using DomainMap = llvm::SmallDenseMap>; + + // A collection of original domains of all of the expressions, + // so they can be restored in case of failure. + DomainMap domains; + + struct ExprCollector : public ASTWalker { + Expr *PrimaryExpr; + + // The primary constraint system. + ConstraintSystem &CS; + + // All of the sub-expressions which are suitable to be solved + // separately from the main system e.g. binary expressions, collections, + // function calls, coercions etc. + llvm::SmallVector Candidates; + + // Counts the number of overload sets present in the tree so far. + // Note that the traversal is depth-first. + llvm::SmallVector, 4> ApplyExprs; + + // A collection of original domains of all of the expressions, + // so they can be restored in case of failure. + DomainMap &Domains; + + ExprCollector(Expr *expr, ConstraintSystem &cs, DomainMap &domains) + : PrimaryExpr(expr), CS(cs), Domains(domains) {} + + MacroWalking getMacroWalkingBehavior() const override { + return MacroWalking::Arguments; + } + + PreWalkResult walkToExprPre(Expr *expr) override { + // A dictionary expression is just a set of tuples; try to solve ones + // that have overload sets. + if (auto collectionExpr = dyn_cast(expr)) { + visitCollectionExpr(collectionExpr, + CS.getContextualType(expr, /*forConstraint=*/false), + CS.getContextualTypePurpose(expr)); + // Don't try to walk into the dictionary. + return Action::SkipNode(expr); + } + + // Let's not attempt to type-check closures or expressions + // which constrain closures, because they require special handling + // when dealing with context and parameters declarations. + if (isa(expr)) { + return Action::SkipNode(expr); + } + + // Similar to 'ClosureExpr', 'TapExpr' has a 'VarDecl' the type of which + // is determined by type checking the parent interpolated string literal. + if (isa(expr)) { + return Action::SkipNode(expr); + } + + // Same as TapExpr and ClosureExpr, we'll handle SingleValueStmtExprs + // separately. + if (isa(expr)) + return Action::SkipNode(expr); + + if (auto coerceExpr = dyn_cast(expr)) { + if (coerceExpr->isLiteralInit()) + ApplyExprs.push_back({coerceExpr, 1}); + visitCoerceExpr(coerceExpr); + return Action::SkipNode(expr); + } + + if (auto OSR = dyn_cast(expr)) { + Domains[OSR] = OSR->getDecls(); + } + + if (auto applyExpr = dyn_cast(expr)) { + auto func = applyExpr->getFn(); + // Let's record this function application for post-processing + // as well as if it contains overload set, see walkToExprPost. + ApplyExprs.push_back( + {applyExpr, isa(func) || isa(func)}); + } + + return Action::Continue(expr); + } + + /// Determine whether this is an arithmetic expression comprised entirely + /// of literals. + static bool isArithmeticExprOfLiterals(Expr *expr) { + expr = expr->getSemanticsProvidingExpr(); + + if (auto prefix = dyn_cast(expr)) + return isArithmeticExprOfLiterals(prefix->getOperand()); + + if (auto postfix = dyn_cast(expr)) + return isArithmeticExprOfLiterals(postfix->getOperand()); + + if (auto binary = dyn_cast(expr)) + return isArithmeticExprOfLiterals(binary->getLHS()) && + isArithmeticExprOfLiterals(binary->getRHS()); + + return isa(expr) || isa(expr); + } + + PostWalkResult walkToExprPost(Expr *expr) override { + auto isSrcOfPrimaryAssignment = [&](Expr *expr) -> bool { + if (auto *AE = dyn_cast(PrimaryExpr)) + return expr == AE->getSrc(); + return false; + }; + + if (expr == PrimaryExpr || isSrcOfPrimaryAssignment(expr)) { + // If this is primary expression and there are no candidates + // to be solved, let's not record it, because it's going to be + // solved regardless. + if (Candidates.empty()) + return Action::Continue(expr); + + auto contextualType = CS.getContextualType(expr, + /*forConstraint=*/false); + // If there is a contextual type set for this expression. + if (!contextualType.isNull()) { + Candidates.push_back(Candidate(CS, PrimaryExpr, contextualType, + CS.getContextualTypePurpose(expr))); + return Action::Continue(expr); + } + + // Or it's a function application or assignment with other candidates + // present. Assignment should be easy to solve because we'd get a + // contextual type from the destination expression, otherwise shrink + // might produce incorrect results without considering aforementioned + // destination type. + if (isa(expr) || isa(expr)) { + Candidates.push_back(Candidate(CS, PrimaryExpr)); + return Action::Continue(expr); + } + } + + if (!isa(expr)) + return Action::Continue(expr); + + unsigned numOverloadSets = 0; + // Let's count how many overload sets do we have. + while (!ApplyExprs.empty()) { + auto &application = ApplyExprs.back(); + auto applyExpr = application.first; + + // Add overload sets tracked by current expression. + numOverloadSets += application.second; + ApplyExprs.pop_back(); + + // We've found the current expression, so record the number of + // overloads. + if (expr == applyExpr) { + ApplyExprs.push_back({applyExpr, numOverloadSets}); + break; + } + } + + // If there are fewer than two overloads in the chain + // there is no point of solving this expression, + // because we won't be able to reduce its domain. + if (numOverloadSets > 1 && !isArithmeticExprOfLiterals(expr)) + Candidates.push_back(Candidate(CS, expr)); + + return Action::Continue(expr); + } + + private: + /// Extract type of the element from given collection type. + /// + /// \param collection The type of the collection container. + /// + /// \returns Null type, ErrorType or UnresolvedType on failure, + /// properly constructed type otherwise. + Type extractElementType(Type collection) { + auto &ctx = CS.getASTContext(); + if (!collection || collection->hasError()) + return collection; + + auto base = collection.getPointer(); + auto isInvalidType = [](Type type) -> bool { + return type.isNull() || type->hasUnresolvedType() || + type->hasError(); + }; + + // Array type. + if (auto array = dyn_cast(base)) { + auto elementType = array->getBaseType(); + // If base type is invalid let's return error type. + return elementType; + } + + // Map or Set or any other associated collection type. + if (auto boundGeneric = dyn_cast(base)) { + if (boundGeneric->hasUnresolvedType()) + return boundGeneric; + + llvm::SmallVector params; + for (auto &type : boundGeneric->getGenericArgs()) { + // One of the generic arguments in invalid or unresolved. + if (isInvalidType(type)) + return type; + + params.push_back(type); + } + + // If there is just one parameter, let's return it directly. + if (params.size() == 1) + return params[0].getType(); + + return TupleType::get(params, ctx); + } + + return Type(); + } + + bool isSuitableCollection(TypeRepr *collectionTypeRepr) { + // Only generic identifier, array or dictionary. + switch (collectionTypeRepr->getKind()) { + case TypeReprKind::UnqualifiedIdent: + return cast(collectionTypeRepr) + ->hasGenericArgList(); + + case TypeReprKind::Array: + case TypeReprKind::Dictionary: + return true; + + default: + break; + } + + return false; + } + + void visitCoerceExpr(CoerceExpr *coerceExpr) { + auto subExpr = coerceExpr->getSubExpr(); + // Coerce expression is valid only if it has sub-expression. + if (!subExpr) return; + + unsigned numOverloadSets = 0; + subExpr->forEachChildExpr([&](Expr *childExpr) -> Expr * { + if (isa(childExpr)) { + ++numOverloadSets; + return childExpr; + } + + if (auto nestedCoerceExpr = dyn_cast(childExpr)) { + visitCoerceExpr(nestedCoerceExpr); + // Don't walk inside of nested coercion expression directly, + // that is be done by recursive call to visitCoerceExpr. + return nullptr; + } + + // If sub-expression we are trying to coerce to type is a collection, + // let's allow collector discover it with assigned contextual type + // of coercion, which allows collections to be solved in parts. + if (auto collectionExpr = dyn_cast(childExpr)) { + auto *const typeRepr = coerceExpr->getCastTypeRepr(); + + if (typeRepr && isSuitableCollection(typeRepr)) { + const auto coercionType = TypeResolution::resolveContextualType( + typeRepr, CS.DC, std::nullopt, + // FIXME: Should we really be unconditionally complaining + // about unbound generics and placeholders here? For + // example: + // let foo: [Array] = [[0], [1], [2]] as [Array] + // let foo: [Array] = [[0], [1], [2]] as [Array<_>] + /*unboundTyOpener*/ nullptr, /*placeholderHandler*/ nullptr, + /*packElementOpener*/ nullptr); + + // Looks like coercion type is invalid, let's skip this sub-tree. + if (coercionType->hasError()) + return nullptr; + + // Visit collection expression inline. + visitCollectionExpr(collectionExpr, coercionType, + CTP_CoerceOperand); + } + } + + return childExpr; + }); + + // It's going to be inefficient to try and solve + // coercion in parts, so let's just make it a candidate directly, + // if it contains at least a single overload set. + + if (numOverloadSets > 0) + Candidates.push_back(Candidate(CS, coerceExpr)); + } + + void visitCollectionExpr(CollectionExpr *collectionExpr, + Type contextualType = Type(), + ContextualTypePurpose CTP = CTP_Unused) { + // If there is a contextual type set for this collection, + // let's propagate it to the candidate. + if (!contextualType.isNull()) { + auto elementType = extractElementType(contextualType); + // If we couldn't deduce element type for the collection, let's + // not attempt to solve it. + if (!elementType || + elementType->hasError() || + elementType->hasUnresolvedType()) + return; + + contextualType = elementType; + } + + for (auto element : collectionExpr->getElements()) { + unsigned numOverloads = 0; + element->walk(OverloadSetCounter(numOverloads)); + + // There are no overload sets in the element; skip it. + if (numOverloads == 0) + continue; + + // Record each of the collection elements, which passed + // number of overload sets rule, as a candidate for solving + // with contextual type of the collection. + Candidates.push_back(Candidate(CS, element, contextualType, CTP)); + } + } + }; + + ExprCollector collector(expr, *this, domains); + + // Collect all of the binary/unary and call sub-expressions + // so we can start solving them separately. + expr->walk(collector); + + llvm::SmallSetVector shrunkExprs; + for (auto &candidate : collector.Candidates) { + // If there are no results, let's forget everything we know about the + // system so far. This actually is ok, because some of the expressions + // might require manual salvaging. + if (candidate.solve(shrunkExprs)) { + // Let's restore all of the original OSR domains for this sub-expression, + // this means that we can still make forward progress with solving of the + // top sub-expressions. + candidate.getExpr()->forEachChildExpr([&](Expr *childExpr) -> Expr * { + if (auto OSR = dyn_cast(childExpr)) { + auto domain = domains.find(OSR); + if (domain == domains.end()) + return childExpr; + + OSR->setDecls(domain->getSecond()); + shrunkExprs.remove(OSR); + } + + return childExpr; + }); + } + } + + // Once "shrinking" is done let's re-allocate final version of + // the candidate list to the permanent arena, so it could + // survive even after primary constraint system is destroyed. + for (auto &OSR : shrunkExprs) { + auto choices = OSR->getDecls(); + auto decls = + getASTContext().AllocateUninitialized(choices.size()); + + std::uninitialized_copy(choices.begin(), choices.end(), decls.begin()); + OSR->setDecls(decls); + } +} + static bool debugConstraintSolverForTarget(ASTContext &C, SyntacticElementTarget target) { if (C.TypeCheckerOpts.DebugConstraintSolver) @@ -1153,6 +1703,8 @@ bool ConstraintSystem::solveForCodeCompletion( // Set up the expression type checker timer. startExpressionTimer(expr); + + shrink(expr); } if (isDebugMode()) { diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 016a1c90a162f..b1e574ed2ee99 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -450,6 +450,10 @@ TypeChecker::typeCheckTarget(SyntacticElementTarget &target, // diagnostics and is a hint for various performance optimizations. cs.setContextualInfo(expr, target.getExprContextualTypeInfo()); + // Try to shrink the system by reducing disjunction domains. This + // goes through every sub-expression and generate its own sub-system, to + // try to reduce the domains of those subexpressions. + cs.shrink(expr); target.setExpr(expr); } From 46e058ec915c6248a93294ed89aae492c1cb5a0d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 09:28:27 -0800 Subject: [PATCH 69/72] [TypeChecker] NFC: Remove resurrected use of `SolverShrinkUnsolvedThreshold` --- include/swift/Sema/ConstraintSystem.h | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 0138674030c80..1147dfa6bd717 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2521,9 +2521,8 @@ class ConstraintSystem { } } - unsigned threshold = - cs->getASTContext().TypeCheckerOpts.SolverShrinkUnsolvedThreshold; - return unsolvedDisjunctions >= threshold; + // The threshold used to be `TypeCheckerOpts.SolverShrinkUnsolvedThreshold` + return unsolvedDisjunctions >= 10; } }; From 4a61d8f9134db5ac7c6053bbd768bc57930f424e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 10:14:40 -0800 Subject: [PATCH 70/72] [TypeChecker] Bring back `SolverDisableShrink` --- include/swift/Basic/LangOptions.h | 3 +++ include/swift/Option/FrontendOptions.td | 4 ++++ lib/Frontend/CompilerInvocation.cpp | 3 +++ .../Sema/type_checker_perf/fast/array_concatenation.swift | 2 +- .../type_checker_perf/fast/property_vs_unapplied_func.swift | 2 +- .../Sema/type_checker_perf/slow/rdar26564101.swift | 2 +- 6 files changed, 13 insertions(+), 3 deletions(-) diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index f607b4562a0cd..2b5d994e2038f 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -906,6 +906,9 @@ namespace swift { /// is for testing purposes. std::vector DebugForbidTypecheckPrefixes; + /// Disable the shrink phase of the expression type checker. + bool SolverDisableShrink = false; + /// Enable experimental operator designated types feature. bool EnableOperatorDesignatedTypes = false; diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index 7a005271a3be1..5beee7143a796 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -832,6 +832,10 @@ def solver_scope_threshold_EQ : Joined<["-"], "solver-scope-threshold=">, def solver_trail_threshold_EQ : Joined<["-"], "solver-trail-threshold=">, HelpText<"Expression type checking trail change limit">; +def solver_disable_shrink : + Flag<["-"], "solver-disable-shrink">, + HelpText<"Disable the shrink phase of expression type checking">; + def solver_disable_splitter : Flag<["-"], "solver-disable-splitter">, HelpText<"Disable the component splitter phase of expression type checking">; diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index 01fdeb8f42a14..6b81e36b2f7c6 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -1859,6 +1859,9 @@ static bool ParseTypeCheckerArgs(TypeCheckerOptions &Opts, ArgList &Args, Opts.DebugForbidTypecheckPrefixes.push_back(A); } + if (Args.getLastArg(OPT_solver_disable_shrink)) + Opts.SolverDisableShrink = true; + if (Args.getLastArg(OPT_solver_disable_splitter)) Opts.SolverDisableSplitter = true; diff --git a/validation-test/Sema/type_checker_perf/fast/array_concatenation.swift b/validation-test/Sema/type_checker_perf/fast/array_concatenation.swift index 873889b4a20bd..1ce9d4293dc49 100644 --- a/validation-test/Sema/type_checker_perf/fast/array_concatenation.swift +++ b/validation-test/Sema/type_checker_perf/fast/array_concatenation.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift +// RUN: %target-typecheck-verify-swift -solver-disable-shrink // Self-contained test case protocol P1 {}; func f(_: T, _: T) -> T { fatalError() } diff --git a/validation-test/Sema/type_checker_perf/fast/property_vs_unapplied_func.swift b/validation-test/Sema/type_checker_perf/fast/property_vs_unapplied_func.swift index 0f44ea3c31032..9cd53902f42ad 100644 --- a/validation-test/Sema/type_checker_perf/fast/property_vs_unapplied_func.swift +++ b/validation-test/Sema/type_checker_perf/fast/property_vs_unapplied_func.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 +// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -solver-disable-shrink // REQUIRES: tools-release,no_asan struct Date { diff --git a/validation-test/Sema/type_checker_perf/slow/rdar26564101.swift b/validation-test/Sema/type_checker_perf/slow/rdar26564101.swift index e9e1d32c90f05..09ca79329d7a3 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar26564101.swift +++ b/validation-test/Sema/type_checker_perf/slow/rdar26564101.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 +// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -solver-disable-shrink // REQUIRES: tools-release,no_asan // UNSUPPORTED: swift_test_mode_optimize_none && OS=linux-gnu From 92147f891906242704f5a11c8afc9dff02366bef Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 11:29:33 -0800 Subject: [PATCH 71/72] [Tests] NFC: Mark tests affected by solver-perf revert as slow --- .../{fast => slow}/swift_package_index_1.swift | 4 ++-- .../{fast => slow}/swift_package_index_2.swift.gyb | 2 ++ .../{fast => slow}/swift_package_index_3.swift | 4 ++-- .../{fast => slow}/swift_package_index_4.swift | 4 ++-- .../{fast => slow}/swift_package_index_5.swift | 4 ++-- 5 files changed, 10 insertions(+), 8 deletions(-) rename validation-test/Sema/type_checker_perf/{fast => slow}/swift_package_index_1.swift (77%) rename validation-test/Sema/type_checker_perf/{fast => slow}/swift_package_index_2.swift.gyb (90%) rename validation-test/Sema/type_checker_perf/{fast => slow}/swift_package_index_3.swift (75%) rename validation-test/Sema/type_checker_perf/{fast => slow}/swift_package_index_4.swift (94%) rename validation-test/Sema/type_checker_perf/{fast => slow}/swift_package_index_5.swift (54%) diff --git a/validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift b/validation-test/Sema/type_checker_perf/slow/swift_package_index_1.swift similarity index 77% rename from validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift rename to validation-test/Sema/type_checker_perf/slow/swift_package_index_1.swift index f415f68e9994b..934a80472f9c9 100644 --- a/validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift +++ b/validation-test/Sema/type_checker_perf/slow/swift_package_index_1.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=1000 +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=1000 // REQUIRES: tools-release,no_asan public class Cookie { @@ -13,7 +13,7 @@ public class Cookie { private let fixedByteSize: Int32 = 56 var totalByteCount: Int32 { - return fixedByteSize + + return fixedByteSize + // expected-error {{the compiler is unable to type-check this expression in reasonable time}} (port != nil ? 2 : 0) + Int32(comment?.utf8.count ?? 0) + Int32(commentURL?.utf8.count ?? 0) + diff --git a/validation-test/Sema/type_checker_perf/fast/swift_package_index_2.swift.gyb b/validation-test/Sema/type_checker_perf/slow/swift_package_index_2.swift.gyb similarity index 90% rename from validation-test/Sema/type_checker_perf/fast/swift_package_index_2.swift.gyb rename to validation-test/Sema/type_checker_perf/slow/swift_package_index_2.swift.gyb index 258a967c38021..08f3063bd26d6 100644 --- a/validation-test/Sema/type_checker_perf/fast/swift_package_index_2.swift.gyb +++ b/validation-test/Sema/type_checker_perf/slow/swift_package_index_2.swift.gyb @@ -1,6 +1,8 @@ // RUN: %scale-test --begin 1 --end 10 --step 1 --select NumConstraintScopes %s // REQUIRES: tools-release,no_asan,asserts +// REQUIRES: rdar135382075 + var v: [String] = [] var vv: [String]? = nil diff --git a/validation-test/Sema/type_checker_perf/fast/swift_package_index_3.swift b/validation-test/Sema/type_checker_perf/slow/swift_package_index_3.swift similarity index 75% rename from validation-test/Sema/type_checker_perf/fast/swift_package_index_3.swift rename to validation-test/Sema/type_checker_perf/slow/swift_package_index_3.swift index 25afd5b754967..2327507262418 100644 --- a/validation-test/Sema/type_checker_perf/fast/swift_package_index_3.swift +++ b/validation-test/Sema/type_checker_perf/slow/swift_package_index_3.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=50000 +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=50000 // REQUIRES: tools-release,no_asan extension String { @@ -9,7 +9,7 @@ extension String { func getProperties( from ics: String ) -> [(name: String, value: String)] { - return ics + return ics // expected-error {{the compiler is unable to type-check this expression in reasonable time}} .replacingOccurrences(of: "\r\n ", with: "") .components(separatedBy: "\r\n") .map { $0.split(separator: ":", maxSplits: 1, omittingEmptySubsequences: true) } diff --git a/validation-test/Sema/type_checker_perf/fast/swift_package_index_4.swift b/validation-test/Sema/type_checker_perf/slow/swift_package_index_4.swift similarity index 94% rename from validation-test/Sema/type_checker_perf/fast/swift_package_index_4.swift rename to validation-test/Sema/type_checker_perf/slow/swift_package_index_4.swift index eaff8718abba3..8b05bedbea950 100644 --- a/validation-test/Sema/type_checker_perf/fast/swift_package_index_4.swift +++ b/validation-test/Sema/type_checker_perf/slow/swift_package_index_4.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=60000 +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=60000 // REQUIRES: tools-release,no_asan protocol ArgumentProtocol {} @@ -47,7 +47,7 @@ struct Options { static func evaluate(_ mode: CommandMode) -> Result { let defaultBuildDirectory = "" - return create + return create // expected-error {{the compiler is unable to type-check this expression in reasonable time}} <*> mode <| Option(key: "", defaultValue: nil, usage: "") <*> mode <| Option(key: "", defaultValue: nil, usage: "") <*> mode <| Option(key: "", defaultValue: FileManager.default.currentDirectoryPath, usage: "") diff --git a/validation-test/Sema/type_checker_perf/fast/swift_package_index_5.swift b/validation-test/Sema/type_checker_perf/slow/swift_package_index_5.swift similarity index 54% rename from validation-test/Sema/type_checker_perf/fast/swift_package_index_5.swift rename to validation-test/Sema/type_checker_perf/slow/swift_package_index_5.swift index e05dda77878eb..e20ea384f9a5f 100644 --- a/validation-test/Sema/type_checker_perf/fast/swift_package_index_5.swift +++ b/validation-test/Sema/type_checker_perf/slow/swift_package_index_5.swift @@ -1,8 +1,8 @@ -// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=1000 -swift-version 5 +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=1000 -swift-version 5 // REQUIRES: tools-release,no_asan func g(_: T) throws { - let _: T? = + let _: T? = //expected-error {{the compiler is unable to type-check this expression in reasonable time}} (try? f() as? T) ?? (try? f() as? T) ?? (try? f() as? T) ?? From 41fd4dec5f47a3ba9d243068db045a2161e0fb35 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 4 Feb 2025 11:30:11 -0800 Subject: [PATCH 72/72] [Tests] NFC: Adjust async tests that are affected by performance hacks --- test/Constraints/async.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/Constraints/async.swift b/test/Constraints/async.swift index 9985f169c0a22..52f7a44e46d55 100644 --- a/test/Constraints/async.swift +++ b/test/Constraints/async.swift @@ -212,9 +212,10 @@ func test_async_calls_in_async_context(v: Int) async { } // Only implicit `.init` should be accepted with a warning due type-checker previously picking an incorrect overload. - _ = Test(v) // expected-warning {{expression is 'async' but is not marked with 'await'; this is an error in the Swift 6 language mode}} expected-note {{call is 'async'}} + // FIXME: This should produce a warning once type-checker performance hacks are removed. + _ = Test(v) // Temporary okay _ = Test.init(v) // expected-error {{expression is 'async' but is not marked with 'await'}} expected-note {{call is 'async'}} Test.test(v) // expected-error {{expression is 'async' but is not marked with 'await'}} expected-note {{call is 'async'}} - Test(v).test(v) // expected-error {{expression is 'async' but is not marked with 'await'}} expected-note 2 {{call is 'async'}} + Test(v).test(v) // expected-error {{expression is 'async' but is not marked with 'await'}} expected-note {{call is 'async'}} }