From f650c0ac6ee8c580b1d23719ef545c21596afb17 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 22 Nov 2024 17:25:45 -0800 Subject: [PATCH 01/52] [CSBindings] Prevent `BindingSet::isViable` from dropping viable bindings (v2) The original attempt to do this was reverted by https://github.com/swiftlang/swift/pull/77653 The problem is that the fix was too broad, I narrowed it down to non-exact uses of stdlib collections that support upcasts. (cherry picked from commit b7e749307644bd2f5cc5daf243a0edfdf67741e0) --- lib/Sema/CSBindings.cpp | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 807cdec5f30fc..54ad8b4a191d6 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -31,7 +31,6 @@ using namespace swift; using namespace constraints; using namespace inference; - void ConstraintGraphNode::initBindingSet() { ASSERT(!hasBindingSet()); ASSERT(forRepresentativeVar()); @@ -39,6 +38,12 @@ 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); @@ -1348,7 +1353,31 @@ bool BindingSet::isViable(PotentialBinding &binding, bool isTransitive) { if (!existingNTD || NTD != existingNTD) continue; - // FIXME: What is going on here needs to be thoroughly re-evaluated. + // 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; + } + } // If new type has a type variable it shouldn't // be considered viable. From 69bd48e2ea37df607b8048462ffd33060e7e76a3 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 25 Nov 2024 20:30:04 -0800 Subject: [PATCH 02/52] [CSSimplify] CGFloat-Double: Rank narrowing correctly when result is injected into an optional If result of `CGFloat` -> `Double` conversion is injected into an optional it should be ranked based on depth just like when locator is fully simplified. For example: ```swift func test(v: CGFloat?) { _ = v ?? 2.0 / 3.0 } ``` In this expression division should be performed on `Double` and result narrowed down (based on the rule that narrowing conversion should always be delayed) but `Double` -> `CGFloat?` was given an incorrect score and instead of picking `?? (_: T?, _: T) -> T` overload, the solver would use `?? (_: T?, _: T?) -> T?`. (cherry picked from commit cb876cbd9e4123cc6ba9a989b0cd91e5f79dc36a) --- lib/Sema/CSSimplify.cpp | 16 +++++++++++++--- test/Constraints/bidirectional_conversions.swift | 5 ++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 786c7a8cad5f1..f511c4881c6d8 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -15184,9 +15184,19 @@ ConstraintSystem::simplifyRestrictedConstraintImpl( restriction == ConversionRestrictionKind::CGFloatToDouble ? 2 : 10; if (restriction == ConversionRestrictionKind::DoubleToCGFloat) { - if (auto *anchor = locator.trySimplifyToExpr()) { - if (auto depth = getExprDepth(anchor)) - impact = (*depth + 1) * impact; + 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; } } else if (locator.directlyAt() || locator.endsWith()) { diff --git a/test/Constraints/bidirectional_conversions.swift b/test/Constraints/bidirectional_conversions.swift index de5077c507a32..43620893f9693 100644 --- a/test/Constraints/bidirectional_conversions.swift +++ b/test/Constraints/bidirectional_conversions.swift @@ -6,7 +6,7 @@ import CoreGraphics ///////////// -struct G { // expected-note {{arguments to generic parameter 'T' ('CGFloat?' and 'CGFloat') are expected to be equal}} +struct G { var t: T } @@ -32,8 +32,7 @@ func foo4(x: (() -> ())?, y: @escaping @convention(block) () -> ()) -> G<() -> ( func foo5(x: CGFloat?, y: Double) -> G { let g = G(t: x ?? y) - // FIXME - return g // expected-error {{cannot convert return expression of type 'G' to return type 'G'}} + return g } func foo6(x: Double?, y: CGFloat) -> G { From 087c36c7c1b2ad706b053c0e448c732197258f2a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 16 Dec 2024 18:57:06 -0800 Subject: [PATCH 03/52] [ConstraintSystem] Fix `getEffectiveOverloadType` handling of `mutating` methods (cherry picked from commit c767f7aff7ec02d0d772455df56bb8cc8cdd8ff3) --- lib/Sema/TypeOfReference.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp index e8911387832ab..11e1f162a9f54 100644 --- a/lib/Sema/TypeOfReference.cpp +++ b/lib/Sema/TypeOfReference.cpp @@ -1900,11 +1900,15 @@ Type ConstraintSystem::getEffectiveOverloadType(ConstraintLocator *locator, type, var, useDC, GetClosureType{*this}, ClosureIsolatedByPreconcurrency{*this}); } else if (isa(decl) || isa(decl)) { - if (decl->isInstanceMember() && - (!overload.getBaseType() || - (!overload.getBaseType()->getAnyNominal() && - !overload.getBaseType()->is()))) - return Type(); + if (decl->isInstanceMember()) { + auto baseTy = overload.getBaseType(); + if (!baseTy) + return Type(); + + baseTy = baseTy->getRValueType(); + if (!baseTy->getAnyNominal() && !baseTy->is()) + return Type(); + } // Cope with 'Self' returns. if (!decl->getDeclContext()->getSelfProtocolDecl()) { From c18c626a0f3a3078919801d5a669e9f1012cb8a4 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 15 Jun 2025 23:38:36 -0700 Subject: [PATCH 04/52] [ConstraintSystem] Fix `getEffectiveOverloadType` to recognize when base is opaque result type Previously only nominal type and existential where allowed but that's outdated, it's possible to reference a member of an opaque result type as well, the concrete type in this case is going to be deduced by the solver. --- lib/Sema/TypeOfReference.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp index 11e1f162a9f54..313da3e045a03 100644 --- a/lib/Sema/TypeOfReference.cpp +++ b/lib/Sema/TypeOfReference.cpp @@ -1906,7 +1906,8 @@ Type ConstraintSystem::getEffectiveOverloadType(ConstraintLocator *locator, return Type(); baseTy = baseTy->getRValueType(); - if (!baseTy->getAnyNominal() && !baseTy->is()) + if (!baseTy->getAnyNominal() && !baseTy->is() && + !baseTy->is()) return Type(); } From 1b2ba7e059f77f5d9b69d96e34334538909b269d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 10 Feb 2023 14:54:06 -0800 Subject: [PATCH 05/52] [CSStep] Modify `selectDisjunction` to return favored choices directly This is currently unused because current mechanism set favored choices directly but it would be utilized by the disjunction optimizer. (cherry picked from commit e404ed722a99f880e3dbcf163283ba31da06c4e5) --- include/swift/Sema/ConstraintSystem.h | 18 +++++++++++++----- lib/Sema/CSSolver.cpp | 11 ++++++----- lib/Sema/CSStep.cpp | 9 +++++---- lib/Sema/CSStep.h | 10 +++++++++- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index c2550698d7cea..34649bb666ac1 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5397,8 +5397,9 @@ class ConstraintSystem { /// Pick a disjunction from the InactiveConstraints list. /// - /// \returns The selected disjunction. - Constraint *selectDisjunction(); + /// \returns The selected disjunction and a set of it's favored choices. + std::optional>> + selectDisjunction(); /// Pick a conjunction from the InactiveConstraints list. /// @@ -6332,7 +6333,8 @@ class DisjunctionChoiceProducer : public BindingProducer { public: using Element = DisjunctionChoice; - DisjunctionChoiceProducer(ConstraintSystem &cs, Constraint *disjunction) + DisjunctionChoiceProducer(ConstraintSystem &cs, Constraint *disjunction, + llvm::TinyPtrVector &favorites) : BindingProducer(cs, disjunction->shouldRememberChoice() ? disjunction->getLocator() : nullptr), @@ -6342,6 +6344,11 @@ 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); } @@ -6386,8 +6393,9 @@ 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/CSSolver.cpp b/lib/Sema/CSSolver.cpp index eb8581a130752..e35cf7990dbaf 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2381,15 +2381,16 @@ void DisjunctionChoiceProducer::partitionDisjunction( assert(Ordering.size() == Choices.size()); } -Constraint *ConstraintSystem::selectDisjunction() { +std::optional>> +ConstraintSystem::selectDisjunction() { SmallVector disjunctions; collectDisjunctions(disjunctions); if (disjunctions.empty()) - return nullptr; + return std::nullopt; if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) - return disjunction; + return std::make_pair(disjunction, llvm::TinyPtrVector()); // Pick the disjunction with the smallest number of favored, then active choices. auto cs = this; @@ -2431,9 +2432,9 @@ Constraint *ConstraintSystem::selectDisjunction() { }); if (minDisjunction != disjunctions.end()) - return *minDisjunction; + return std::make_pair(*minDisjunction, llvm::TinyPtrVector()); - return nullptr; + return std::nullopt; } Constraint *ConstraintSystem::selectConjunction() { diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index eb8dd95741603..bf6be9e02ff47 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -272,7 +272,7 @@ StepResult ComponentStep::take(bool prevFailed) { } }); - auto *disjunction = CS.selectDisjunction(); + auto disjunction = CS.selectDisjunction(); auto *conjunction = CS.selectConjunction(); if (CS.isDebugMode()) { @@ -315,7 +315,8 @@ 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)) + if (disjunction && + !bestBindings->favoredOverDisjunction(disjunction->first)) return StepKind::Disjunction; if (conjunction && !bestBindings->favoredOverConjunction(conjunction)) @@ -338,9 +339,9 @@ StepResult ComponentStep::take(bool prevFailed) { return suspend( std::make_unique(*bestBindings, Solutions)); case StepKind::Disjunction: { - CS.retireConstraint(disjunction); + CS.retireConstraint(disjunction->first); 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 bfd103f5edccd..906fda9895050 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -613,9 +613,17 @@ 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}, solutions), Disjunction(disjunction) { + : BindingStep(cs, {cs, disjunction, favoredChoices}, solutions), + Disjunction(disjunction) { assert(Disjunction->getKind() == ConstraintKind::Disjunction); pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; From 0871358ec4f4540ee2a6083aed0d221d7c644ee7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 17 May 2025 00:40:24 -0700 Subject: [PATCH 06/52] [TypeChecker] Update `async` initializer warning downgrade to check the call instead of callee `calleeFn` now returns the underlying declaration reference looking through `ConstructorRefCallExpr`, which means the downgrade logic needs to check whether the call is using initializer reference before making a decision. --- lib/Sema/TypeCheckEffects.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Sema/TypeCheckEffects.cpp b/lib/Sema/TypeCheckEffects.cpp index a8425dcadf08d..4b6727a90916d 100644 --- a/lib/Sema/TypeCheckEffects.cpp +++ b/lib/Sema/TypeCheckEffects.cpp @@ -1672,7 +1672,7 @@ class ApplyClassifier { /// Check to see if the given function application throws, is async, or /// involves unsafe behavior. Classification classifyApply( - Expr *call, + ApplyExpr *call, const AbstractFunction &fnRef, Expr *calleeFn, const AnyFunctionType *fnType, @@ -1820,7 +1820,7 @@ class ApplyClassifier { // to fix their code. if (kind == EffectKind::Async && fnRef.getKind() == AbstractFunction::Function) { - if (auto *ctor = dyn_cast(calleeFn)) { + if (auto *ctor = dyn_cast(call->getFn())) { if (ctor->getFn()->isImplicit() && args->isUnlabeledUnary()) result.setDowngradeToWarning(true); } From c04068db1c59e333a663d4c383afb835ee2e416d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 24 Jun 2025 17:07:18 -0700 Subject: [PATCH 07/52] [ConstraintSystem] Initial implementation of disjunction optimizer This brings back CSOptimizer from https://github.com/swiftlang/swift/pull/63585 --- include/swift/Sema/ConstraintSystem.h | 7 + lib/Sema/CMakeLists.txt | 1 + lib/Sema/CSOptimizer.cpp | 1329 +++++++++++++++++++++++++ lib/Sema/CSSolver.cpp | 11 +- lib/Sema/TypeCheckConstraints.cpp | 4 + 5 files changed, 1346 insertions(+), 6 deletions(-) create mode 100644 lib/Sema/CSOptimizer.cpp diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 34649bb666ac1..2717c96559456 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -491,6 +491,10 @@ 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. /// @@ -5401,6 +5405,9 @@ class ConstraintSystem { std::optional>> selectDisjunction(); + /// The old method that is only used when performance hacks are enabled. + Constraint *selectDisjunctionWithHacks(); + /// Pick a conjunction from the InactiveConstraints list. /// /// \returns The selected conjunction. diff --git a/lib/Sema/CMakeLists.txt b/lib/Sema/CMakeLists.txt index 84ae48ce0f610..96d8839268e29 100644 --- a/lib/Sema/CMakeLists.txt +++ b/lib/Sema/CMakeLists.txt @@ -14,6 +14,7 @@ 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 new file mode 100644 index 0000000000000..8f40c07eaf658 --- /dev/null +++ b/lib/Sema/CSOptimizer.cpp @@ -0,0 +1,1329 @@ +//===--- CSOptimizer.cpp - Constraint Optimizer ---------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2025 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 "TypeChecker.h" +#include "swift/AST/ConformanceLookup.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 + +using namespace swift; +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`. + +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 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; + + auto choices = disjunction->getNestedConstraints(); + auto *decl = getOverloadChoiceDecl(choices.front()); + + auto name = decl->getBaseIdentifier(); + if (name.isArithmeticOperator() || name.isStandardComparisonOperator() || + name.isBitwiseOperator()) { + return true; + } + + // Operators like &<<, &>>, &+, .== etc. + if (llvm::any_of(choices, [](Constraint *choice) { + return isSIMDOperator(getOverloadChoiceDecl(choice)); + })) { + return true; + } + + 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(); +} + +static bool isArithmeticOperator(ValueDecl *decl) { + return decl->isOperator() && decl->getBaseIdentifier().isArithmeticOperator(); +} + +static bool isSupportedDisjunction(Constraint *disjunction) { + auto choices = disjunction->getNestedConstraints(); + + 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) { + if (choice->isDisabled()) + return true; + + 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; + + return decl->getInterfaceType()->is(); + } + + return false; + }); +} + +NullablePtr getApplicableFnConstraint(ConstraintGraph &CG, + Constraint *disjunction) { + auto *boundVar = disjunction->getNestedConstraints()[0] + ->getFirstType() + ->getAs(); + if (!boundVar) + return nullptr; + + auto constraints = + CG.gatherNearbyConstraints(boundVar, [](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; + + // 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->getDeclContext()) { + if (!useDC->isDeclImported(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()); + } +} + +static OverloadedDeclRefExpr *isOverloadedDeclRef(Constraint *disjunction) { + assert(disjunction->getKind() == ConstraintKind::Disjunction); + + auto *locator = disjunction->getLocator(); + if (locator->getPath().empty()) + return getAsExpr(locator->getAnchor()); + return nullptr; +} + +static unsigned numOverloadChoicesMatchingOnArity(OverloadedDeclRefExpr *ODRE, + ArgumentList *arguments) { + return llvm::count_if(ODRE->getDecls(), [&arguments](auto *choice) { + if (auto *paramList = choice->getParameterList()) + 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. +static void findFavoredChoicesBasedOnArity( + ConstraintSystem &cs, Constraint *disjunction, ArgumentList *argumentList, + llvm::function_ref favoredChoice) { + auto *ODRE = isOverloadedDeclRef(disjunction); + if (!ODRE) + return; + + if (numOverloadChoicesMatchingOnArity(ODRE, argumentList) > 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(*decl->getParameterList(), [](auto *param) { + return !param->isDefaultArgument(); + }) == argumentList->size()) + favored.push_back(choice); + }); + + if (hasVariadicGenerics) + return; + + for (auto *choice : favored) + favoredChoice(choice); +} + +} // 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 &result) { + 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); + }; + + 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); + + 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 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: { + 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}); + continue; + } + + default: + break; + } + } + } + + continue; + } + + auto argFuncType = + applicableFn.get()->getFirstType()->getAs(); + + auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); + if (!argumentList) + return; + + for (const auto &argument : *argumentList) { + if (auto *expr = argument.getExpr()) { + // Directly `<#...#>` or has one inside. + if (isa(expr) || + cs.containsIDEInspectionTarget(expr)) + return; + } + } + + // This maintains an "old hack" behavior where overloads + // 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}); + continue; + } + } + + if (!isSupportedDisjunction(disjunction)) + continue; + + SmallVector argsWithLabels; + { + argsWithLabels.append(argFuncType->getParams().begin(), + argFuncType->getParams().end()); + 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()); + + 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 types; + if (auto *typeVar = argType->getAs()) { + auto bindingSet = cs.getBindingsFor(typeVar); + + for (const auto &binding : bindingSet.Bindings) { + types.push_back({binding.BindingType}); + } + + for (const auto &literal : bindingSet.Literals) { + if (literal.second.hasDefaultType()) { + // Add primary default type + types.push_back( + {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, + /*fromInitializerCall=*/true}); + } + } + } + } else { + types.push_back({argType, /*fromLiteral=*/false}); + } + + argumentCandidates[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); + } + + // 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 = argumentCandidates[argIdx]; + return llvm::any_of(candidates, [&](const auto &candidate) { + return !candidate.fromLiteral; + }); + }); + + // Match arguments to the given overload choice. + auto matchArguments = [&](OverloadChoice choice, FunctionType *overloadType) + -> std::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, std::nullopt); + }; + + // 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->getSuperclassDecl()) { + if (!isSubclassOf(candidateType, + superclass->getDeclaredInterfaceType())) + return false; + } + + auto result = TypeChecker::containsProtocol(candidateType, P, + /*allowMissing=*/false); + return result.first || result.second; + }); + } + + auto *subclassDecl = candidateType->getClassOrBoundGenericClass(); + auto *superclassDecl = superclassType->getClassOrBoundGenericClass(); + + if (!(subclassDecl && superclassDecl)) + return false; + + return superclassDecl->isSuperclassOf(subclassDecl); + }; + + enum class MatchFlag { + OnParam = 0x01, + Literal = 0x02, + ExactOnly = 0x04, + DisableCGFloatDoubleConversion = 0x08, + }; + + 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 + // + // 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, ValueDecl *, Type, + Type, MatchOptions)> + scoreCandidateMatch = + [&](GenericSignature genericSig, ValueDecl *choice, + Type candidateType, Type paramType, + MatchOptions options) -> std::optional { + auto areEqual = [&](Type a, Type b) { + 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 (candidateType->isCGFloat() && paramType->isDouble()) { + return options.contains(MatchFlag::Literal) ? 0.2 : 0.9; + } + } + + // 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->isArray() ? 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; + + // Exact match between candidate and parameter types. + if (areEqual(candidateType, paramType)) { + return options.contains(MatchFlag::Literal) ? 0.3 : 1; + } + + 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 paramType->isDouble() ? 0.2 : 0.3; + + if (candidateType->isDouble() && + TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) + return 0.3; + } + + 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()) { + auto score = scoreCandidateMatch(genericSig, choice, 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; + } + + // Optionality mismatch. + return 0; + } + } + + // Candidate could be converted to a superclass. + if (isSubclassOf(candidateType, paramType)) + return 1; + + // Possible Array -> Unsafe*Pointer conversion. + if (options.contains(MatchFlag::OnParam)) { + if (candidateType->isArray() && + 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. + if (genericSig && paramType->isTypeParameter()) { + // Light-weight check if cases where `checkRequirements` is not + // applicable. + auto checkProtocolRequirementsOnly = [&]() -> double { + 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; + } + + 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 (requirement.getKind() != RequirementKind::Layout) + recordReferencesGenericParams(requirement.getSecondType()); + + if (llvm::any_of(toExamine, [&](GenericTypeParamType *GP) { + return paramType->isEqual(GP); + })) { + 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 { + 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 choice->isOperator() ? 0.9 : 1.0; + + return 0; + } + + // 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; + + // 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; + } + + 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(); + } + } + + auto matchings = + matchArguments(choice->getOverloadChoice(), overloadType); + 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; + + if (preserveFavoringOfUnlabeledUnaryArgument) { + // Old behavior completely disregarded the fact that some of + // the parameters could be defaulted. + if (overloadType->getNumParams() != 1) + return; + + 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(); + paramIdx != n; ++paramIdx) { + const auto ¶m = overloadType->getParams()[paramIdx]; + + auto argIndices = matchings->parameterBindings[paramIdx]; + switch (argIndices.size()) { + case 0: + // Current parameter is defaulted, mark and continue. + ++numDefaulted; + continue; + + case 1: + // One-to-one match between argument and parameter. + break; + + default: + // Cannot deal with multiple possible matchings at the moment. + return; + } + + auto argIdx = argIndices.front(); + + // Looks like there is nothing know about the argument. + if (argumentCandidates[argIdx].empty()) + continue; + + 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; + + // 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(argumentCandidates[argIdx].size()); + + for (unsigned candidateIdx : + indices(argumentCandidates[argIdx])) { + // If one of the candidates matched exactly there is no reason + // to continue checking. + if (bestCandidateScore == 1) + break; + + auto candidate = argumentCandidates[argIdx][candidateIdx]; + + // `inout` parameter accepts only l-value argument. + if (paramFlags.isInOut() && !candidate.type->is()) { + mismatches.set(candidateIdx); + continue; + } + + MatchOptions options(MatchFlag::OnParam); + if (candidate.fromLiteral) + options |= MatchFlag::Literal; + 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; + + // 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); + + if (!candidateScore) + continue; + + if (candidateScore > 0) { + bestCandidateScore = + std::max(bestCandidateScore, candidateScore.value()); + continue; + } + + // 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()) + 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; + } + + // 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); + + // 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 && decl->isOperator()) + score = 0.1; + + // 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 (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, decl, + overloadType->getResult(), + candidateResultTy, + /*options=*/{}) > 0; + })) { + score += 1.0; + } + } + + 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) && + overloadType->getNumParams() == 2) { + auto resultTy = overloadType->getResult(); + if (!resultTy->hasTypeParameter() && + llvm::all_of(overloadType->getParams(), + [&resultTy](const auto ¶m) { + return param.getPlainType()->isEqual(resultTy); + })) + score += 0.1; + } + + 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"; + } + + bestOverallScore = std::max(bestOverallScore, bestScore); + + DisjunctionInfo info(/*score=*/bestScore); + + for (const auto &choice : favoredChoices) { + if (choice.second == bestScore) + info.FavoredChoices.push_back(choice.first); + } + + recordResult(disjunction, std::move(info)); + } + + 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 (auto *disjunction : disjunctions) { + auto &entry = result[disjunction]; + getLogger(/*extraIndent=*/4) + << "[Disjunction '" + << disjunction->getNestedConstraints()[0]->getFirstType()->getString( + PO) + << "' with score = " << entry.Score.value_or(0) << '\n'; + + for (const auto *choice : entry.FavoredChoices) { + auto &log = getLogger(/*extraIndent=*/6); + + log << "- "; + choice->print(log, &cs.getASTContext().SourceMgr); + log << '\n'; + } + + getLogger(/*extraIdent=*/4) << "]\n"; + } + + getLogger() << ")\n"; + } +} + +// 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().gatherNearbyConstraints( + typeVar, [](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; +} + +/// 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() { + if (performanceHacksEnabled()) { + if (auto *disjunction = selectDisjunctionWithHacks()) + return std::make_pair(disjunction, llvm::TinyPtrVector()); + return std::nullopt; + } + + SmallVector disjunctions; + + collectDisjunctions(disjunctions); + if (disjunctions.empty()) + return std::nullopt; + + if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) + return std::make_pair(disjunction, llvm::TinyPtrVector()); + + 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( + disjunctions.begin(), disjunctions.end(), + [&](Constraint *first, Constraint *second) -> bool { + unsigned firstActive = first->countActiveNestedConstraints(); + unsigned secondActive = second->countActiveNestedConstraints(); + + auto &[firstScore, firstFavoredChoices] = favorings[first]; + auto &[secondScore, secondFavoredChoices] = favorings[second]; + + // Rank based on scores only if both disjunctions are supported. + if (firstScore && secondScore) { + // 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 = firstFavoredChoices.size(); + unsigned numSecondFavored = secondFavoredChoices.size(); + + if (numFirstFavored == numSecondFavored) { + if (firstActive != secondActive) + return firstActive < secondActive; + } + + numFirstFavored = numFirstFavored ? numFirstFavored : firstActive; + numSecondFavored = numSecondFavored ? numSecondFavored : secondActive; + + return numFirstFavored < numSecondFavored; + }); + + if (bestDisjunction != disjunctions.end()) + return std::make_pair(*bestDisjunction, + favorings[*bestDisjunction].FavoredChoices); + + return std::nullopt; +} diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index e35cf7990dbaf..51d78cb50aafa 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2381,16 +2381,15 @@ void DisjunctionChoiceProducer::partitionDisjunction( assert(Ordering.size() == Choices.size()); } -std::optional>> -ConstraintSystem::selectDisjunction() { +Constraint *ConstraintSystem::selectDisjunctionWithHacks() { SmallVector disjunctions; collectDisjunctions(disjunctions); if (disjunctions.empty()) - return std::nullopt; + return nullptr; if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) - return std::make_pair(disjunction, llvm::TinyPtrVector()); + return disjunction; // Pick the disjunction with the smallest number of favored, then active choices. auto cs = this; @@ -2432,9 +2431,9 @@ ConstraintSystem::selectDisjunction() { }); if (minDisjunction != disjunctions.end()) - return std::make_pair(*minDisjunction, llvm::TinyPtrVector()); + return *minDisjunction; - return std::nullopt; + return nullptr; } Constraint *ConstraintSystem::selectConjunction() { diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 8b6ae459a75d4..2167e0bd75458 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -204,6 +204,10 @@ 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); From 2957da359131ad48ba8e8dd271b3cbb2d13579de Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 25 Jun 2025 10:38:27 -0700 Subject: [PATCH 08/52] [ConstraintSystem] Disable performance hacks by default This also changes the flag to `-enable-constraint-solver-performance-hacks`. --- include/swift/Basic/LangOptions.h | 4 +- include/swift/Option/FrontendOptions.td | 4 +- include/swift/Sema/ConstraintSystem.h | 8 +- lib/Frontend/CompilerInvocation.cpp | 4 +- test/Constraints/async.swift | 5 +- test/Constraints/casts_swift6.swift | 6 +- test/Constraints/common_type.swift | 2 + .../implicit_double_cgfloat_conversion.swift | 24 ++ .../old_hack_related_ambiguities.swift | 249 ++++++++++++++++++ test/Constraints/operator.swift | 63 +++++ test/IDE/complete_operators.swift | 2 +- test/expr/expressions.swift | 8 +- .../fast/array_concatenation.swift | 2 +- ...l_with_operators_and_double_literals.swift | 31 +++ ...rals_and_operators_cast_to_float.swift.gyb | 10 + .../fast/cgfloat_with_unary_operators.swift | 19 ++ .../complex_swiftui_padding_conditions.swift | 17 ++ ...ns_and_operators_with_iou_base_types.swift | 35 +++ .../{slow => fast}/nil_coalescing.swift.gyb | 2 +- ...or_chain_with_hetergeneous_arguments.swift | 1 - .../fast/rdar133340307.swift | 134 ++++++++++ .../{slow => fast}/rdar17170728.swift | 1 - .../{slow => fast}/rdar22770433.swift | 1 - .../{slow => fast}/rdar23682605.swift | 1 - .../{slow => fast}/rdar31742586.swift | 1 - .../{slow => fast}/rdar35213699.swift | 1 - .../rdar46713933_literal_arg.swift | 1 - .../{slow => fast}/rdar91310777.swift | 1 - .../simd_scalar_multiple.swift.gyb | 2 +- .../swift_package_index_1.swift | 2 +- .../swift_package_index_3.swift | 2 +- .../swift_package_index_4.swift | 4 +- .../swift_package_index_5.swift | 2 +- .../borderline_flat_map_operator_mix.swift | 7 +- .../collections_chained_with_plus.swift.gyb | 18 ++ .../type_checker_perf/slow/rdar26564101.swift | 2 +- .../35129b57d7eb80f4.swift | 2 +- .../eb61865acc77c50.swift | 2 +- .../stdlib/FixedPointDiagnostics.swift.gyb | 4 +- 39 files changed, 640 insertions(+), 44 deletions(-) create mode 100644 test/Constraints/old_hack_related_ambiguities.swift create mode 100644 validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift create mode 100644 validation-test/Sema/type_checker_perf/fast/array_of_literals_and_operators_cast_to_float.swift.gyb create mode 100644 validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift create mode 100644 validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift create mode 100644 validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift rename validation-test/Sema/type_checker_perf/{slow => fast}/nil_coalescing.swift.gyb (58%) rename validation-test/Sema/type_checker_perf/{slow => fast}/operator_chain_with_hetergeneous_arguments.swift (90%) create mode 100644 validation-test/Sema/type_checker_perf/fast/rdar133340307.swift rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar17170728.swift (74%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar22770433.swift (74%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar23682605.swift (91%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar31742586.swift (67%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar35213699.swift (67%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar46713933_literal_arg.swift (85%) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar91310777.swift (66%) rename validation-test/Sema/type_checker_perf/{slow => fast}/simd_scalar_multiple.swift.gyb (70%) rename validation-test/Sema/type_checker_perf/{slow => fast}/swift_package_index_1.swift (85%) rename validation-test/Sema/type_checker_perf/{slow => fast}/swift_package_index_3.swift (84%) rename validation-test/Sema/type_checker_perf/{slow => fast}/swift_package_index_4.swift (94%) rename validation-test/Sema/type_checker_perf/{slow => fast}/swift_package_index_5.swift (74%) rename validation-test/Sema/type_checker_perf/{fast => slow}/borderline_flat_map_operator_mix.swift (50%) create mode 100644 validation-test/Sema/type_checker_perf/slow/collections_chained_with_plus.swift.gyb rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/35129b57d7eb80f4.swift (70%) rename validation-test/{compiler_crashers_2 => compiler_crashers_2_fixed}/eb61865acc77c50.swift (71%) diff --git a/include/swift/Basic/LangOptions.h b/include/swift/Basic/LangOptions.h index d6a9e36ecbf8b..f1896d42383f7 100644 --- a/include/swift/Basic/LangOptions.h +++ b/include/swift/Basic/LangOptions.h @@ -986,8 +986,8 @@ namespace swift { /// Enable experimental operator designated types feature. bool EnableOperatorDesignatedTypes = false; - /// Disable constraint system performance hacks. - bool DisableConstraintSolverPerformanceHacks = false; + /// Enable old constraint system performance hacks. + bool EnableConstraintSolverPerformanceHacks = false; /// See \ref FrontendOptions.PrintFullConvention bool PrintFullConvention = false; diff --git a/include/swift/Option/FrontendOptions.td b/include/swift/Option/FrontendOptions.td index 3a353ddbca74a..1ca56c6cff83e 100644 --- a/include/swift/Option/FrontendOptions.td +++ b/include/swift/Option/FrontendOptions.td @@ -862,8 +862,8 @@ def solver_trail_threshold_EQ : Joined<["-"], "solver-trail-threshold=">, def solver_disable_splitter : Flag<["-"], "solver-disable-splitter">, HelpText<"Disable the component splitter phase of expression type checking">; -def disable_constraint_solver_performance_hacks : Flag<["-"], "disable-constraint-solver-performance-hacks">, - HelpText<"Disable all the hacks in the constraint solver">; +def enable_constraint_solver_performance_hacks : Flag<["-"], "enable-constraint-solver-performance-hacks">, + HelpText<"Enable all the old hacks in the constraint solver">; def enable_operator_designated_types : Flag<["-"], "enable-operator-designated-types">, diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 2717c96559456..7ded9f8090d06 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -3595,11 +3595,11 @@ class ConstraintSystem { return Options.contains(ConstraintSystemFlags::ForCodeCompletion); } - /// Check whether type-checker performance hacks has been explicitly - /// disabled by a flag. + /// Check whether old type-checker performance hacks has been explicitly + /// enabled by a flag. bool performanceHacksEnabled() const { - return !getASTContext() - .TypeCheckerOpts.DisableConstraintSolverPerformanceHacks; + return getASTContext() + .TypeCheckerOpts.EnableConstraintSolverPerformanceHacks; } /// Log and record the application of the fix. Return true iff any diff --git a/lib/Frontend/CompilerInvocation.cpp b/lib/Frontend/CompilerInvocation.cpp index afbf32fdbb1a4..b658685952fb9 100644 --- a/lib/Frontend/CompilerInvocation.cpp +++ b/lib/Frontend/CompilerInvocation.cpp @@ -2009,8 +2009,8 @@ static bool ParseTypeCheckerArgs(TypeCheckerOptions &Opts, ArgList &Args, SWIFT_ONONE_SUPPORT); } - Opts.DisableConstraintSolverPerformanceHacks |= - Args.hasArg(OPT_disable_constraint_solver_performance_hacks); + Opts.EnableConstraintSolverPerformanceHacks |= + Args.hasArg(OPT_enable_constraint_solver_performance_hacks); Opts.EnableOperatorDesignatedTypes |= Args.hasArg(OPT_enable_operator_designated_types); diff --git a/test/Constraints/async.swift b/test/Constraints/async.swift index 52f7a44e46d55..9985f169c0a22 100644 --- a/test/Constraints/async.swift +++ b/test/Constraints/async.swift @@ -212,10 +212,9 @@ 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. - // FIXME: This should produce a warning once type-checker performance hacks are removed. - _ = Test(v) // Temporary okay + _ = 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'}} _ = 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 {{call is 'async'}} + Test(v).test(v) // expected-error {{expression is 'async' but is not marked with 'await'}} expected-note 2 {{call is 'async'}} } diff --git a/test/Constraints/casts_swift6.swift b/test/Constraints/casts_swift6.swift index 5161a493e9dbd..17cbd89100741 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' ('[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]')}} + _ = (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]')}} _ = (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/test/Constraints/common_type.swift b/test/Constraints/common_type.swift index 32f3bf3c8bb35..bad4415cf63f1 100644 --- a/test/Constraints/common_type.swift +++ b/test/Constraints/common_type.swift @@ -1,6 +1,8 @@ // 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/implicit_double_cgfloat_conversion.swift b/test/Constraints/implicit_double_cgfloat_conversion.swift index 014b112f8de10..f902a5db4f84e 100644 --- a/test/Constraints/implicit_double_cgfloat_conversion.swift +++ b/test/Constraints/implicit_double_cgfloat_conversion.swift @@ -361,3 +361,27 @@ func test_ternary_and_nil_coalescing() { test(v ?? 0.0) // Ok } } + +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 + } +} + +func test_cgfloat_operator_is_attempted_with_literal_arguments(v: CGFloat?) { + let ratio = v ?? (2.0 / 16.0) + let _: CGFloat = ratio // Ok +} diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift new file mode 100644 index 0000000000000..b935fc974c43e --- /dev/null +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -0,0 +1,249 @@ +// 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 + } +} + +// error: initializer for conditional binding must have Optional type, not 'S' +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? { + nil + } + + func f(_ x: String) -> S { + fatalError() + } + + func g(_ x: String) -> Int? { + guard let y = f(x) else { + return nil + } + return y.n + } +} + +// 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)) + } +} + +// 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 + } +} + +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 +} diff --git a/test/Constraints/operator.swift b/test/Constraints/operator.swift index a54ff50a4770d..d199f88c65357 100644 --- a/test/Constraints/operator.swift +++ b/test/Constraints/operator.swift @@ -330,3 +330,66 @@ 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 + } +} + +// 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 + } +} diff --git a/test/IDE/complete_operators.swift b/test/IDE/complete_operators.swift index 2772678e1dbc2..f7e0392a2b5a6 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: ***[#Double#] +// POSTFIX_7: Decl[PostfixOperatorFunction]/CurrModule/TypeRelation[Convertible]: ***[#Double#] func testPostfix8(x: S) { x#^POSTFIX_8^# diff --git a/test/expr/expressions.swift b/test/expr/expressions.swift index b30d808a56264..d84994169fbb4 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 'any (~Copyable & ~Escapable).Type' to 'nil' always returns false}} -_ = nil == Int.self // expected-warning {{comparing non-optional value of type 'any (~Copyable & ~Escapable).Type' to 'nil' always returns false}} -_ = Int.self != nil // expected-warning {{comparing non-optional value of type 'any (~Copyable & ~Escapable).Type' to 'nil' always returns true}} -_ = nil != Int.self // expected-warning {{comparing non-optional value of type 'any (~Copyable & ~Escapable).Type' to 'nil' always returns true}} +_ = 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 == .none // expected-warning {{comparing non-optional value of type 'any (~Copyable & ~Escapable).Type' to 'Optional.none' always returns false}} _ = .none == Int.self // expected-warning {{comparing non-optional value of type 'any (~Copyable & ~Escapable).Type' to 'Optional.none' always returns false}} 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 9ba40cc773b83..873889b4a20bd 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 -disable-constraint-solver-performance-hacks +// RUN: %target-typecheck-verify-swift // Self-contained test case protocol P1 {}; func f(_: T, _: T) -> T { fatalError() } 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 new file mode 100644 index 0000000000000..49ce8b4d6abac --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift @@ -0,0 +1,31 @@ +// 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) +] 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 new file mode 100644 index 0000000000000..2cd48795d50c5 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/array_of_literals_and_operators_cast_to_float.swift.gyb @@ -0,0 +1,10 @@ +// 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] 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 new file mode 100644 index 0000000000000..ebf7490e9cfdf --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift @@ -0,0 +1,19 @@ +// 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)) +} 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 new file mode 100644 index 0000000000000..df038a16ff4cc --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift @@ -0,0 +1,17 @@ +// 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) + } +} 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 new file mode 100644 index 0000000000000..d00b81af40f38 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift @@ -0,0 +1,35 @@ +// 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) +} diff --git a/validation-test/Sema/type_checker_perf/slow/nil_coalescing.swift.gyb b/validation-test/Sema/type_checker_perf/fast/nil_coalescing.swift.gyb similarity index 58% rename from validation-test/Sema/type_checker_perf/slow/nil_coalescing.swift.gyb rename to validation-test/Sema/type_checker_perf/fast/nil_coalescing.swift.gyb index 2ef5c8f16a3d7..af8639d9bc159 100644 --- a/validation-test/Sema/type_checker_perf/slow/nil_coalescing.swift.gyb +++ b/validation-test/Sema/type_checker_perf/fast/nil_coalescing.swift.gyb @@ -1,4 +1,4 @@ -// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select NumLeafScopes %s +// RUN: %scale-test --begin 1 --end 10 --step 1 --select NumLeafScopes %s // REQUIRES: asserts,no_asan func t(_ x: Int?) -> Int { diff --git a/validation-test/Sema/type_checker_perf/slow/operator_chain_with_hetergeneous_arguments.swift b/validation-test/Sema/type_checker_perf/fast/operator_chain_with_hetergeneous_arguments.swift similarity index 90% rename from validation-test/Sema/type_checker_perf/slow/operator_chain_with_hetergeneous_arguments.swift rename to validation-test/Sema/type_checker_perf/fast/operator_chain_with_hetergeneous_arguments.swift index 5d6c8c4440ece..787509fb565b2 100644 --- a/validation-test/Sema/type_checker_perf/slow/operator_chain_with_hetergeneous_arguments.swift +++ b/validation-test/Sema/type_checker_perf/fast/operator_chain_with_hetergeneous_arguments.swift @@ -5,5 +5,4 @@ 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}} } diff --git a/validation-test/Sema/type_checker_perf/fast/rdar133340307.swift b/validation-test/Sema/type_checker_perf/fast/rdar133340307.swift new file mode 100644 index 0000000000000..9740b4d15f06c --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/rdar133340307.swift @@ -0,0 +1,134 @@ +// 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 + } +} diff --git a/validation-test/Sema/type_checker_perf/slow/rdar17170728.swift b/validation-test/Sema/type_checker_perf/fast/rdar17170728.swift similarity index 74% rename from validation-test/Sema/type_checker_perf/slow/rdar17170728.swift rename to validation-test/Sema/type_checker_perf/fast/rdar17170728.swift index 62c2bb2ea367b..1e64e4194e4b6 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar17170728.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar17170728.swift @@ -5,7 +5,6 @@ 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)) } diff --git a/validation-test/Sema/type_checker_perf/slow/rdar22770433.swift b/validation-test/Sema/type_checker_perf/fast/rdar22770433.swift similarity index 74% rename from validation-test/Sema/type_checker_perf/slow/rdar22770433.swift rename to validation-test/Sema/type_checker_perf/fast/rdar22770433.swift index f063e685689de..0c51c9346d7ac 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar22770433.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar22770433.swift @@ -2,7 +2,6 @@ // 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/slow/rdar23682605.swift b/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift similarity index 91% rename from validation-test/Sema/type_checker_perf/slow/rdar23682605.swift rename to validation-test/Sema/type_checker_perf/fast/rdar23682605.swift index b7bc757fe1989..4be53b4d2ef83 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar23682605.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar23682605.swift @@ -14,7 +14,6 @@ func memoize( body: @escaping ((T)->U, T)->U ) -> (T)->U { } let fibonacci = memoize { - // expected-error@-1 {{reasonable time}} fibonacci, n in n < 2 ? Double(n) : fibonacci(n - 1) + fibonacci(n - 2) } diff --git a/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift b/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift similarity index 67% rename from validation-test/Sema/type_checker_perf/slow/rdar31742586.swift rename to validation-test/Sema/type_checker_perf/fast/rdar31742586.swift index 0cc33ce8253cf..2c694c570a137 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift @@ -3,5 +3,4 @@ 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/slow/rdar35213699.swift b/validation-test/Sema/type_checker_perf/fast/rdar35213699.swift similarity index 67% rename from validation-test/Sema/type_checker_perf/slow/rdar35213699.swift rename to validation-test/Sema/type_checker_perf/fast/rdar35213699.swift index 5d89ab1541a89..b5a73088942bb 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar35213699.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar35213699.swift @@ -3,6 +3,5 @@ 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/slow/rdar46713933_literal_arg.swift b/validation-test/Sema/type_checker_perf/fast/rdar46713933_literal_arg.swift similarity index 85% rename from validation-test/Sema/type_checker_perf/slow/rdar46713933_literal_arg.swift rename to validation-test/Sema/type_checker_perf/fast/rdar46713933_literal_arg.swift index a0628335b9c36..5256a92a787c7 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar46713933_literal_arg.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar46713933_literal_arg.swift @@ -8,5 +8,4 @@ func wrap(_ key: String, _ value: T) -> T { retur func wrapped() -> Int { return wrap("1", 1) + wrap("1", 1) + wrap("1", 1) + wrap("1", 1) + wrap("1", 1) + wrap("1", 1) - // expected-error@-1 {{the compiler is unable to type-check this expression in reasonable time}} } diff --git a/validation-test/Sema/type_checker_perf/slow/rdar91310777.swift b/validation-test/Sema/type_checker_perf/fast/rdar91310777.swift similarity index 66% rename from validation-test/Sema/type_checker_perf/slow/rdar91310777.swift rename to validation-test/Sema/type_checker_perf/fast/rdar91310777.swift index eb9dcbee848c8..18450b15fe401 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar91310777.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar91310777.swift @@ -9,7 +9,6 @@ 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) } } diff --git a/validation-test/Sema/type_checker_perf/slow/simd_scalar_multiple.swift.gyb b/validation-test/Sema/type_checker_perf/fast/simd_scalar_multiple.swift.gyb similarity index 70% rename from validation-test/Sema/type_checker_perf/slow/simd_scalar_multiple.swift.gyb rename to validation-test/Sema/type_checker_perf/fast/simd_scalar_multiple.swift.gyb index 497afd10785f5..1d5e4f9e0ea6f 100644 --- a/validation-test/Sema/type_checker_perf/slow/simd_scalar_multiple.swift.gyb +++ b/validation-test/Sema/type_checker_perf/fast/simd_scalar_multiple.swift.gyb @@ -1,4 +1,4 @@ -// RUN: %scale-test --invert-result --begin 1 --end 5 --step 1 --select NumLeafScopes %s +// RUN: %scale-test --begin 1 --end 5 --step 1 --select NumLeafScopes %s // REQUIRES: asserts,no_asan diff --git a/validation-test/Sema/type_checker_perf/slow/swift_package_index_1.swift b/validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift similarity index 85% rename from validation-test/Sema/type_checker_perf/slow/swift_package_index_1.swift rename to validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift index 934a80472f9c9..9034d252b39c9 100644 --- a/validation-test/Sema/type_checker_perf/slow/swift_package_index_1.swift +++ b/validation-test/Sema/type_checker_perf/fast/swift_package_index_1.swift @@ -13,7 +13,7 @@ public class Cookie { private let fixedByteSize: Int32 = 56 var totalByteCount: Int32 { - return fixedByteSize + // expected-error {{the compiler is unable to type-check this expression in reasonable time}} + return fixedByteSize + (port != nil ? 2 : 0) + Int32(comment?.utf8.count ?? 0) + Int32(commentURL?.utf8.count ?? 0) + diff --git a/validation-test/Sema/type_checker_perf/slow/swift_package_index_3.swift b/validation-test/Sema/type_checker_perf/fast/swift_package_index_3.swift similarity index 84% rename from validation-test/Sema/type_checker_perf/slow/swift_package_index_3.swift rename to validation-test/Sema/type_checker_perf/fast/swift_package_index_3.swift index 2327507262418..37f0cb798dbc7 100644 --- a/validation-test/Sema/type_checker_perf/slow/swift_package_index_3.swift +++ b/validation-test/Sema/type_checker_perf/fast/swift_package_index_3.swift @@ -9,7 +9,7 @@ extension String { func getProperties( from ics: String ) -> [(name: String, value: String)] { - return ics // expected-error {{the compiler is unable to type-check this expression in reasonable time}} + return ics .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/slow/swift_package_index_4.swift b/validation-test/Sema/type_checker_perf/fast/swift_package_index_4.swift similarity index 94% rename from validation-test/Sema/type_checker_perf/slow/swift_package_index_4.swift rename to validation-test/Sema/type_checker_perf/fast/swift_package_index_4.swift index 8b05bedbea950..eaff8718abba3 100644 --- a/validation-test/Sema/type_checker_perf/slow/swift_package_index_4.swift +++ b/validation-test/Sema/type_checker_perf/fast/swift_package_index_4.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -solver-scope-threshold=60000 +// RUN: %target-swift-frontend -typecheck %s -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 // expected-error {{the compiler is unable to type-check this expression in reasonable time}} + return create <*> 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/slow/swift_package_index_5.swift b/validation-test/Sema/type_checker_perf/fast/swift_package_index_5.swift similarity index 74% rename from validation-test/Sema/type_checker_perf/slow/swift_package_index_5.swift rename to validation-test/Sema/type_checker_perf/fast/swift_package_index_5.swift index e20ea384f9a5f..3f8bf5f2021c7 100644 --- a/validation-test/Sema/type_checker_perf/slow/swift_package_index_5.swift +++ b/validation-test/Sema/type_checker_perf/fast/swift_package_index_5.swift @@ -2,7 +2,7 @@ // REQUIRES: tools-release,no_asan func g(_: T) throws { - let _: T? = //expected-error {{the compiler is unable to type-check this expression in reasonable time}} + let _: T? = (try? f() as? T) ?? (try? f() as? T) ?? (try? f() as? T) ?? diff --git a/validation-test/Sema/type_checker_perf/fast/borderline_flat_map_operator_mix.swift b/validation-test/Sema/type_checker_perf/slow/borderline_flat_map_operator_mix.swift similarity index 50% rename from validation-test/Sema/type_checker_perf/fast/borderline_flat_map_operator_mix.swift rename to validation-test/Sema/type_checker_perf/slow/borderline_flat_map_operator_mix.swift index cca07954da499..b88a73d90121f 100644 --- a/validation-test/Sema/type_checker_perf/fast/borderline_flat_map_operator_mix.swift +++ b/validation-test/Sema/type_checker_perf/slow/borderline_flat_map_operator_mix.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=10 +// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 // REQUIRES: no_asan @@ -10,8 +10,11 @@ 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 + return x.flatMap { z in // expected-error {{the compiler is unable to type-check this expression in reasonable time}} 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/slow/collections_chained_with_plus.swift.gyb b/validation-test/Sema/type_checker_perf/slow/collections_chained_with_plus.swift.gyb new file mode 100644 index 0000000000000..82e558a5a73fb --- /dev/null +++ b/validation-test/Sema/type_checker_perf/slow/collections_chained_with_plus.swift.gyb @@ -0,0 +1,18 @@ +// 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 +} diff --git a/validation-test/Sema/type_checker_perf/slow/rdar26564101.swift b/validation-test/Sema/type_checker_perf/slow/rdar26564101.swift index a6a869ffb7ef7..e9e1d32c90f05 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 -disable-constraint-solver-performance-hacks +// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 // REQUIRES: tools-release,no_asan // UNSUPPORTED: swift_test_mode_optimize_none && OS=linux-gnu diff --git a/validation-test/compiler_crashers_2/35129b57d7eb80f4.swift b/validation-test/compiler_crashers_2_fixed/35129b57d7eb80f4.swift similarity index 70% rename from validation-test/compiler_crashers_2/35129b57d7eb80f4.swift rename to validation-test/compiler_crashers_2_fixed/35129b57d7eb80f4.swift index f1e36b4fb7ab4..043be0648f71a 100644 --- a/validation-test/compiler_crashers_2/35129b57d7eb80f4.swift +++ b/validation-test/compiler_crashers_2_fixed/35129b57d7eb80f4.swift @@ -1,3 +1,3 @@ // {"signature":"(anonymous namespace)::ExprWalker::rewriteTarget(swift::constraints::SyntacticElementTarget)"} -// RUN: not --crash %target-swift-frontend -typecheck %s +// RUN: not %target-swift-frontend -typecheck %s { Sendable(Sendable< Date: Sat, 21 Sep 2024 20:50:27 -0700 Subject: [PATCH 09/52] [ConstraintSystem] Narrowly disable `tryOptimizeGenericDisjunction` when some of the arguments are number literals Don't attempt this optimization if call has number literals. This is intended to narrowly fix situations like: ```swift 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. (cherry picked from commit 8d5cb112efdc86d736f8cc7fda9b3e43a23d2fdb) --- include/swift/Sema/ConstraintSystem.h | 4 ++++ lib/Sema/CSSolver.cpp | 23 +++++++++++++++++++ lib/Sema/TypeCheckConstraints.cpp | 4 ++++ .../old_hack_related_ambiguities.swift | 10 ++++++++ 4 files changed, 41 insertions(+) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 7ded9f8090d06..1c7a8482be4ee 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -491,6 +491,10 @@ 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 51d78cb50aafa..ff0577f21ed05 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2011,6 +2011,29 @@ tryOptimizeGenericDisjunction(ConstraintSystem &cs, Constraint *disjunction, return nullptr; } + if (!cs.performanceHacksEnabled()) { + // 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 2167e0bd75458..b56bc0e40b6a9 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -204,6 +204,10 @@ 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 b935fc974c43e..da8da45cc76dc 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -189,6 +189,16 @@ 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 +} + // swift-distributed-tracing snippet that relies on old hack behavior. protocol TracerInstant { } From 80e12b9dde575d1b3dfad76c92e0e25dbfa59429 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 5 Feb 2025 17:00:37 -0800 Subject: [PATCH 10/52] [CSOptimizer] Improve contextual result type handling during overload matching Result type should only be matched if there are matches on arguments or there are no viable candidates. --- lib/Sema/CSOptimizer.cpp | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 8f40c07eaf658..4b2c025b01cf3 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -416,6 +416,7 @@ static void determineBestChoicesInContext( llvm::TinyPtrVector resultTypes; + bool hasArgumentCandidates = false; for (unsigned i = 0, n = argFuncType->getNumParams(); i != n; ++i) { const auto ¶m = argFuncType->getParams()[i]; auto argType = cs.simplifyType(param.getPlainType()); @@ -458,6 +459,7 @@ static void determineBestChoicesInContext( } argumentCandidates[i].append(types); + hasArgumentCandidates |= !types.empty(); } auto resultType = cs.simplifyType(argFuncType->getResult()); @@ -475,7 +477,7 @@ static void determineBestChoicesInContext( // 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 && + hasArgumentCandidates && llvm::none_of( indices(argFuncType->getParams()), [&](const unsigned argIdx) { auto &candidates = argumentCandidates[argIdx]; @@ -841,10 +843,16 @@ static void 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; + auto canUseContextualResultTypes = [&decl]() { + return decl->isOperator() && !isStandardComparisonOperator(decl); + }; + + // Require exact matches only if all of the arguments + // are literals and there are no usable contextual result + // types that could help narrow favored choices. + bool favorExactMatchesOnly = + onlyLiteralCandidates && + (!canUseContextualResultTypes() || resultTypes.empty()); if (preserveFavoringOfUnlabeledUnaryArgument) { // Old behavior completely disregarded the fact that some of @@ -1007,12 +1015,12 @@ static void determineBestChoicesInContext( // 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 (decl->isOperator() && !isStandardComparisonOperator(decl)) { + // It's only safe to match result types of operators + // because regular functions/methods/subscripts and + // especially initializers could end up with a lot of + // favored overloads because on the result type alone. + if (canUseContextualResultTypes() && + (score > 0 || !hasArgumentCandidates)) { if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) { // Avoid increasing weight based on CGFloat result type // match because that could require narrowing conversion From 2f8a343aef01c6eed75c85c13ebf297b96b665c3 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 5 Feb 2025 17:14:39 -0800 Subject: [PATCH 11/52] [CSOptimizer] Infer result types through ternary expressions --- include/swift/Sema/CSBindings.h | 6 ++++++ lib/Sema/CSOptimizer.cpp | 13 +++++++++++++ ...rator_chains_separated_by_ternary.swift.gyb | 18 ++++++++++++++++++ 3 files changed, 37 insertions(+) create mode 100644 validation-test/Sema/type_checker_perf/fast/operator_chains_separated_by_ternary.swift.gyb diff --git a/include/swift/Sema/CSBindings.h b/include/swift/Sema/CSBindings.h index e67ad8602ee8b..b47bb84294f2e 100644 --- a/include/swift/Sema/CSBindings.h +++ b/include/swift/Sema/CSBindings.h @@ -527,6 +527,12 @@ class BindingSet { void forEachLiteralRequirement( llvm::function_ref callback) const; + void forEachAdjacentVariable( + llvm::function_ref callback) const { + for (auto *typeVar : AdjacentVars) + callback(typeVar); + } + /// Return a literal requirement that has the most impact on the binding /// score. LiteralBindingKind getLiteralForScore() const; diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 4b2c025b01cf3..2a91511e0e2d8 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -469,6 +469,19 @@ static void determineBestChoicesInContext( for (const auto &binding : bindingSet.Bindings) { resultTypes.push_back(binding.BindingType); } + + // Infer bindings for each side of a ternary condition. + bindingSet.forEachAdjacentVariable( + [&cs, &resultTypes](TypeVariableType *adjacentVar) { + auto *adjacentLoc = adjacentVar->getImpl().getLocator(); + // This is one of the sides of a ternary operator. + if (adjacentLoc->directlyAt()) { + auto adjacentBindings = cs.getBindingsFor(adjacentVar); + + for (const auto &binding : adjacentBindings.Bindings) + resultTypes.push_back(binding.BindingType); + } + }); } else { resultTypes.push_back(resultType); } diff --git a/validation-test/Sema/type_checker_perf/fast/operator_chains_separated_by_ternary.swift.gyb b/validation-test/Sema/type_checker_perf/fast/operator_chains_separated_by_ternary.swift.gyb new file mode 100644 index 0000000000000..9e3a298c66295 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/operator_chains_separated_by_ternary.swift.gyb @@ -0,0 +1,18 @@ +// RUN: %scale-test --begin 1 --end 12 --step 1 --select NumLeafScopes %s +// REQUIRES: asserts,no_asan + +func compute(_: UInt32) { +} + +func test(cond: Bool) { + compute(cond + ? 1 +%for i in range(1, N): + + 1 +%end + : 1 +%for i in range(1, N): + * 1 +%end + ) +} From 6c28cdfb7850329d15aae933b72dae896f96b042 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 5 Feb 2025 17:15:32 -0800 Subject: [PATCH 12/52] [CSOptimizer] Infer argument candidates through optionals For example, `??` operator could produce an optional type so `test(<> ?? 0) could result in an optional argument that wraps a type variable. It should be possible to infer bindings from underlying type variable and restore optionality. --- lib/Sema/CSOptimizer.cpp | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 2a91511e0e2d8..6953d708a5a20 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -421,19 +421,40 @@ static void determineBestChoicesInContext( const auto ¶m = argFuncType->getParams()[i]; auto argType = cs.simplifyType(param.getPlainType()); + SmallVector optionals; + // i.e. `??` operator could produce an optional type + // so `test(<> ?? 0) could result in an optional + // argument that wraps a type variable. It should be possible + // to infer bindings from underlying type variable and restore + // optionality. + if (argType->hasTypeVariable()) { + if (auto *typeVar = argType->lookThroughAllOptionalTypes(optionals) + ->getAs()) + argType = typeVar; + } + SmallVector types; if (auto *typeVar = argType->getAs()) { auto bindingSet = cs.getBindingsFor(typeVar); + auto restoreOptionality = [](Type type, unsigned numOptionals) { + for (unsigned i = 0; i != numOptionals; ++i) + type = type->wrapInOptionalType(); + return type; + }; + for (const auto &binding : bindingSet.Bindings) { - types.push_back({binding.BindingType}); + auto type = restoreOptionality(binding.BindingType, optionals.size()); + types.push_back({type}); } for (const auto &literal : bindingSet.Literals) { if (literal.second.hasDefaultType()) { // Add primary default type - types.push_back( - {literal.second.getDefaultType(), /*fromLiteral=*/true}); + auto type = restoreOptionality(literal.second.getDefaultType(), + optionals.size()); + types.push_back({type, + /*fromLiteral=*/true}); } } From 899b2bc1e9b020c2ea2231ff07148092a9f4bf1c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 6 Feb 2025 00:22:40 -0800 Subject: [PATCH 13/52] [CSOptimizer] Allow matching against CGFloat as a contextual result type --- lib/Sema/CSOptimizer.cpp | 13 ------------- .../complex_swiftui_padding_conditions.swift | 2 +- ...oat_type_with_operator_chain_arguments.swift | 17 +++++++++++++++++ 3 files changed, 18 insertions(+), 14 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/contextual_cgfloat_type_with_operator_chain_arguments.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 6953d708a5a20..275ca7d35a178 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1056,19 +1056,6 @@ static void determineBestChoicesInContext( if (canUseContextualResultTypes() && (score > 0 || !hasArgumentCandidates)) { 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, decl, overloadType->getResult(), candidateResultTy, 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 index df038a16ff4cc..4b96ac54851ea 100644 --- 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 @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -swift-version 5 +// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -solver-scope-threshold=1000 // REQUIRES: OS=macosx import SwiftUI diff --git a/validation-test/Sema/type_checker_perf/fast/contextual_cgfloat_type_with_operator_chain_arguments.swift b/validation-test/Sema/type_checker_perf/fast/contextual_cgfloat_type_with_operator_chain_arguments.swift new file mode 100644 index 0000000000000..72fb47673c4ea --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/contextual_cgfloat_type_with_operator_chain_arguments.swift @@ -0,0 +1,17 @@ +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=50 + +// REQUIRES: OS=macosx,no_asan +// REQUIRES: objc_interop + +import Foundation + +struct Size { + var width: CGFloat + var height: CGFloat +} + +func frame(width: CGFloat?, height: CGFloat?) {} + +func test(size: Size?) { + frame(width: ((size?.width ?? 0) * 1) + 1.0, height: ((size?.height ?? 0) * 1) + 1.0) +} From 0dfb2044ef0e2fea98037e109f2a9a151585528d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 6 Feb 2025 10:22:20 -0800 Subject: [PATCH 14/52] [CSOptimizer] Infer types from init calls used as arguments This used to be limited to Double/CGFloat and operator arguments but it's safe to do in general. --- lib/Sema/CSOptimizer.cpp | 89 +++++++++++++++++++++++++++++++++------- 1 file changed, 74 insertions(+), 15 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 275ca7d35a178..3d55ad78dafef 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -23,6 +23,7 @@ #include "swift/Sema/ConstraintSystem.h" #include "llvm/ADT/BitVector.h" #include "llvm/ADT/DenseMap.h" +#include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/TinyPtrVector.h" #include "llvm/Support/SaveAndRestore.h" @@ -144,6 +145,63 @@ static bool isSupportedDisjunction(Constraint *disjunction) { }); } +/// Given the type variable that represents a result type of a +/// function call, check whether that call is to an initializer +/// and based on that deduce possible type for the result. +/// +/// @return A type and a flag that indicates whether there +/// are any viable failable overloads and empty pair if the +/// type variable isn't a result of an initializer call. +static llvm::PointerIntPair +inferTypeFromInitializerResultType(ConstraintSystem &cs, + TypeVariableType *typeVar, + ArrayRef disjunctions) { + assert(typeVar->getImpl().isFunctionResult()); + + auto *resultLoc = typeVar->getImpl().getLocator(); + auto *call = getAsExpr(resultLoc->getAnchor()); + if (!call) + return {}; + + auto *fn = call->getFn()->getSemanticsProvidingExpr(); + + Type instanceTy; + ConstraintLocator *ctorLocator = nullptr; + if (auto *typeExpr = getAsExpr(fn)) { + instanceTy = cs.getType(typeExpr)->getMetatypeInstanceType(); + ctorLocator = + cs.getConstraintLocator(call, {LocatorPathElt::ApplyFunction(), + LocatorPathElt::ConstructorMember()}); + } else if (auto *UDE = getAsExpr(fn)) { + if (!UDE->getName().getBaseName().isConstructor()) + return {}; + instanceTy = cs.getType(UDE->getBase())->getMetatypeInstanceType(); + ctorLocator = cs.getConstraintLocator(UDE, LocatorPathElt::Member()); + } + + if (!instanceTy || !ctorLocator) + return {}; + + auto initRef = + llvm::find_if(disjunctions, [&ctorLocator](Constraint *disjunction) { + return disjunction->getLocator() == ctorLocator; + }); + + if (initRef == disjunctions.end()) + return {}; + + bool hasFailable = + llvm::any_of((*initRef)->getNestedConstraints(), [](Constraint *choice) { + if (choice->isDisabled()) + return false; + auto *decl = + dyn_cast_or_null(getOverloadChoiceDecl(choice)); + return decl && decl->isFailable(); + }); + + return {instanceTy, hasFailable}; +} + NullablePtr getApplicableFnConstraint(ConstraintGraph &CG, Constraint *disjunction) { auto *boundVar = disjunction->getNestedConstraints()[0] @@ -458,21 +516,22 @@ static void determineBestChoicesInContext( } } - // 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, - /*fromInitializerCall=*/true}); - } + // Help situations like `1 + {Double, CGFloat}(...)` by inferring + // a type for the second operand of `+` based on a type being + // constructed. + if (typeVar->getImpl().isFunctionResult()) { + auto binding = + inferTypeFromInitializerResultType(cs, typeVar, disjunctions); + + if (auto instanceTy = binding.getPointer()) { + types.push_back({instanceTy, + /*fromLiteral=*/false, + /*fromInitializerCall=*/true}); + + if (binding.getInt()) + types.push_back({instanceTy->wrapInOptionalType(), + /*fromLiteral=*/false, + /*fromInitializerCall=*/true}); } } } else { From 84d034c38897293dcc89439544d9c83d15b1d4d9 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 6 Feb 2025 15:41:04 -0800 Subject: [PATCH 15/52] [CSOptimizer] Don't reduce the ranking for non-default literal types Since each candidate and overload choice are considered independenty there is no way to judge whether non-default literal type is going to result in a worse solution than non-default one. --- lib/Sema/CSOptimizer.cpp | 71 ++++++++++++++++++--------------- test/Constraints/overload.swift | 22 ++++++++++ 2 files changed, 61 insertions(+), 32 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 3d55ad78dafef..216c45d4b5429 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -717,8 +717,11 @@ static void determineBestChoicesInContext( : 0; } - if (options.contains(MatchFlag::ExactOnly)) - return areEqual(candidateType, paramType) ? 1 : 0; + if (options.contains(MatchFlag::ExactOnly)) { + if (!areEqual(candidateType, paramType)) + return 0; + return options.contains(MatchFlag::Literal) ? 0.3 : 1; + } // Exact match between candidate and parameter types. if (areEqual(candidateType, paramType)) { @@ -726,20 +729,39 @@ static void 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 paramType->isDouble() ? 0.2 : 0.3; - - if (candidateType->isDouble() && - TypeChecker::conformsToKnownProtocol( - paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) - return 0.3; + if (candidateType->isInt() || candidateType->isDouble()) { + if (paramType->hasTypeParameter() || + paramType->isAnyExistentialType()) { + // Attempt to match literal default to generic parameter. + // This helps to determine whether there are any generic + // overloads that are a possible match. + auto score = + scoreCandidateMatch(genericSig, choice, candidateType, + paramType, options - MatchFlag::Literal); + if (score == 0) + return 0; + + // Optional injection lowers the score for operators to match + // pre-optimizer behavior. + return choice->isOperator() && paramType->getOptionalObjectType() + ? 0.2 + : 0.3; + } else { + // Integer and floating-point literals can match any parameter + // type that conforms to `ExpressibleBy{Integer, Float}Literal` + // protocol. Since this assessment is done in isolation we don't + // lower the score even though this would be a non-default binding + // for a literal. + if (candidateType->isInt() && + TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByIntegerLiteral)) + return 0.3; + + if (candidateType->isDouble() && + TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) + return 0.3; + } } return 0; @@ -1069,10 +1091,7 @@ static void determineBestChoicesInContext( continue; } - // 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 (!candidate.type->hasTypeVariable()) mismatches.set(candidateIdx); } @@ -1093,18 +1112,6 @@ static void 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 && decl->isOperator()) - score = 0.1; - // If one of the result types matches exactly, that's a good // indication that overload choice should be favored. // diff --git a/test/Constraints/overload.swift b/test/Constraints/overload.swift index 87c132e21c5ac..699c1c06d97cc 100644 --- a/test/Constraints/overload.swift +++ b/test/Constraints/overload.swift @@ -350,6 +350,28 @@ do { } } +// Make sure that the solver properly handles mix of non-default integer and floating-point literals +do { + func test( + withInitialValue initialValue: Float, + increment: Float, + count: Int) -> [Float] {} + + func test( + withInitialValue initialValue: Double, + increment: Double, + count: Int) -> [Double] {} + + + func testDoubleVsFloat(count: Int) { + let returnedResult = test(withInitialValue: 0, + increment: 0.1, + count: count) + + let _: [Double] = returnedResult // Ok + } +} + do { struct S { let x: Int From 666aa2422442e5710842010c283f2bc045a7e46e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 6 Feb 2025 16:32:12 -0800 Subject: [PATCH 16/52] [CSOptimizer] Add support for `??` operator --- lib/Sema/CSOptimizer.cpp | 10 +++++++--- .../implicit_double_cgfloat_conversion.swift | 4 ++++ test/Constraints/nil-coalescing-favoring.swift | 16 ++++++++++++++++ .../nil_coalescing_dictionary_values.swift.gyb | 10 ++++++++++ .../nil_coalescing_dictionary_values.swift.gyb | 10 ---------- 5 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/nil_coalescing_dictionary_values.swift.gyb delete mode 100644 validation-test/Sema/type_checker_perf/slow/nil_coalescing_dictionary_values.swift.gyb diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 216c45d4b5429..6a9ef975abdd9 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -78,7 +78,7 @@ static bool isSupportedOperator(Constraint *disjunction) { auto name = decl->getBaseIdentifier(); if (name.isArithmeticOperator() || name.isStandardComparisonOperator() || - name.isBitwiseOperator()) { + name.isBitwiseOperator() || name.isNilCoalescingOperator()) { return true; } @@ -783,7 +783,8 @@ static void determineBestChoicesInContext( // 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 > 0 && choice->isOperator() ? score.value() - 0.1 + : score; } // Optionality mismatch. @@ -1023,6 +1024,9 @@ static void determineBestChoicesInContext( auto paramType = param.getPlainType(); + if (paramFlags.isAutoClosure()) + paramType = paramType->castTo()->getResult(); + // FIXME: Let's skip matching function types for now // because they have special rules for e.g. Concurrency // (around @Sendable) and @convention(c). @@ -1146,7 +1150,7 @@ static void determineBestChoicesInContext( [&resultTy](const auto ¶m) { return param.getPlainType()->isEqual(resultTy); })) - score += 0.1; + score += 0.01; } favoredChoices.push_back({choice, score}); diff --git a/test/Constraints/implicit_double_cgfloat_conversion.swift b/test/Constraints/implicit_double_cgfloat_conversion.swift index f902a5db4f84e..0f4266ef5fa7d 100644 --- a/test/Constraints/implicit_double_cgfloat_conversion.swift +++ b/test/Constraints/implicit_double_cgfloat_conversion.swift @@ -382,6 +382,10 @@ do { } func test_cgfloat_operator_is_attempted_with_literal_arguments(v: CGFloat?) { + // Make sure that @autoclosure thunk calls CGFloat./ and not Double./ + // CHECK-LABEL: sil private [transparent] [ossa] @$s34implicit_double_cgfloat_conversion05test_C45_operator_is_attempted_with_literal_arguments1vy12CoreGraphics7CGFloatVSg_tFAFyKXEfu_ + // CHECK: [[CGFLOAT_DIV_OP:%.*]] = function_ref @$s12CoreGraphics7CGFloatV34implicit_double_cgfloat_conversionE1doiyA2C_ACtFZ : $@convention(method) (CGFloat, CGFloat, @thin CGFloat.Type) -> CGFloat + // CHECK-NEXT: {{.*}} = apply [[CGFLOAT_DIV_OP]]({{.*}}, %2) : $@convention(method) (CGFloat, CGFloat, @thin CGFloat.Type) -> CGFloat let ratio = v ?? (2.0 / 16.0) let _: CGFloat = ratio // Ok } diff --git a/test/Constraints/nil-coalescing-favoring.swift b/test/Constraints/nil-coalescing-favoring.swift index 1723900d5a58d..5dbdeb1f69b10 100644 --- a/test/Constraints/nil-coalescing-favoring.swift +++ b/test/Constraints/nil-coalescing-favoring.swift @@ -12,3 +12,19 @@ struct A { self.init(other ?? ._none) } } + +do { + class Super {} + class Sub: Super {} + + func flatMap(_: (Int) -> R?) -> R? {} + + func test() { + let dict: Dictionary + let sup: Super + + // CHECK: declref_expr type="(consuming Super?, @autoclosure () throws -> Super) throws -> Super" {{.*}} decl="Swift.(file).?? + let x = flatMap { dict[$0] } ?? sup // Ok + let _: Super = x + } +} diff --git a/validation-test/Sema/type_checker_perf/fast/nil_coalescing_dictionary_values.swift.gyb b/validation-test/Sema/type_checker_perf/fast/nil_coalescing_dictionary_values.swift.gyb new file mode 100644 index 0000000000000..4c15caad1276c --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/nil_coalescing_dictionary_values.swift.gyb @@ -0,0 +1,10 @@ +// RUN: %scale-test --begin 1 --end 10 --step 1 --select NumLeafScopes %s -Xfrontend=-typecheck +// REQUIRES: asserts, no_asan + +let x: Int? + +let _ = [ +%for i in range(0, N): + "%{i}" : x ?? 0, +%end +] diff --git a/validation-test/Sema/type_checker_perf/slow/nil_coalescing_dictionary_values.swift.gyb b/validation-test/Sema/type_checker_perf/slow/nil_coalescing_dictionary_values.swift.gyb deleted file mode 100644 index 878c0a9dc78d1..0000000000000 --- a/validation-test/Sema/type_checker_perf/slow/nil_coalescing_dictionary_values.swift.gyb +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %scale-test --invert-result --begin 1 --end 8 --step 1 --select NumLeafScopes %s -Xfrontend=-typecheck -// REQUIRES: asserts, no_asan - -let x: Int? - -let _ = [ -%for i in range(0, N): - "k" : x ?? 0, -%end -] From 26b86c2962e1d92f66b525be26c59b2aadfd5a52 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 7 Feb 2025 21:14:50 -0800 Subject: [PATCH 17/52] [Tests] NFC: Add a test-case with literal chain passed as an argument to a constructor --- .../Sema/type_checker_perf/fast/rdar144100160.swift | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 validation-test/Sema/type_checker_perf/fast/rdar144100160.swift diff --git a/validation-test/Sema/type_checker_perf/fast/rdar144100160.swift b/validation-test/Sema/type_checker_perf/fast/rdar144100160.swift new file mode 100644 index 0000000000000..7e80fd5c3d6c8 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/rdar144100160.swift @@ -0,0 +1,12 @@ +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=100 +// REQUIRES: asserts,no_asan + +typealias TimeInterval = Double + +struct Date { + func addingTimeInterval(_: TimeInterval) -> Date { Date() } +} + +func test(date: Date) { + _ = date.addingTimeInterval(TimeInterval(60 * 60 * 24 * 6 + 12 * 60 + 12 + 1)) +} From b96139eb5bae25b557b39e622abaf54f0a5152f2 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 8 Feb 2025 10:54:50 -0800 Subject: [PATCH 18/52] [CSOptimizer] Emulate old hack for unary argument matching more precisely Having it be part of the other matching wasn't a good idea because previous "favoring" happened only in a few situations - if argument was a declaration reference, application or (dynamic) subscript that had overload choice selected during constraint generation. --- lib/Sema/CSOptimizer.cpp | 76 ++++++++++++++----- .../old_hack_related_ambiguities.swift | 20 ++++- 2 files changed, 75 insertions(+), 21 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 6a9ef975abdd9..34c370ffc09bc 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -338,6 +338,52 @@ static void findFavoredChoicesBasedOnArity( favoredChoice(choice); } +/// Preserves old behavior where, for unary calls, the solver would not previously +/// consider choices that didn't match on the number of parameters (regardless of +/// defaults and variadics) and only exact matches were favored. +static std::optional preserveFavoringOfUnlabeledUnaryArgument( + ConstraintSystem &cs, Constraint *disjunction, ArgumentList *argumentList) { + if (!argumentList->isUnlabeledUnary()) + return std::nullopt; + + auto ODRE = isOverloadedDeclRef(disjunction); + bool preserveFavoringOfUnlabeledUnaryArgument = + !ODRE || numOverloadChoicesMatchingOnArity(ODRE, argumentList) < 2; + + if (!preserveFavoringOfUnlabeledUnaryArgument) + return std::nullopt; + + auto *argument = + argumentList->getUnlabeledUnaryExpr()->getSemanticsProvidingExpr(); + // The hack operated on "favored" types and only declaration references, + // applications, and (dynamic) subscripts had them if they managed to + // get an overload choice selected during constraint generation. + if (!(isExpr(argument) || isExpr(argument) || + isExpr(argument) || + isExpr(argument))) + return {/*score=*/0}; + + auto argumentType = cs.getType(argument); + if (argumentType->hasTypeVariable() || argumentType->hasDependentMember()) + return {/*score=*/0}; + + SmallVector favoredChoices; + forEachDisjunctionChoice( + cs, disjunction, + [&argumentType, &favoredChoices](Constraint *choice, ValueDecl *decl, + FunctionType *overloadType) { + if (overloadType->getNumParams() != 1) + return; + + auto paramType = overloadType->getParams()[0].getPlainType(); + if (paramType->isEqual(argumentType)) + favoredChoices.push_back(choice); + }); + + return DisjunctionInfo(/*score=*/favoredChoices.empty() ? 0 : 1, + favoredChoices); +} + } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -444,6 +490,16 @@ static void determineBestChoicesInContext( } } + // 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. + if (auto info = preserveFavoringOfUnlabeledUnaryArgument(cs, disjunction, + argumentList)) { + recordResult(disjunction, std::move(info.value())); + continue; + } + if (!isSupportedDisjunction(disjunction)) continue; @@ -931,17 +987,6 @@ static void 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; - } - forEachDisjunctionChoice( cs, disjunction, [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { @@ -970,15 +1015,6 @@ static void determineBestChoicesInContext( onlyLiteralCandidates && (!canUseContextualResultTypes() || resultTypes.empty()); - if (preserveFavoringOfUnlabeledUnaryArgument) { - // Old behavior completely disregarded the fact that some of - // the parameters could be defaulted. - if (overloadType->getNumParams() != 1) - return; - - 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*, ...) -> ...`. diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index da8da45cc76dc..c9e268948d440 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -6,14 +6,18 @@ func entity(_: Int) -> Int { struct Test { func test(_ v: Int) -> Int { v } + // expected-note@-1 {{found this candidate}} func test(_ v: Int?) -> Int? { v } + // expected-note@-1 {{found this candidate}} } func test_ternary_literal(v: Test) -> Int? { - true ? v.test(0) : nil // Ok + // Literals don't have a favored type + true ? v.test(0) : nil // expected-error {{ambiguous use of 'test'}} } func test_ternary(v: Test) -> Int? { + // Because calls had favored types set if they were resolved during constraint generation. true ? v.test(entity(0)) : nil // Ok } @@ -159,12 +163,14 @@ do { var p: UnsafeMutableRawPointer { get { fatalError() } } func f(_ p: UnsafeMutableRawPointer) { + // The old hack (which is now removed) couldn't handle member references, only direct declaration references. guard let x = UnsafeMutablePointer(OpaquePointer(self.p)) else { return } _ = x guard let x = UnsafeMutablePointer(OpaquePointer(p)) else { + // expected-error@-1 {{initializer for conditional binding must have Optional type, not 'UnsafeMutablePointer'}} return } _ = x @@ -257,3 +263,15 @@ func test_non_default_literal_use(arg: Float) { let v = arg * 2.0 // shouldn't use `(Float, Double) -> Double` overload let _: Float = v // Ok } + +// This should be ambiguous without contextual type but was accepted before during to +// unlabeled unary argument favoring. +func test_variadic_static_member_is_preferred_over_partially_applied_instance_overload() { + struct Test { + func fn() {} + static func fn(_: Test...) {} + } + + let t: Test + Test.fn(t) // Ok +} From 0e0b5f9a80a8e2c3c92eca2eedd94e3cb156dfe7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 8 Feb 2025 22:21:53 -0800 Subject: [PATCH 19/52] [CSOptimizer] Always prefer a disjunction with a single active choice Disjunctions with a single element are sometimes introduced after disfavoring, so we need to make sure that they are always preferred during disjunction selection. --- lib/Sema/CSOptimizer.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 34c370ffc09bc..46f516d570bd3 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1431,6 +1431,9 @@ ConstraintSystem::selectDisjunction() { unsigned firstActive = first->countActiveNestedConstraints(); unsigned secondActive = second->countActiveNestedConstraints(); + if (firstActive == 1 || secondActive == 1) + return secondActive != 1; + auto &[firstScore, firstFavoredChoices] = favorings[first]; auto &[secondScore, secondFavoredChoices] = favorings[second]; From 0e72686a2ca6f00da93ca778a11baaa1793323ed Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 8 Feb 2025 22:46:40 -0800 Subject: [PATCH 20/52] [CSSolver] Attempt choices marked as disfavored at the end of partition Since such choices are all but guaranteed to be worst than any other non-disfavored choice, let's attempt them last to avoid having to form a complete solution just to filter it out during ranking. --- lib/Sema/CSSolver.cpp | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index ff0577f21ed05..13efcb2406db1 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -2310,6 +2310,8 @@ void DisjunctionChoiceProducer::partitionDisjunction( // end of the partitioning. SmallVector favored; SmallVector everythingElse; + // Disfavored choices are part of `everythingElse` but introduced at the end. + SmallVector disfavored; SmallVector simdOperators; SmallVector disabled; SmallVector unavailable; @@ -2342,6 +2344,11 @@ void DisjunctionChoiceProducer::partitionDisjunction( everythingElse.push_back(index); return true; } + + if (decl->getAttrs().hasAttribute()) { + disfavored.push_back(index); + return true; + } } return false; @@ -2385,6 +2392,9 @@ void DisjunctionChoiceProducer::partitionDisjunction( return true; }); + // Introduce disfavored choices at the end. + everythingElse.append(disfavored); + // Local function to create the next partition based on the options // passed in. PartitionAppendCallback appendPartition = From 2646efac9819046d85a7258474c590645862cf13 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 10 Feb 2025 16:42:15 -0800 Subject: [PATCH 21/52] [CSOptimizer] Expand literal support to bools, strings and dictionaries Optimizer now covers all of the most common ExpressibleBy*Literal protocols. --- lib/Sema/CSOptimizer.cpp | 137 +++++++++++++++++++++++---------------- 1 file changed, 82 insertions(+), 55 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 46f516d570bd3..2b8fe847ad588 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -69,6 +69,12 @@ static bool isUnboundArrayType(Type type) { return false; } +static bool isUnboundDictionaryType(Type type) { + if (auto *UGT = type->getAs()) + return UGT->getDecl() == type->getASTContext().getDictionaryDecl(); + return false; +} + static bool isSupportedOperator(Constraint *disjunction) { if (!isOperatorDisjunction(disjunction)) return false; @@ -749,31 +755,17 @@ static void determineBestChoicesInContext( } } - // Match `[...]` to Array<...> and/or `ExpressibleByArrayLiteral` - // conforming types. - if (options.contains(MatchFlag::OnParam) && - options.contains(MatchFlag::Literal) && - isUnboundArrayType(candidateType)) { + if (options.contains(MatchFlag::ExactOnly)) { // 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->isArray() ? 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::Literal)) { + if (isUnboundArrayType(candidateType)) + return paramType->isArrayType() ? 0.3 : 0; + + if (isUnboundDictionaryType(candidateType)) + return cs.isDictionaryType(paramType) ? 0.3 : 0; + } - if (options.contains(MatchFlag::ExactOnly)) { if (!areEqual(candidateType, paramType)) return 0; return options.contains(MatchFlag::Literal) ? 0.3 : 1; @@ -785,39 +777,73 @@ static void determineBestChoicesInContext( } if (options.contains(MatchFlag::Literal)) { - if (candidateType->isInt() || candidateType->isDouble()) { - if (paramType->hasTypeParameter() || - paramType->isAnyExistentialType()) { - // Attempt to match literal default to generic parameter. - // This helps to determine whether there are any generic - // overloads that are a possible match. - auto score = - scoreCandidateMatch(genericSig, choice, candidateType, - paramType, options - MatchFlag::Literal); - if (score == 0) - return 0; - - // Optional injection lowers the score for operators to match - // pre-optimizer behavior. - return choice->isOperator() && paramType->getOptionalObjectType() - ? 0.2 - : 0.3; - } else { - // Integer and floating-point literals can match any parameter - // type that conforms to `ExpressibleBy{Integer, Float}Literal` - // protocol. Since this assessment is done in isolation we don't - // lower the score even though this would be a non-default binding - // for a literal. - if (candidateType->isInt() && - TypeChecker::conformsToKnownProtocol( - paramType, KnownProtocolKind::ExpressibleByIntegerLiteral)) - return 0.3; - - if (candidateType->isDouble() && - TypeChecker::conformsToKnownProtocol( - paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) - return 0.3; - } + if (paramType->hasTypeParameter() || + paramType->isAnyExistentialType()) { + // Attempt to match literal default to generic parameter. + // This helps to determine whether there are any generic + // overloads that are a possible match. + auto score = + scoreCandidateMatch(genericSig, choice, candidateType, + paramType, options - MatchFlag::Literal); + if (score == 0) + return 0; + + // Optional injection lowers the score for operators to match + // pre-optimizer behavior. + return choice->isOperator() && paramType->getOptionalObjectType() + ? 0.2 + : 0.3; + } else { + // Integer and floating-point literals can match any parameter + // type that conforms to `ExpressibleBy{Integer, Float}Literal` + // protocol. Since this assessment is done in isolation we don't + // lower the score even though this would be a non-default binding + // for a literal. + if (candidateType->isInt() && + TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByIntegerLiteral)) + return 0.3; + + if (candidateType->isDouble() && + TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) + return 0.3; + + if (candidateType->isBool() && + TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByBooleanLiteral)) + return 0.3; + + if (candidateType->isString() && + (TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByStringLiteral) || + TypeChecker::conformsToKnownProtocol( + paramType, + KnownProtocolKind::ExpressibleByStringInterpolation))) + return 0.3; + + auto &ctx = cs.getASTContext(); + + // Check if the other side conforms to `ExpressibleByArrayLiteral` + // protocol (in some way). We want an overly optimistic result + // here to avoid under-favoring. + if (candidateType->isArray() && + checkConformanceWithoutContext( + paramType, + ctx.getProtocol(KnownProtocolKind::ExpressibleByArrayLiteral), + /*allowMissing=*/true)) + return 0.3; + + // Check if the other side conforms to + // `ExpressibleByDictionaryLiteral` protocol (in some way). + // We want an overly optimistic result here to avoid under-favoring. + if (candidateType->isDictionary() && + checkConformanceWithoutContext( + paramType, + ctx.getProtocol( + KnownProtocolKind::ExpressibleByDictionaryLiteral), + /*allowMissing=*/true)) + return 0.3; } return 0; @@ -896,6 +922,7 @@ static void determineBestChoicesInContext( // dependent member type (i.e. `Self.T`), let's check conformances // only and lower the score. if (candidateType->hasTypeVariable() || + candidateType->hasUnboundGenericType() || paramType->is()) { return checkProtocolRequirementsOnly(); } From b936900eb88ad00378a94edafdf32b9a3dcb0de9 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 10 Feb 2025 18:10:53 -0800 Subject: [PATCH 22/52] [CSOptimizer] Support non-operator generic choices with simple signatures If there are no-same type requirements and parameters use either concrete types or generic parameter types directly, the optimizer should be able to handle ranking. Currently candidate arguments are considered in isolation which makes it impossible to deal with same-type requirements and complex generic signatures. --- lib/Sema/CSOptimizer.cpp | 55 +++++++++++++++++-- ...gfloat_double_conversion_correctness.swift | 5 ++ 2 files changed, 56 insertions(+), 4 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 2b8fe847ad588..55784683c22bf 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -117,11 +117,47 @@ static bool isArithmeticOperator(ValueDecl *decl) { return decl->isOperator() && decl->getBaseIdentifier().isArithmeticOperator(); } +/// Generic choices are supported only if they are not complex enough +/// that would they'd require solving to figure out whether they are a +/// potential match or not. +static bool isSupportedGenericOverloadChoice(ValueDecl *decl, + GenericFunctionType *choiceType) { + // Same type requirements cannot be handled because each + // candidate-parameter pair is (currently) considered in isolation. + if (llvm::any_of(choiceType->getRequirements(), [](const Requirement &req) { + switch (req.getKind()) { + case RequirementKind::SameType: + case RequirementKind::SameShape: + return true; + + case RequirementKind::Conformance: + case RequirementKind::Superclass: + case RequirementKind::Layout: + return false; + } + })) + return false; + + // If there are no same-type requirements, allow signatures + // that use only concrete types or generic parameters directly + // in their parameter positions i.e. `(T, Int)`. + + auto *paramList = getParameterList(decl); + if (!paramList) + return false; + + return llvm::all_of(paramList->getArray(), [](const ParamDecl *P) { + auto paramType = P->getInterfaceType(); + return paramType->is() || + !paramType->hasTypeParameter(); + }); +} + static bool isSupportedDisjunction(Constraint *disjunction) { auto choices = disjunction->getNestedConstraints(); - if (isSupportedOperator(disjunction)) - return true; + if (isOperatorDisjunction(disjunction)) + return isSupportedOperator(disjunction); if (auto *ctor = dyn_cast_or_null( getOverloadChoiceDecl(choices.front()))) { @@ -130,7 +166,7 @@ static bool isSupportedDisjunction(Constraint *disjunction) { } // Non-operator disjunctions are supported only if they don't - // have any generic choices. + // have any complex generic choices. return llvm::all_of(choices, [&](Constraint *choice) { if (choice->isDisabled()) return true; @@ -144,7 +180,18 @@ static bool isSupportedDisjunction(Constraint *disjunction) { if (decl->isImplicitlyUnwrappedOptional()) return false; - return decl->getInterfaceType()->is(); + auto choiceType = decl->getInterfaceType()->getAs(); + if (!choiceType || choiceType->hasError()) + return false; + + // Non-generic choices are always supported. + if (choiceType->is()) + return true; + + if (auto *genericFn = choiceType->getAs()) + return isSupportedGenericOverloadChoice(decl, genericFn); + + return false; } return false; diff --git a/validation-test/Sema/implicit_cgfloat_double_conversion_correctness.swift b/validation-test/Sema/implicit_cgfloat_double_conversion_correctness.swift index 121812a279c67..4aeb205e7f129 100644 --- a/validation-test/Sema/implicit_cgfloat_double_conversion_correctness.swift +++ b/validation-test/Sema/implicit_cgfloat_double_conversion_correctness.swift @@ -47,3 +47,8 @@ func test_atan_ambiguity(points: (CGPoint, CGPoint)) { test = atan((points.1.y - points.0.y) / (points.1.x - points.0.x)) // Ok _ = test } + +func test_ambigity_with_generic_funcs(a: CGFloat, b: CGFloat) -> [CGFloat] { + let result = [round(abs(a - b) * 100) / 100.0] + return result +} From aedc4fe25e69ae69fa06bdf60c66e685d9ebc193 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 15 Feb 2025 16:49:44 -0800 Subject: [PATCH 23/52] [CSOptimizer] Remove `selectBestBindingDisjunction` hack --- lib/Sema/CSOptimizer.cpp | 60 ---------------------------------------- 1 file changed, 60 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 55784683c22bf..917d1b5e5f9b8 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1342,63 +1342,6 @@ static void determineBestChoicesInContext( } } -// 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().gatherNearbyConstraints( - typeVar, [](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; -} - /// Prioritize `build{Block, Expression, ...}` and any chained /// members that are connected to individual builder elements /// i.e. `ForEach(...) { ... }.padding(...)`, once `ForEach` @@ -1486,9 +1429,6 @@ ConstraintSystem::selectDisjunction() { if (disjunctions.empty()) return std::nullopt; - if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) - return std::make_pair(disjunction, llvm::TinyPtrVector()); - llvm::DenseMap favorings; determineBestChoicesInContext(*this, disjunctions, favorings); From ac24a8e251991711851dfd29e0f376cf873ad426 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 16 Feb 2025 00:16:09 -0800 Subject: [PATCH 24/52] [CSOptimizer] Introduce `_OptionalNilComparisonType` as candidate for `nil` while ranking `==` and `!=` operators `==` and `!=` operators have special overloads that allow matching `nil` literal on either side even if wrapped type on the other side doesn't conform to `Equatable`. --- include/swift/AST/KnownStdlibTypes.def | 1 + lib/Sema/CSOptimizer.cpp | 20 ++++++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/include/swift/AST/KnownStdlibTypes.def b/include/swift/AST/KnownStdlibTypes.def index 425c846e26854..77b9d5b84e463 100644 --- a/include/swift/AST/KnownStdlibTypes.def +++ b/include/swift/AST/KnownStdlibTypes.def @@ -70,6 +70,7 @@ KNOWN_STDLIB_TYPE_DECL(WritableKeyPath, NominalTypeDecl, 2) KNOWN_STDLIB_TYPE_DECL(ReferenceWritableKeyPath, NominalTypeDecl, 2) KNOWN_STDLIB_TYPE_DECL(Optional, EnumDecl, 1) +KNOWN_STDLIB_TYPE_DECL(_OptionalNilComparisonType, NominalTypeDecl, 0) KNOWN_STDLIB_TYPE_DECL(OptionSet, NominalTypeDecl, 1) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 917d1b5e5f9b8..17be1a1f9870b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -113,6 +113,13 @@ static bool isStandardComparisonOperator(ValueDecl *decl) { decl->getBaseIdentifier().isStandardComparisonOperator(); } +static bool isOperatorNamed(Constraint *disjunction, StringRef name) { + auto *choice = disjunction->getNestedConstraints()[0]; + if (auto *decl = getOverloadChoiceDecl(choice)) + return decl->isOperator() && decl->getBaseIdentifier().is(name); + return false; +} + static bool isArithmeticOperator(ValueDecl *decl) { return decl->isOperator() && decl->getBaseIdentifier().isArithmeticOperator(); } @@ -622,6 +629,19 @@ static void determineBestChoicesInContext( optionals.size()); types.push_back({type, /*fromLiteral=*/true}); + } else if (literal.first == + cs.getASTContext().getProtocol( + KnownProtocolKind::ExpressibleByNilLiteral) && + literal.second.IsDirectRequirement) { + // `==` and `!=` operators have special overloads that accept `nil` + // as `_OptionalNilComparisonType` which is preferred over a + // generic form `(T?, T?)`. + if (isOperatorNamed(disjunction, "==") || + isOperatorNamed(disjunction, "!=")) { + auto nilComparisonTy = + cs.getASTContext().get_OptionalNilComparisonTypeType(); + types.push_back({nilComparisonTy, /*fromLiteral=*/true}); + } } } From 0525818a33b738f8e77fbf26359b53b6aad271ec Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 16 Feb 2025 18:51:00 -0800 Subject: [PATCH 25/52] [CSOptimizer] Allow matching candidates with optional types against generic paameter types For example passing `Int?` to `T` should be considered a match if `T` doesn't have any requirements that block it. --- lib/Sema/CSOptimizer.cpp | 10 +++++++++- test/Constraints/nil-coalescing-favoring.swift | 14 ++++++++++++++ test/SILOptimizer/infinite_recursion.swift | 2 +- .../nil_coalescing_dictionary_values.swift.gyb | 2 +- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 17be1a1f9870b..5121b4dda4bc0 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -926,7 +926,10 @@ static void determineBestChoicesInContext( paramType = paramType->lookThroughAllOptionalTypes(paramOptionals); if (!candidateOptionals.empty() || !paramOptionals.empty()) { - if (paramOptionals.size() >= candidateOptionals.size()) { + // Can match i.e. Int? to Int or T to Int? + if ((paramOptionals.empty() && + paramType->is()) || + paramOptionals.size() >= candidateOptionals.size()) { auto score = scoreCandidateMatch(genericSig, choice, candidateType, paramType, options); // Injection lowers the score slightly to comply with @@ -1031,6 +1034,11 @@ static void determineBestChoicesInContext( } } + // If there are no requirements associated with the generic + // parameter or dependent member type it could match any type. + if (requirements.empty()) + return 0.7; + // If some of the requirements cannot be satisfied, because // they reference other generic parameters, for example: // ``, let's perform a diff --git a/test/Constraints/nil-coalescing-favoring.swift b/test/Constraints/nil-coalescing-favoring.swift index 5dbdeb1f69b10..341f8609cdd14 100644 --- a/test/Constraints/nil-coalescing-favoring.swift +++ b/test/Constraints/nil-coalescing-favoring.swift @@ -28,3 +28,17 @@ do { let _: Super = x } } + +// Reduced from vapor project. Favoring _only_ an overload of `??` and takes `T?` as a second parameter would result in an invalid solution. +extension Array where Element == UInt8 { + init?(decodingBase32 str: String) { + guard let decoded = str.utf8.withContiguousStorageIfAvailable({ Array(decodingBase32: $0) }) ?? Array(decodingBase32: Array(str.utf8)) else { // Ok + return nil + } + self = decoded + } + + init?(decodingBase32 bytes: C) where C: RandomAccessCollection, C.Element == UInt8, C.Index == Int { + fatalError() + } +} diff --git a/test/SILOptimizer/infinite_recursion.swift b/test/SILOptimizer/infinite_recursion.swift index 42e64ce15ff2f..c794ba76d24e6 100644 --- a/test/SILOptimizer/infinite_recursion.swift +++ b/test/SILOptimizer/infinite_recursion.swift @@ -282,7 +282,7 @@ public class U { } func == (l: S?, r: S?) -> Bool { - if l == nil && r == nil { return true } // expected-warning {{function call causes an infinite recursion}} + if l == nil && r == nil { return true } guard let l = l, let r = r else { return false } return l === r } diff --git a/validation-test/Sema/type_checker_perf/fast/nil_coalescing_dictionary_values.swift.gyb b/validation-test/Sema/type_checker_perf/fast/nil_coalescing_dictionary_values.swift.gyb index 4c15caad1276c..125e6b4218266 100644 --- a/validation-test/Sema/type_checker_perf/fast/nil_coalescing_dictionary_values.swift.gyb +++ b/validation-test/Sema/type_checker_perf/fast/nil_coalescing_dictionary_values.swift.gyb @@ -5,6 +5,6 @@ let x: Int? let _ = [ %for i in range(0, N): - "%{i}" : x ?? 0, + "${i}" : x ?? 0, %end ] From 4eec3f6788ae6870f1295ccf0592c37c9a6ba1d7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 17 Feb 2025 15:59:25 -0800 Subject: [PATCH 26/52] [CSOptimizer] Introduce a drop of inference to preserved unary argument behavior Thanks to `LinkedExprAnalyzer` unary argument hack was able to infer matching based on literals and arithmetic operator chains, let's preserve that behavior in a more principled manner. --- lib/Sema/CSOptimizer.cpp | 122 +++++++++++++++++- .../old_hack_related_ambiguities.swift | 5 +- 2 files changed, 117 insertions(+), 10 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 5121b4dda4bc0..b458aac1732c5 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -262,6 +262,79 @@ inferTypeFromInitializerResultType(ConstraintSystem &cs, return {instanceTy, hasFailable}; } +/// If the given expression represents a chain of operators that only have +/// only literals as arguments, attempt to deduce a potential type of the +/// chain. For example if chain has only integral literals it's going to +/// be `Int`, if there are some floating-point literals mixed in - it's going +/// to be `Double`. +static Type inferTypeOfArithmeticOperatorChain(DeclContext *dc, ASTNode node) { + auto binaryOp = getAsExpr(node); + if (!binaryOp) + return Type(); + + class OperatorChainAnalyzer : public ASTWalker { + ASTContext &C; + DeclContext *DC; + + llvm::SmallPtrSet literals; + + bool unsupported = false; + + PreWalkResult walkToExprPre(Expr *expr) override { + if (isa(expr)) + return Action::Continue(expr); + + if (isa(expr)) + return Action::Continue(expr); + + // This inference works only with arithmetic operators + // because we know the structure of their overloads. + if (auto *ODRE = dyn_cast(expr)) { + if (auto *choice = ODRE->getDecls().front()) { + if (choice->getBaseIdentifier().isArithmeticOperator()) + return Action::Continue(expr); + } + } + + if (auto *LE = dyn_cast(expr)) { + if (auto *P = TypeChecker::getLiteralProtocol(C, LE)) { + if (auto defaultTy = TypeChecker::getDefaultType(P, DC)) { + if (defaultTy->isInt()) { + // Don't add `Int` if `Double` is already in the list. + if (literals.contains(C.getDoubleType())) + return Action::Continue(expr); + } else if (defaultTy->isDouble()) { + // A single use of a floating-point literal flips the + // type of the entire chain to `Double`. + (void)literals.erase(C.getIntType()); + } + + literals.insert(defaultTy); + return Action::Continue(expr); + } + } + } + + unsupported = true; + return Action::Stop(); + } + + public: + OperatorChainAnalyzer(DeclContext *DC) : C(DC->getASTContext()), DC(DC) {} + + Type chainType() const { + if (unsupported) + return Type(); + return literals.size() != 1 ? Type() : *literals.begin(); + } + }; + + OperatorChainAnalyzer analyzer(dc); + binaryOp->walk(analyzer); + + return analyzer.chainType(); +} + NullablePtr getApplicableFnConstraint(ConstraintGraph &CG, Constraint *disjunction) { auto *boundVar = disjunction->getNestedConstraints()[0] @@ -418,25 +491,62 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( // The hack operated on "favored" types and only declaration references, // applications, and (dynamic) subscripts had them if they managed to // get an overload choice selected during constraint generation. + // It's sometimes possible to infer a type of a literal and an operator + // chain, so it should be allowed as well. if (!(isExpr(argument) || isExpr(argument) || isExpr(argument) || - isExpr(argument))) + isExpr(argument) || + isExpr(argument) || isExpr(argument))) return {/*score=*/0}; - auto argumentType = cs.getType(argument); + auto argumentType = cs.getType(argument)->getRValueType(); + + // For chains like `1 + 2 * 3` it's easy to deduce the type because + // we know what literal types are preferred. + if (isa(argument)) { + auto chainTy = inferTypeOfArithmeticOperatorChain(cs.DC, argument); + if (!chainTy) + return {/*score=*/0}; + + argumentType = chainTy; + } + + // Use default type of a literal (when available) to make a guess. + // This is what old hack used to do as well. + if (auto *LE = dyn_cast(argument)) { + auto *P = TypeChecker::getLiteralProtocol(cs.getASTContext(), LE); + if (!P) + return {/*score=*/0}; + + auto defaultTy = TypeChecker::getDefaultType(P, cs.DC); + if (!defaultTy) + return {/*score=*/0}; + + argumentType = defaultTy; + } + + ASSERT(argumentType); + if (argumentType->hasTypeVariable() || argumentType->hasDependentMember()) return {/*score=*/0}; SmallVector favoredChoices; forEachDisjunctionChoice( cs, disjunction, - [&argumentType, &favoredChoices](Constraint *choice, ValueDecl *decl, - FunctionType *overloadType) { + [&argumentType, &favoredChoices, &argument]( + Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { if (overloadType->getNumParams() != 1) return; - auto paramType = overloadType->getParams()[0].getPlainType(); - if (paramType->isEqual(argumentType)) + auto ¶m = overloadType->getParams()[0]; + + // Literals are speculative, let's not attempt to apply them too + // eagerly. + if (!param.getParameterFlags().isNone() && + (isa(argument) || isa(argument))) + return; + + if (argumentType->isEqual(param.getPlainType())) favoredChoices.push_back(choice); }); diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index c9e268948d440..1aebb0aa1531b 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -6,14 +6,11 @@ func entity(_: Int) -> Int { struct Test { func test(_ v: Int) -> Int { v } - // expected-note@-1 {{found this candidate}} func test(_ v: Int?) -> Int? { v } - // expected-note@-1 {{found this candidate}} } func test_ternary_literal(v: Test) -> Int? { - // Literals don't have a favored type - true ? v.test(0) : nil // expected-error {{ambiguous use of 'test'}} + true ? v.test(0) : nil // Ok } func test_ternary(v: Test) -> Int? { From df4ae0a546d652227b7d393befce833d13114390 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 17 Feb 2025 22:19:32 -0800 Subject: [PATCH 27/52] [CSOptimizer] Update candidate selection to use arithmetic operator chain inference --- lib/Sema/CSOptimizer.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index b458aac1732c5..a4d60210095a0 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -701,6 +701,8 @@ static void determineBestChoicesInContext( llvm::TinyPtrVector resultTypes; bool hasArgumentCandidates = false; + bool isOperator = isOperatorDisjunction(disjunction); + for (unsigned i = 0, n = argFuncType->getNumParams(); i != n; ++i) { const auto ¶m = argFuncType->getParams()[i]; auto argType = cs.simplifyType(param.getPlainType()); @@ -759,6 +761,17 @@ static void determineBestChoicesInContext( // a type for the second operand of `+` based on a type being // constructed. if (typeVar->getImpl().isFunctionResult()) { + auto *resultLoc = typeVar->getImpl().getLocator(); + + // We don't want to try and infer parts of operator + // chains. + if (!isOperator) { + if (auto type = inferTypeOfArithmeticOperatorChain( + cs.DC, resultLoc->getAnchor())) { + types.push_back({type, /*fromLiteral=*/true}); + } + } + auto binding = inferTypeFromInitializerResultType(cs, typeVar, disjunctions); From bcc749fa1acfd6345054cd110e98018228be5a29 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 18 Feb 2025 01:12:24 -0800 Subject: [PATCH 28/52] [CSOptimizer] Reset the overall score of operator disjunctions that is based speculation New ranking + selection algorithm suffered from over-eagerly selecting operator disjunctions vs. unsupported non-operator ones even if the ranking was based purely on literal candidates. This change introduces a notion of a speculative candidate - one which has a type inferred from a literal or an initializer call that has failable overloads and/or implicit conversions (i.e. Double/CGFloat). `determineBestChoicesInContext` would reset the score of an operator disjunction which was computed based on speculative candidates alone but would preserve favoring information. This way selection algorithm would not be skewed towards operators and at the same time if there is no no choice by to select one we'd still have favoring information available which is important for operator chains that consist purely of literals. --- lib/Sema/CSOptimizer.cpp | 104 ++++++++++++++---- .../fast/operators_inside_closure.swift | 9 ++ .../type_checker_perf/fast/rdar47492691.swift | 4 + .../fast/should_skip_bitwise_2.swift | 2 +- .../fast/swift_package_index_1.swift | 2 +- .../slow/array_count_property_vs_method.swift | 11 ++ .../{fast => slow}/rdar23682605.swift | 1 + .../swift_package_index_3.swift | 4 +- 8 files changed, 113 insertions(+), 24 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/operators_inside_closure.swift create mode 100644 validation-test/Sema/type_checker_perf/slow/array_count_property_vs_method.swift rename validation-test/Sema/type_checker_perf/{fast => slow}/rdar23682605.swift (91%) rename validation-test/Sema/type_checker_perf/{fast => slow}/swift_package_index_3.swift (75%) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index a4d60210095a0..f20b53e964972 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -108,9 +108,12 @@ static bool isSupportedSpecialConstructor(ConstructorDecl *ctor) { return false; } -static bool isStandardComparisonOperator(ValueDecl *decl) { - return decl->isOperator() && - decl->getBaseIdentifier().isStandardComparisonOperator(); +static bool isStandardComparisonOperator(Constraint *disjunction) { + auto *choice = disjunction->getNestedConstraints()[0]; + if (auto *decl = getOverloadChoiceDecl(choice)) + return decl->isOperator() && + decl->getBaseIdentifier().isStandardComparisonOperator(); + return false; } static bool isOperatorNamed(Constraint *disjunction, StringRef name) { @@ -694,6 +697,38 @@ static void determineBestChoicesInContext( fromInitializerCall(fromInitializerCall) {} }; + // Determine whether there are any non-speculative choices + // in the given set of candidates. Speculative choices are + // literals or types inferred from initializer calls. + auto anyNonSpeculativeCandidates = + [&](ArrayRef candidates) { + // If there is only one (non-CGFloat) candidate inferred from + // an initializer call we don't consider this a speculation. + // + // CGFloat inference is always speculative because of the + // implicit conversion between Double and CGFloat. + if (llvm::count_if(candidates, [&](const auto &candidate) { + return candidate.fromInitializerCall && + !candidate.type->isCGFloat(); + }) == 1) + return true; + + // If there are no non-literal and non-initializer-inferred types + // in the list, consider this is a speculation. + return llvm::any_of(candidates, [&](const auto &candidate) { + return !candidate.fromLiteral && !candidate.fromInitializerCall; + }); + }; + + auto anyNonSpeculativeResultTypes = [](ArrayRef results) { + return llvm::any_of(results, [](Type resultTy) { + // Double and CGFloat are considered speculative because + // there exists an implicit conversion between them and + // preference is based on score impact in the overall solution. + return !(resultTy->isDouble() || resultTy->isCGFloat()); + }); + }; + SmallVector, 2> argumentCandidates; argumentCandidates.resize(argFuncType->getNumParams()); @@ -818,19 +853,19 @@ static void 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 = + // Determine whether all of the argument candidates are speculative (i.e. + // literals). This information is going to be used later on when we need to + // decide how to score a matching choice. + bool onlySpeculativeArgumentCandidates = hasArgumentCandidates && llvm::none_of( indices(argFuncType->getParams()), [&](const unsigned argIdx) { - auto &candidates = argumentCandidates[argIdx]; - return llvm::any_of(candidates, [&](const auto &candidate) { - return !candidate.fromLiteral; - }); + return anyNonSpeculativeCandidates(argumentCandidates[argIdx]); }); + bool canUseContextualResultTypes = + isOperator && !isStandardComparisonOperator(disjunction); + // Match arguments to the given overload choice. auto matchArguments = [&](OverloadChoice choice, FunctionType *overloadType) -> std::optional { @@ -1229,16 +1264,12 @@ static void determineBestChoicesInContext( if (!matchings) return; - auto canUseContextualResultTypes = [&decl]() { - return decl->isOperator() && !isStandardComparisonOperator(decl); - }; - // Require exact matches only if all of the arguments // are literals and there are no usable contextual result // types that could help narrow favored choices. bool favorExactMatchesOnly = - onlyLiteralCandidates && - (!canUseContextualResultTypes() || resultTypes.empty()); + onlySpeculativeArgumentCandidates && + (!canUseContextualResultTypes || resultTypes.empty()); // This is important for SIMD operators in particular because // a lot of their overloads have same-type requires to a concrete @@ -1384,7 +1415,7 @@ static void determineBestChoicesInContext( // because regular functions/methods/subscripts and // especially initializers could end up with a lot of // favored overloads because on the result type alone. - if (canUseContextualResultTypes() && + if (canUseContextualResultTypes && (score > 0 || !hasArgumentCandidates)) { if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) { return scoreCandidateMatch(genericSig, decl, @@ -1439,6 +1470,34 @@ static void determineBestChoicesInContext( info.FavoredChoices.push_back(choice.first); } + // Reset operator disjunction score but keep favoring + // choices only available candidates where speculative + // with no contextual information available, standard + // comparison operators are a special cases because + // their return type is fixed to `Bool` unlike that of + // bitwise, arithmetic, and other operators. + // + // This helps to prevent over-eager selection of the + // operators over unsupported non-operator declarations. + if (isOperator && onlySpeculativeArgumentCandidates && + (!canUseContextualResultTypes || + !anyNonSpeculativeResultTypes(resultTypes))) { + if (cs.isDebugMode()) { + PrintOptions PO; + PO.PrintTypesForDebugging = true; + + llvm::errs().indent(cs.solverState->getCurrentIndent()) + << "<<< Disjunction " + << disjunction->getNestedConstraints()[0] + ->getFirstType() + ->getString(PO) + << " score " << bestScore + << " was reset due to having only speculative candidates\n"; + } + + info.Score = 0; + } + recordResult(disjunction, std::move(info)); } @@ -1610,8 +1669,13 @@ ConstraintSystem::selectDisjunction() { return *firstScore > *secondScore; } - unsigned numFirstFavored = firstFavoredChoices.size(); - unsigned numSecondFavored = secondFavoredChoices.size(); + // Use favored choices only if disjunction score is higher + // than zero. This means that we can maintain favoring + // choices without impacting selection decisions. + unsigned numFirstFavored = + firstScore.value_or(0) ? firstFavoredChoices.size() : 0; + unsigned numSecondFavored = + secondScore.value_or(0) ? secondFavoredChoices.size() : 0; if (numFirstFavored == numSecondFavored) { if (firstActive != secondActive) diff --git a/validation-test/Sema/type_checker_perf/fast/operators_inside_closure.swift b/validation-test/Sema/type_checker_perf/fast/operators_inside_closure.swift new file mode 100644 index 0000000000000..49d44feb86b28 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/operators_inside_closure.swift @@ -0,0 +1,9 @@ +// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=10000 +// REQUIRES: tools-release,no_asan + +// Selecting operators from the closure before arguments to `zip` makes this "too complex" +func compute(_ ids: [UInt64]) { + let _ = zip(ids[ids.indices.dropLast()], ids[ids.indices.dropFirst()]).map { pair in + ((pair.0 % 2 == 0) && (pair.1 % 2 == 1)) ? UInt64(pair.1 - pair.0) : 42 + } +} diff --git a/validation-test/Sema/type_checker_perf/fast/rdar47492691.swift b/validation-test/Sema/type_checker_perf/fast/rdar47492691.swift index 47fcdcb0771af..7a25cca734f79 100644 --- a/validation-test/Sema/type_checker_perf/fast/rdar47492691.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar47492691.swift @@ -7,3 +7,7 @@ import simd func test(foo: CGFloat, bar: CGFloat) { _ = CGRect(x: 0.0 + 1.0, y: 0.0 + foo, width: 3.0 - 1 - 1 - 1.0, height: bar) } + +func test_with_generic_func_and_literals(bounds: CGRect) { + _ = CGRect(x: 0, y: 0, width: 1, height: bounds.height - 2 + bounds.height / 2 + max(bounds.height / 2, bounds.height / 2)) +} diff --git a/validation-test/Sema/type_checker_perf/fast/should_skip_bitwise_2.swift b/validation-test/Sema/type_checker_perf/fast/should_skip_bitwise_2.swift index fdcfab315d3ad..d7d37c8b65b93 100644 --- a/validation-test/Sema/type_checker_perf/fast/should_skip_bitwise_2.swift +++ b/validation-test/Sema/type_checker_perf/fast/should_skip_bitwise_2.swift @@ -1,3 +1,3 @@ -// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=2000 +// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=4000 func f896() { let _ = ((0 >> ((0 >> 0) + ((0 / 0) & 0))) >> (0 << ((0 << 0) >> (0 << (0 << 0))))) } 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 9034d252b39c9..edcc58b5c71eb 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-typecheck-verify-swift -solver-scope-threshold=1000 +// RUN: %target-swift-frontend -typecheck %s -solver-scope-threshold=700 // REQUIRES: tools-release,no_asan public class Cookie { diff --git a/validation-test/Sema/type_checker_perf/slow/array_count_property_vs_method.swift b/validation-test/Sema/type_checker_perf/slow/array_count_property_vs_method.swift new file mode 100644 index 0000000000000..abcd1cfc545ce --- /dev/null +++ b/validation-test/Sema/type_checker_perf/slow/array_count_property_vs_method.swift @@ -0,0 +1,11 @@ +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=5000 +// REQUIRES: tools-release,no_asan +// REQUIRES: OS=macosx + +// FIXME: Array literals are considered "speculative" bindings at the moment but they cannot actually +// assume different types unlike integer and floating-pointer literals. +func f(n: Int, a: [Int]) { + // expected-error@+1 {{the compiler is unable to type-check this expression in reasonable time}} + let _ = [(0 ..< n + a.count).map { Int8($0) }] + + [(0 ..< n + a.count).map { Int8($0) }.reversed()] // Ok +} 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/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 37f0cb798dbc7..2ef209606305e 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-typecheck-verify-swift -solver-scope-threshold=50000 +// RUN: %target-typecheck-verify-swift %s -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) } From e2fe5589563fd17b6ccf4a7c3130c38e1a85c729 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 18 Feb 2025 01:37:55 -0800 Subject: [PATCH 29/52] [CSOptimizer] Introduce a way to preference disjunctions before scores are considered Some of the disjunctions are not supported by the optimizers but could still be a better choice than an operator. Using a non-score based preference mechanism first allows us to make sure that operator disjunctions are not selected too eagerly in some situations when i.e. a member (supported or not) could be a better choice. `isPreferable` currently targets only operators in result builder contexts but it could be expanded to more uses in the future. --- lib/Sema/CSOptimizer.cpp | 193 +++++++++++------- ...ple_chained_members_in_inner_closure.swift | 34 +++ .../fast/builder_with_nested_closures.swift | 71 +++++++ .../complex_swiftui_padding_conditions.swift | 2 +- 4 files changed, 227 insertions(+), 73 deletions(-) create mode 100644 validation-test/Sema/SwiftUI/swiftui_multiple_chained_members_in_inner_closure.swift create mode 100644 validation-test/Sema/type_checker_perf/fast/builder_with_nested_closures.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index f20b53e964972..16af93c0ee6d4 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -18,6 +18,7 @@ #include "swift/AST/ConformanceLookup.h" #include "swift/AST/ExistentialLayout.h" #include "swift/AST/GenericSignature.h" +#include "swift/Basic/Defer.h" #include "swift/Basic/OptionSet.h" #include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" @@ -49,6 +50,103 @@ struct DisjunctionInfo { : Score(score), FavoredChoices(favoredChoices) {} }; +static DeclContext *getDisjunctionDC(Constraint *disjunction) { + auto *choice = disjunction->getNestedConstraints()[0]; + switch (choice->getKind()) { + case ConstraintKind::BindOverload: + return choice->getOverloadUseDC(); + case ConstraintKind::ValueMember: + case ConstraintKind::UnresolvedValueMember: + case ConstraintKind::ValueWitness: + return choice->getMemberUseDC(); + default: + return nullptr; + } +} + +/// Determine whether the given disjunction appears in a context +/// transformed by a result builder. +static bool isInResultBuilderContext(ConstraintSystem &cs, + Constraint *disjunction) { + auto *DC = getDisjunctionDC(disjunction); + if (!DC) + return false; + + do { + auto fnContext = AnyFunctionRef::fromDeclContext(DC); + if (!fnContext) + return false; + + if (cs.getAppliedResultBuilderTransform(*fnContext)) + return true; + + } while ((DC = DC->getParent())); + + return false; +} + +/// If the given operator disjunction appears in some position +// inside of a not yet resolved call i.e. `a.b(1 + c(4) - 1)` +// both `+` and `-` are "in" argument context of `b`. +static bool isOperatorPassedToUnresolvedCall(ConstraintSystem &cs, + Constraint *disjunction) { + ASSERT(isOperatorDisjunction(disjunction)); + + auto *curr = castToExpr(disjunction->getLocator()->getAnchor()); + while (auto *parent = cs.getParentExpr(curr)) { + SWIFT_DEFER { curr = parent; }; + + switch (parent->getKind()) { + case ExprKind::OptionalEvaluation: + case ExprKind::Paren: + case ExprKind::Binary: + case ExprKind::PrefixUnary: + case ExprKind::PostfixUnary: + continue; + + // a.b(<> ? <> : <<...>>) + case ExprKind::Ternary: { + auto *T = cast(parent); + // If the operator is located in the condition it's + // not tied to the context. + if (T->getCondExpr() == curr) + return false; + + // But the branches are connected to the context. + continue; + } + + // Handles `a(<>), `a[<>]`, + // `.a(<>)` etc. + case ExprKind::Call: { + auto *call = cast(parent); + + // Type(...) + if (isa(call->getFn())) { + auto *ctorLoc = cs.getConstraintLocator( + call, {LocatorPathElt::ApplyFunction(), + LocatorPathElt::ConstructorMember()}); + return !cs.findSelectedOverloadFor(ctorLoc); + } + + // Ignore injected result builder methods like `buildExpression` + // and `buildBlock`. + if (auto *UDE = dyn_cast(call->getFn())) { + if (isResultBuilderMethodReference(cs.getASTContext(), UDE)) + return false; + } + + return !cs.findSelectedOverloadFor(call->getFn()); + } + + default: + return false; + } + } + + return false; +} + // TODO: both `isIntegerType` and `isFloatType` should be available on Type // as `isStdlib{Integer, Float}Type`. @@ -1552,77 +1650,30 @@ static void determineBestChoicesInContext( } } -/// 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; +static std::optional isPreferable(ConstraintSystem &cs, + Constraint *disjunctionA, + Constraint *disjunctionB) { + // Consider only operator vs. non-operator situations. + if (isOperatorDisjunction(disjunctionA) == + isOperatorDisjunction(disjunctionB)) + return std::nullopt; - if (auto depth = cs.getExprDepth(member)) { - if (!best.first || best.second > depth) - best = std::make_pair(disjunction, depth.value()); + // Prevent operator selection if its passed as an argument + // to not-yet resolved call. This helps to make sure that + // in result builder context chained members and other + // non-operator disjunctions are always selected first, + // because they provide the context and help to prune the system. + if (isInResultBuilderContext(cs, disjunctionA)) { + if (isOperatorDisjunction(disjunctionA)) { + if (isOperatorPassedToUnresolvedCall(cs, disjunctionA)) + return false; + } else { + if (isOperatorPassedToUnresolvedCall(cs, disjunctionB)) + return true; } } - return best.first; + return std::nullopt; } std::optional>> @@ -1642,11 +1693,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( @@ -1658,6 +1704,9 @@ ConstraintSystem::selectDisjunction() { if (firstActive == 1 || secondActive == 1) return secondActive != 1; + if (auto preference = isPreferable(*this, first, second)) + return preference.value(); + auto &[firstScore, firstFavoredChoices] = favorings[first]; auto &[secondScore, secondFavoredChoices] = favorings[second]; diff --git a/validation-test/Sema/SwiftUI/swiftui_multiple_chained_members_in_inner_closure.swift b/validation-test/Sema/SwiftUI/swiftui_multiple_chained_members_in_inner_closure.swift new file mode 100644 index 0000000000000..dbfc309569c85 --- /dev/null +++ b/validation-test/Sema/SwiftUI/swiftui_multiple_chained_members_in_inner_closure.swift @@ -0,0 +1,34 @@ +// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx12 -solver-scope-threshold=10000 + +// REQUIRES: OS=macosx +// REQUIRES: objc_interop + +import Foundation +import SwiftUI + +struct MyView: View { + public enum Style { + case focusRing(platterSize: CGSize, stroke: CGFloat, offset: CGFloat) + } + + var style: Style + var isFocused: Bool + var focusColor: Color + + var body: some View { + Group { + switch style { + case let .focusRing(platterSize: platterSize, stroke: focusRingStroke, offset: focusRingOffset): + Circle() + .overlay { + Circle() + .stroke(isFocused ? focusColor : Color.clear, lineWidth: focusRingStroke) + .frame( + width: platterSize.width + (2 * focusRingOffset) + focusRingStroke, + height: platterSize.height + (2 * focusRingOffset) + focusRingStroke + ) + } + } + } + } +} diff --git a/validation-test/Sema/type_checker_perf/fast/builder_with_nested_closures.swift b/validation-test/Sema/type_checker_perf/fast/builder_with_nested_closures.swift new file mode 100644 index 0000000000000..e4c1ebb55efd4 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/builder_with_nested_closures.swift @@ -0,0 +1,71 @@ +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=200 +// REQUIRES: tools-release,no_asan +// REQUIRES: OS=macosx + +struct Time { + static func +(_: Time, _: Double) -> Time { + Time() + } + + static func now() -> Time { Time() } +} + +struct Queue { + static let current = Queue() + + func after(deadline: Time, execute: () -> Void) {} + func after(deadline: Time, execute: (Int) -> Void) {} +} + +func compute(_: () -> Void) {} + +protocol P { +} + +@resultBuilder +struct Builder { + static func buildExpression(_ v: T) -> T { + v + } + + static func buildBlock(_ v: T) -> T { + v + } + + static func buildBlokc(_ v0: T0, _ v1: T1) -> (T0, T1) { + (v0, v1) + } +} + +struct MyP: P { + func onAction(_: () -> Void) -> some P { self } +} + +class Test { + var value = 0.0 + + @Builder func test() -> some P { + MyP().onAction { + Queue.current.after(deadline: .now() + 0.1) { + compute { + value = 0.3 + Queue.current.after(deadline: .now() + 0.2) { + compute { + value = 1.0 + Queue.current.after(deadline: .now() + 0.2) { + compute { + value = 0.3 + Queue.current.after(deadline: .now() + 0.2) { + compute { + value = 1.0 + } + } + } + } + } + } + } + } + } + } +} 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 index 4b96ac54851ea..c9f3af5130530 100644 --- 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 @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -solver-scope-threshold=1000 +// RUN: %target-typecheck-verify-swift -target %target-cpu-apple-macosx10.15 -solver-scope-threshold=5500 // REQUIRES: OS=macosx import SwiftUI From 617338f25051752b6856e6e6253c16ab26e9dfd5 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 20 Feb 2025 13:46:18 -0800 Subject: [PATCH 30/52] [CSOptimizer] Prevent candidate inference from unresolved generic parameters and ternary expressions We need to have a notion of "complete" binding set before we can allow inference from generic parameters and ternary, otherwise we'd make a favoring decision that might not be correct i.e. `v ?? (<> ? nil : o)` where `o` is `Int`. `getBindingsFor` doesn't currently infer transitive bindings which means that for a ternary we'd only have a single binding - `Int` which could lead to favoring overload of `??` and has non-optional parameter on the right-hand side. --- include/swift/Sema/ConstraintSystem.h | 4 ++++ lib/Sema/CSOptimizer.cpp | 12 +++++++++++ lib/Sema/TypeCheckConstraints.cpp | 4 ++++ .../Constraints/nil-coalescing-favoring.swift | 20 +++++++++++++++++++ 4 files changed, 40 insertions(+) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 1c7a8482be4ee..251f6d7b53b29 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -499,6 +499,10 @@ class TypeVariableType::Implementation { /// function call. bool isFunctionResult() const; + /// Determine whether this type variable represents a type of the ternary + /// operator. + bool isTernary() 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 16af93c0ee6d4..1b397b5c7535b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -856,6 +856,18 @@ static void determineBestChoicesInContext( if (auto *typeVar = argType->getAs()) { auto bindingSet = cs.getBindingsFor(typeVar); + // We need to have a notion of "complete" binding set before + // we can allow inference from generic parameters and ternary, + // otherwise we'd make a favoring decision that might not be + // correct i.e. `v ?? (<> ? nil : o)` where `o` is `Int`. + // `getBindingsFor` doesn't currently infer transitive bindings + // which means that for a ternary we'd only have a single + // binding - `Int` which could lead to favoring overload of + // `??` and has non-optional parameter on the right-hand side. + if (typeVar->getImpl().getGenericParameter() || + typeVar->getImpl().isTernary()) + continue; + auto restoreOptionality = [](Type type, unsigned numOptionals) { for (unsigned i = 0; i != numOptionals; ++i) type = type->wrapInOptionalType(); diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index b56bc0e40b6a9..78f49d3de9ca7 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -212,6 +212,10 @@ bool TypeVariableType::Implementation::isFunctionResult() const { return locator && locator->isLastElement(); } +bool TypeVariableType::Implementation::isTernary() const { + return locator && locator->directlyAt(); +} + void *operator new(size_t bytes, ConstraintSystem& cs, size_t alignment) { return cs.getAllocator().Allocate(bytes, alignment); diff --git a/test/Constraints/nil-coalescing-favoring.swift b/test/Constraints/nil-coalescing-favoring.swift index 341f8609cdd14..5caa9ca548156 100644 --- a/test/Constraints/nil-coalescing-favoring.swift +++ b/test/Constraints/nil-coalescing-favoring.swift @@ -42,3 +42,23 @@ extension Array where Element == UInt8 { fatalError() } } + +func test_no_incorrect_favoring(v: Int?, o: Int) { + func ternary(_: T, _: T) -> T { fatalError() } + + func nilCoelesing(_: T?, _: T) -> T { fatalError() } + func nilCoelesing(_: T?, _: T?) -> T? { fatalError() } + + let t1 = v ?? (true ? nil : v) + let t2 = v ?? ternary(nil, o) + + let s1 = nilCoelesing(v, (true ? nil : v)) + let s2 = nilCoelesing(v, ternary(nil, o)) + + func sameType(_: T, as: T.Type) {} + + sameType(t1, as: Int?.self) + sameType(t2, as: Int?.self) + sameType(s1, as: Int?.self) + sameType(s2, as: Int?.self) +} From a953dfe0c0aae5eafd2c2cd79f3e4c274bf12c26 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 20 Feb 2025 16:03:02 -0800 Subject: [PATCH 31/52] [Tests] NFC: Adjust scope threshold for rdar://rdar31742586 test-case The test was slow with hacks but now it's much faster - takes about 63k scopes to solve, it could be improved by introducing new overloads of prefix `-` to stdlib. --- .../Sema/type_checker_perf/fast/rdar31742586.swift | 6 ------ .../Sema/type_checker_perf/slow/rdar31742586.swift | 11 +++++++++++ 2 files changed, 11 insertions(+), 6 deletions(-) delete mode 100644 validation-test/Sema/type_checker_perf/fast/rdar31742586.swift create mode 100644 validation-test/Sema/type_checker_perf/slow/rdar31742586.swift diff --git a/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift b/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift deleted file mode 100644 index 2c694c570a137..0000000000000 --- a/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift +++ /dev/null @@ -1,6 +0,0 @@ -// RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 -// REQUIRES: tools-release,no_asan - -func rdar31742586() -> Double { - return -(1 + 2) + -(3 + 4) + 5 - (-(1 + 2) + -(3 + 4) + 5) -} diff --git a/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift b/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift new file mode 100644 index 0000000000000..85c705779603c --- /dev/null +++ b/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift @@ -0,0 +1,11 @@ +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=1000 +// REQUIRES: tools-release,no_asan + +// The test takes about 63k scopes here due to the fact +// that prefix `-` operator doesn't have an overload that +// take `Int`. We could fix that in the stdlib. + +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}} +} From 073b48c43b695031ba75ed0777674c8882949f9d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 28 Feb 2025 11:58:47 -0800 Subject: [PATCH 32/52] [CSOptimizer] Restrict unary argument legacy favoring behavior to `ApplyExpr`s The original hack never applied to subscripts. --- lib/Sema/CSOptimizer.cpp | 4 ++++ .../old_hack_related_ambiguities.swift | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 1b397b5c7535b..c9315d6b8367a 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -580,6 +580,10 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( if (!argumentList->isUnlabeledUnary()) return std::nullopt; + if (!isExpr( + cs.getParentExpr(argumentList->getUnlabeledUnaryExpr()))) + return std::nullopt; + auto ODRE = isOverloadedDeclRef(disjunction); bool preserveFavoringOfUnlabeledUnaryArgument = !ODRE || numOverloadChoicesMatchingOnArity(ODRE, argumentList) < 2; diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index 1aebb0aa1531b..de602fcb564e1 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -272,3 +272,21 @@ func test_variadic_static_member_is_preferred_over_partially_applied_instance_ov let t: Test Test.fn(t) // Ok } + +// Unary unlabeled argument favoring hacks never applied to subscripts + +protocol Subscriptable { +} + +extension Subscriptable { + subscript(key: String) -> Any? { nil } +} + +struct MyValue {} + +extension Dictionary : Subscriptable {} + +func test_that_unary_argument_hacks_do_not_apply_to_subscripts(dict: [String: MyValue]) { + let value = dict["hello"] + let _: MyValue? = value // Ok +} From 1dab584f29c02b0b90030c638778424d77dc49d3 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 1 Mar 2025 20:37:10 -0800 Subject: [PATCH 33/52] [CSOptimizer] Account for speculative scores only when matching operators vs. non-operators The problem this is trying to solve is eager selection of operators over unsupported disjunctions, when matching operators let's take speculative information into account because it helps to make better choices in this case. --- lib/Sema/CSOptimizer.cpp | 81 ++++++++++--------- .../operator_chain_with_closures.swift.gyb | 15 ++++ .../type_checker_perf/fast/rdar31742586.swift | 6 ++ .../type_checker_perf/slow/rdar31742586.swift | 11 --- 4 files changed, 66 insertions(+), 47 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/operator_chain_with_closures.swift.gyb create mode 100644 validation-test/Sema/type_checker_perf/fast/rdar31742586.swift delete mode 100644 validation-test/Sema/type_checker_perf/slow/rdar31742586.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c9315d6b8367a..a03471d66bbe9 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -41,13 +41,20 @@ 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; + + /// Whether the decisions were based on speculative information + /// i.e. literal argument candidates or initializer type inference. + bool IsSpeculative = false; + /// 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) {} + DisjunctionInfo(double score, bool speculative = false, + ArrayRef favoredChoices = {}) + : Score(score), IsSpeculative(speculative), + FavoredChoices(favoredChoices) {} }; static DeclContext *getDisjunctionDC(Constraint *disjunction) { @@ -656,7 +663,7 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( }); return DisjunctionInfo(/*score=*/favoredChoices.empty() ? 0 : 1, - favoredChoices); + /*speculative=*/false, favoredChoices); } } // end anonymous namespace @@ -720,7 +727,8 @@ static void determineBestChoicesInContext( return decl && !decl->getInterfaceType()->is(); }); - recordResult(disjunction, {/*score=*/1.0, favoredChoices}); + recordResult(disjunction, {/*score=*/1.0, /*speculative=*/false, + favoredChoices}); continue; } @@ -760,7 +768,8 @@ static void determineBestChoicesInContext( }); if (!favoredChoices.empty()) { - recordResult(disjunction, {/*score=*/0.01, favoredChoices}); + recordResult(disjunction, + {/*score=*/0.01, /*speculative=*/false, favoredChoices}); continue; } } @@ -1577,41 +1586,24 @@ static void determineBestChoicesInContext( bestOverallScore = std::max(bestOverallScore, bestScore); - DisjunctionInfo info(/*score=*/bestScore); + // Determine if the score and favoring decisions here are + // based only on "speculative" sources i.e. inference from + // literals. + // + // This information is going to be used by the disjunction + // selection algorithm to prevent over-eager selection of + // the operators over unsupported non-operator declarations. + bool isSpeculative = onlySpeculativeArgumentCandidates && + (!canUseContextualResultTypes || + !anyNonSpeculativeResultTypes(resultTypes)); + + DisjunctionInfo info(/*score=*/bestScore, isSpeculative); for (const auto &choice : favoredChoices) { if (choice.second == bestScore) info.FavoredChoices.push_back(choice.first); } - // Reset operator disjunction score but keep favoring - // choices only available candidates where speculative - // with no contextual information available, standard - // comparison operators are a special cases because - // their return type is fixed to `Bool` unlike that of - // bitwise, arithmetic, and other operators. - // - // This helps to prevent over-eager selection of the - // operators over unsupported non-operator declarations. - if (isOperator && onlySpeculativeArgumentCandidates && - (!canUseContextualResultTypes || - !anyNonSpeculativeResultTypes(resultTypes))) { - if (cs.isDebugMode()) { - PrintOptions PO; - PO.PrintTypesForDebugging = true; - - llvm::errs().indent(cs.solverState->getCurrentIndent()) - << "<<< Disjunction " - << disjunction->getNestedConstraints()[0] - ->getFirstType() - ->getString(PO) - << " score " << bestScore - << " was reset due to having only speculative candidates\n"; - } - - info.Score = 0; - } - recordResult(disjunction, std::move(info)); } @@ -1723,8 +1715,25 @@ ConstraintSystem::selectDisjunction() { if (auto preference = isPreferable(*this, first, second)) return preference.value(); - auto &[firstScore, firstFavoredChoices] = favorings[first]; - auto &[secondScore, secondFavoredChoices] = favorings[second]; + auto &[firstScore, isFirstSpeculative, firstFavoredChoices] = + favorings[first]; + auto &[secondScore, isSecondSpeculative, secondFavoredChoices] = + favorings[second]; + + bool isFirstOperator = isOperatorDisjunction(first); + bool isSecondOperator = isOperatorDisjunction(second); + + // Not all of the non-operator disjunctions are supported by the + // ranking algorithm, so to prevent eager selection of operators + // when anything concrete is known about them, let's reset the score + // and compare purely based on number of choices. + if (isFirstOperator != isSecondOperator) { + if (isFirstOperator && isFirstSpeculative) + firstScore = 0; + + if (isSecondOperator && isSecondSpeculative) + secondScore = 0; + } // Rank based on scores only if both disjunctions are supported. if (firstScore && secondScore) { diff --git a/validation-test/Sema/type_checker_perf/fast/operator_chain_with_closures.swift.gyb b/validation-test/Sema/type_checker_perf/fast/operator_chain_with_closures.swift.gyb new file mode 100644 index 0000000000000..2703b185b23f1 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/operator_chain_with_closures.swift.gyb @@ -0,0 +1,15 @@ +// RUN: %scale-test --begin 1 --end 10 --step 1 --select NumLeafScopes %s +// REQUIRES: asserts,no_asan + +struct Test { + var values: [Int] +} + +func test(t: [Test]) { + let _ = 0 + + 1 +%for i in range(1, N): + + 1 +%end + + t.map(\.values.count).reduce(0, +) +} diff --git a/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift b/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift new file mode 100644 index 0000000000000..83a3aa1ffa110 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/rdar31742586.swift @@ -0,0 +1,6 @@ +// RUN: %target-typecheck-verify-swift -solver-scope-threshold=1000 +// REQUIRES: tools-release,no_asan + +func rdar31742586() -> Double { + return -(1 + 2) + -(3 + 4) + 5 - (-(1 + 2) + -(3 + 4) + 5) +} diff --git a/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift b/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift deleted file mode 100644 index 85c705779603c..0000000000000 --- a/validation-test/Sema/type_checker_perf/slow/rdar31742586.swift +++ /dev/null @@ -1,11 +0,0 @@ -// RUN: %target-typecheck-verify-swift -solver-scope-threshold=1000 -// REQUIRES: tools-release,no_asan - -// The test takes about 63k scopes here due to the fact -// that prefix `-` operator doesn't have an overload that -// take `Int`. We could fix that in the stdlib. - -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}} -} From e280569988cc08a068111d540b0e5f20af70153f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 1 Mar 2025 21:31:08 -0800 Subject: [PATCH 34/52] [CSOptimizer] Fix scoring while matching against partially resolved parameter types When matching candidate like `[Int]` against `Array` we need to conservatively assume that if the nominals match the argument is a viable exact match because otherwise it's possible to skip some of the valid matches when other overload choice have generic parameters at the same parameter position. --- lib/Sema/CSOptimizer.cpp | 18 +++++++-- .../custom_array_literal_with_operators.swift | 39 +++++++++++++++++++ 2 files changed, 54 insertions(+), 3 deletions(-) create mode 100644 validation-test/Sema/custom_array_literal_with_operators.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index a03471d66bbe9..0c078f496f2ea 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1354,13 +1354,25 @@ static void 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 system. if (paramType->hasTypeParameter()) { auto *candidateDecl = candidateType->getAnyNominal(); auto *paramDecl = paramType->getAnyNominal(); - if (candidateDecl && paramDecl && candidateDecl == paramDecl) - return 0.8; + // Conservatively we need to make sure that this is not worse + // than matching against a generic parameter with or without + // requirements. + if (candidateDecl && paramDecl && candidateDecl == paramDecl) { + // If the candidate it not yet fully resolved, let's lower the + // score slightly to avoid over-favoring generic overload choices. + if (candidateType->hasTypeVariable()) + return 0.8; + + // If the candidate is fully resolved we need to treat this + // as we would generic parameter otherwise there is a risk + // of skipping some of the valid choices. + return choice->isOperator() ? 0.9 : 1.0; + } } return 0; diff --git a/validation-test/Sema/custom_array_literal_with_operators.swift b/validation-test/Sema/custom_array_literal_with_operators.swift new file mode 100644 index 0000000000000..dd064743356bc --- /dev/null +++ b/validation-test/Sema/custom_array_literal_with_operators.swift @@ -0,0 +1,39 @@ +// RUN: %target-typecheck-verify-swift + +protocol ScalarOrArray { +} + +protocol SpecialValue: ScalarOrArray { +} + +extension Array: ScalarOrArray where Element: SpecialValue { +} + +struct CustomArray : ScalarOrArray { +} + +extension CustomArray : ExpressibleByArrayLiteral { + public init(arrayLiteral elements: Int32...) { + self.init() + } +} + +extension Int32: SpecialValue { +} + +func +(_: T, _: CustomArray) -> CustomArray {} +func +(_: CustomArray, _: T) -> CustomArray {} +func +(_: CustomArray, _: CustomArray) -> CustomArray {} + +extension Sequence where Element == Int { + var asInt32: [Int32] { [] } +} + +extension Array where Element == Int { + var asInt32: [Int32] { [] } +} + +func test(v: Int32, b: [Int]) -> [Int32] { + let result = [1, v - v] + b.asInt32 + return result // Ok +} From 5aa3859f17fa81143e8b560320bb1169b95c7d29 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 3 Mar 2025 11:04:26 -0800 Subject: [PATCH 35/52] [CSOptimizer] Disable unary argument hack if overload set has requirements or variadic overloads This matches the behavior of the old hack where favoring choices were rolled back if `mustConsider` produced `true` which happened only for protocol requirements and variadic overload choice regardless of their viability. --- lib/Sema/CSOptimizer.cpp | 43 +++++++++++++------ .../old_hack_related_ambiguities.swift | 36 ++++++++++++++++ 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 0c078f496f2ea..da8bf783e5339 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -529,6 +529,20 @@ static unsigned numOverloadChoicesMatchingOnArity(OverloadedDeclRefExpr *ODRE, }); } +static bool 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(); + }); +} + /// 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. @@ -542,20 +556,6 @@ static void findFavoredChoicesBasedOnArity( if (numOverloadChoicesMatchingOnArity(ODRE, argumentList) > 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; @@ -591,6 +591,21 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( cs.getParentExpr(argumentList->getUnlabeledUnaryExpr()))) return std::nullopt; + // The hack rolled back favoring choices if one of the overloads was a + // protocol requirement or variadic generic. + // + // Note that it doesn't matter whether such overload choices are viable + // or not, their presence disabled this "optimization". + if (llvm::any_of(disjunction->getNestedConstraints(), [](Constraint *choice) { + auto *decl = getOverloadChoiceDecl(choice); + if (!decl) + return false; + + return isa(decl->getDeclContext()) || + isVariadicGenericOverload(decl); + })) + return std::nullopt; + auto ODRE = isOverloadedDeclRef(disjunction); bool preserveFavoringOfUnlabeledUnaryArgument = !ODRE || numOverloadChoicesMatchingOnArity(ODRE, argumentList) < 2; diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index de602fcb564e1..8ee0a2c47c9fd 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -290,3 +290,39 @@ func test_that_unary_argument_hacks_do_not_apply_to_subscripts(dict: [String: My let value = dict["hello"] let _: MyValue? = value // Ok } + +// Unlabeled unary argument hack was disabled if there were any protocol requirements +// or variadic generic overloads present in the result set (regadless of their viability). +// +// Remove the requirement and variadic overloads and this code would start failing even +// though it shouldn't! + +struct Future { +} + +protocol DB { + func get(_: Int, _: Int) -> Future +} + +extension DB { + func get(_: Int, _: Int = 42) async throws -> Int? { nil } + func get(_: Int) -> Future { .init() } + + func fetch(_: Int, _: Int = 42) async throws -> Int? { nil } + func fetch(_: Int) -> Future { .init() } + func fetch(values: repeat each T) -> Int { 42 } +} + +struct TestUnary { + var db: any DB + + func get(v: Int) async throws { + guard let _ = try await self.db.get(v) else { // Ok + return + } + + guard let _ = try await self.db.fetch(v) else { // Ok + return + } + } +} From 125abede66ff3aaf1417afd57b9322ac4311affa Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 3 Mar 2025 20:07:49 -0800 Subject: [PATCH 36/52] [CSOptimizer] Detect when candidate comes from string interpolation Instead of checking both protocols, check one that matches best depending on where `String` literal candidate came from. --- lib/Sema/CSOptimizer.cpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index da8bf783e5339..8f7b3d47ee7ce 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1070,6 +1070,7 @@ static void determineBestChoicesInContext( Literal = 0x02, ExactOnly = 0x04, DisableCGFloatDoubleConversion = 0x08, + StringInterpolation = 0x10, }; using MatchOptions = OptionSet; @@ -1177,13 +1178,16 @@ static void determineBestChoicesInContext( paramType, KnownProtocolKind::ExpressibleByBooleanLiteral)) return 0.3; - if (candidateType->isString() && - (TypeChecker::conformsToKnownProtocol( - paramType, KnownProtocolKind::ExpressibleByStringLiteral) || - TypeChecker::conformsToKnownProtocol( - paramType, - KnownProtocolKind::ExpressibleByStringInterpolation))) - return 0.3; + if (candidateType->isString()) { + auto literalProtocol = + options.contains(MatchFlag::StringInterpolation) + ? KnownProtocolKind::ExpressibleByStringInterpolation + : KnownProtocolKind::ExpressibleByStringLiteral; + + if (TypeChecker::conformsToKnownProtocol(paramType, + literalProtocol)) + return 0.3; + } auto &ctx = cs.getASTContext(); @@ -1523,6 +1527,11 @@ static void determineBestChoicesInContext( candidate.fromInitializerCall) options |= MatchFlag::DisableCGFloatDoubleConversion; + if (isExpr( + argumentList->getExpr(argIdx) + ->getSemanticsProvidingExpr())) + options |= MatchFlag::StringInterpolation; + // The specifier for a candidate only matters for `inout` check. auto candidateScore = scoreCandidateMatch( genericSig, decl, candidate.type->getWithoutSpecifierType(), From e7b351d41d2e2f9b3ac1936b691f05bb0e8c5961 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 3 Mar 2025 20:39:17 -0800 Subject: [PATCH 37/52] [CSOptimizer] NFC: Use builder pattern to construct `DisjunctionInfo` --- lib/Sema/CSOptimizer.cpp | 95 ++++++++++++++++++++++++++++------------ 1 file changed, 66 insertions(+), 29 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 8f7b3d47ee7ce..d5d749398bc2b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -42,19 +42,50 @@ struct DisjunctionInfo { /// If the score is nullopt it means that the disjunction is not optimizable. std::optional Score; - /// Whether the decisions were based on speculative information - /// i.e. literal argument candidates or initializer type inference. - bool IsSpeculative = false; - /// The highest scoring choices that could be favored when disjunction /// is attempted. llvm::TinyPtrVector FavoredChoices; + /// Whether the decisions were based on speculative information + /// i.e. literal argument candidates or initializer type inference. + bool IsSpeculative; + DisjunctionInfo() = default; - DisjunctionInfo(double score, bool speculative = false, - ArrayRef favoredChoices = {}) - : Score(score), IsSpeculative(speculative), - FavoredChoices(favoredChoices) {} + DisjunctionInfo(std::optional score, + ArrayRef favoredChoices, bool speculative) + : Score(score), FavoredChoices(favoredChoices), + IsSpeculative(speculative) {} + + static DisjunctionInfo none() { return {std::nullopt, {}, false}; } +}; + +class DisjunctionInfoBuilder { + std::optional Score; + SmallVector FavoredChoices; + bool IsSpeculative; + +public: + DisjunctionInfoBuilder(std::optional score) + : DisjunctionInfoBuilder(score, {}) {} + + DisjunctionInfoBuilder(std::optional score, + ArrayRef favoredChoices) + : Score(score), + FavoredChoices(favoredChoices.begin(), favoredChoices.end()), + IsSpeculative(false) {} + + void setFavoredChoices(ArrayRef choices) { + FavoredChoices.clear(); + FavoredChoices.append(choices.begin(), choices.end()); + } + + void addFavoredChoice(Constraint *constraint) { + FavoredChoices.push_back(constraint); + } + + void setSpeculative(bool value = true) { IsSpeculative = value; } + + DisjunctionInfo build() { return {Score, FavoredChoices, IsSpeculative}; } }; static DeclContext *getDisjunctionDC(Constraint *disjunction) { @@ -624,7 +655,7 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( isExpr(argument) || isExpr(argument) || isExpr(argument) || isExpr(argument))) - return {/*score=*/0}; + return DisjunctionInfo::none(); auto argumentType = cs.getType(argument)->getRValueType(); @@ -633,7 +664,7 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( if (isa(argument)) { auto chainTy = inferTypeOfArithmeticOperatorChain(cs.DC, argument); if (!chainTy) - return {/*score=*/0}; + return DisjunctionInfo::none(); argumentType = chainTy; } @@ -643,11 +674,11 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( if (auto *LE = dyn_cast(argument)) { auto *P = TypeChecker::getLiteralProtocol(cs.getASTContext(), LE); if (!P) - return {/*score=*/0}; + return DisjunctionInfo::none(); auto defaultTy = TypeChecker::getDefaultType(P, cs.DC); if (!defaultTy) - return {/*score=*/0}; + return DisjunctionInfo::none(); argumentType = defaultTy; } @@ -655,7 +686,7 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( ASSERT(argumentType); if (argumentType->hasTypeVariable() || argumentType->hasDependentMember()) - return {/*score=*/0}; + return DisjunctionInfo::none(); SmallVector favoredChoices; forEachDisjunctionChoice( @@ -677,8 +708,9 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( favoredChoices.push_back(choice); }); - return DisjunctionInfo(/*score=*/favoredChoices.empty() ? 0 : 1, - /*speculative=*/false, favoredChoices); + return DisjunctionInfoBuilder(/*score=*/favoredChoices.empty() ? 0 : 1, + favoredChoices) + .build(); } } // end anonymous namespace @@ -703,11 +735,12 @@ static void determineBestChoicesInContext( // 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)); + DisjunctionInfoBuilder info(/*score=*/2.0); + for (auto *choice : disjunction->getNestedConstraints()) { + if (choice->isFavored()) + info.addFavoredChoice(choice); + } + recordResult(disjunction, info.build()); continue; } @@ -742,8 +775,9 @@ static void determineBestChoicesInContext( return decl && !decl->getInterfaceType()->is(); }); - recordResult(disjunction, {/*score=*/1.0, /*speculative=*/false, - favoredChoices}); + recordResult( + disjunction, + DisjunctionInfoBuilder(/*score=*/1.0, favoredChoices).build()); continue; } @@ -783,8 +817,9 @@ static void determineBestChoicesInContext( }); if (!favoredChoices.empty()) { - recordResult(disjunction, - {/*score=*/0.01, /*speculative=*/false, favoredChoices}); + recordResult( + disjunction, + DisjunctionInfoBuilder(/*score=*/0.01, favoredChoices).build()); continue; } } @@ -1633,14 +1668,16 @@ static void determineBestChoicesInContext( (!canUseContextualResultTypes || !anyNonSpeculativeResultTypes(resultTypes)); - DisjunctionInfo info(/*score=*/bestScore, isSpeculative); + DisjunctionInfoBuilder info(/*score=*/bestScore); + + info.setSpeculative(isSpeculative); for (const auto &choice : favoredChoices) { if (choice.second == bestScore) - info.FavoredChoices.push_back(choice.first); + info.addFavoredChoice(choice.first); } - recordResult(disjunction, std::move(info)); + recordResult(disjunction, info.build()); } if (cs.isDebugMode() && bestOverallScore > 0) { @@ -1751,9 +1788,9 @@ ConstraintSystem::selectDisjunction() { if (auto preference = isPreferable(*this, first, second)) return preference.value(); - auto &[firstScore, isFirstSpeculative, firstFavoredChoices] = + auto &[firstScore, firstFavoredChoices, isFirstSpeculative] = favorings[first]; - auto &[secondScore, isSecondSpeculative, secondFavoredChoices] = + auto &[secondScore, secondFavoredChoices, isSecondSpeculative] = favorings[second]; bool isFirstOperator = isOperatorDisjunction(first); From 9e97b8e3a1c5a8be8ee8a2773ad4f326294b9d2e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 10 Mar 2025 14:30:26 -0700 Subject: [PATCH 38/52] [CSOptimizer] Skip disfavored choices when they would result in a worse solution --- include/swift/Sema/ConstraintSystem.h | 6 ++++++ lib/Sema/CSStep.cpp | 27 ++++++++++++++++++++++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 251f6d7b53b29..9df425412ccd1 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -6114,6 +6114,12 @@ class DisjunctionChoice { return false; } + bool isDisfavored() const { + if (auto *decl = getOverloadChoiceDecl(Choice)) + return decl->getAttrs().hasAttribute(); + return false; + } + bool isBeginningOfPartition() const { return IsBeginningOfPartition; } // FIXME: All three of the accessors below are required to support diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index bf6be9e02ff47..45c2e10d20bd3 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -635,9 +635,30 @@ bool DisjunctionStep::shouldSkip(const DisjunctionChoice &choice) const { if (choice.isDisabled()) return skip("disabled"); - // Skip unavailable overloads (unless in diagnostic mode). - if (choice.isUnavailable() && !CS.shouldAttemptFixes()) - return skip("unavailable"); + if (!CS.shouldAttemptFixes()) { + // Skip unavailable overloads. + if (choice.isUnavailable()) + return skip("unavailable"); + + // Since the disfavored overloads are always located at the end of + // the partition they could be skipped if there was at least one + // valid solution for this partition already, because the solution + // they produce would always be worse. + if (choice.isDisfavored() && LastSolvedChoice) { + bool canSkipDisfavored = true; + auto &lastScore = LastSolvedChoice->second; + for (unsigned i = 0, n = unsigned(SK_DisfavoredOverload) + 1; i != n; + ++i) { + if (lastScore.Data[i] > 0) { + canSkipDisfavored = false; + break; + } + } + + if (canSkipDisfavored) + return skip("disfavored"); + } + } // If the solver already found a solution with a better overload choice that // can be unconditionally substituted by the current choice, skip the current From db0a9de996fdbf48a974f0e8d6be72607b23bcf7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 11 Mar 2025 14:07:00 -0700 Subject: [PATCH 39/52] [CSOptimizer] Account for the fact that sometimes all initializer choices are failable If all of the viable initializer overloads are failable, the only valid inference choice is an optional candidate type. --- lib/Sema/CSOptimizer.cpp | 103 +++++++++++------- .../old_hack_related_ambiguities.swift | 34 ++++++ 2 files changed, 99 insertions(+), 38 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index d5d749398bc2b..1331406d5c9c5 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -344,6 +344,43 @@ static bool isSupportedDisjunction(Constraint *disjunction) { }); } +/// Determine whether the given overload choice constitutes a +/// valid choice that would be attempted during normal solving +/// without any score increases. +static ValueDecl *isViableOverloadChoice(ConstraintSystem &cs, + Constraint *constraint, + ConstraintLocator *locator) { + if (constraint->isDisabled()) + return nullptr; + + if (constraint->getKind() != ConstraintKind::BindOverload) + return nullptr; + + auto choice = constraint->getOverloadChoice(); + auto *decl = choice.getDeclOrNull(); + if (!decl) + return nullptr; + + // 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)) + return nullptr; + } + } + + // If disjunction choice is unavailable or disfavored we cannot + // do anything with it. + if (decl->getAttrs().hasAttribute() || + cs.isDeclUnavailable(decl, locator)) + return nullptr; + + return decl; +} + /// Given the type variable that represents a result type of a /// function call, check whether that call is to an initializer /// and based on that deduce possible type for the result. @@ -389,16 +426,30 @@ inferTypeFromInitializerResultType(ConstraintSystem &cs, if (initRef == disjunctions.end()) return {}; - bool hasFailable = - llvm::any_of((*initRef)->getNestedConstraints(), [](Constraint *choice) { - if (choice->isDisabled()) - return false; - auto *decl = - dyn_cast_or_null(getOverloadChoiceDecl(choice)); - return decl && decl->isFailable(); - }); + unsigned numFailable = 0; + unsigned total = 0; + for (auto *choice : (*initRef)->getNestedConstraints()) { + auto *decl = isViableOverloadChoice(cs, choice, ctorLocator); + if (!decl || !isa(decl)) + continue; - return {instanceTy, hasFailable}; + auto *ctor = cast(decl); + if (ctor->isFailable()) + ++numFailable; + + ++total; + } + + if (numFailable > 0) { + // If all of the active choices are failable, produce an optional + // type only. + if (numFailable == total) + return {instanceTy->wrapInOptionalType(), /*hasFailable=*/false}; + // Otherwise there are two options. + return {instanceTy, /*hasFailable*/ true}; + } + + return {instanceTy, /*hasFailable=*/false}; } /// If the given expression represents a chain of operators that only have @@ -502,38 +553,14 @@ void forEachDisjunctionChoice( 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(); + auto *decl = + isViableOverloadChoice(cs, constraint, disjunction->getLocator()); 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->getDeclContext()) { - if (!useDC->isDeclImported(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); + Type overloadType = cs.getEffectiveOverloadType( + disjunction->getLocator(), constraint->getOverloadChoice(), + /*allowMembers=*/true, constraint->getOverloadUseDC()); if (!overloadType || !overloadType->is()) continue; diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index 8ee0a2c47c9fd..40cb73cf45146 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -326,3 +326,37 @@ struct TestUnary { } } } + +// Prevent non-optional overload of `??` to be favored when all initializers are failable. + +class A {} +class B {} + +protocol P { + init() +} + +extension P { + init?(v: A) { self.init() } +} + +struct V : P { + init() {} + + @_disfavoredOverload + init?(v: B?) {} + + // Important to keep this to make sure that disabled constraints + // are handled properly. + init(other: T) where T.Element == Character {} +} + +class TestFailableOnly { + var v: V? + + func test(defaultB: B) { + guard let _ = self.v ?? V(v: defaultB) else { // OK (no warnings) + return + } + } +} From 3616dabda73bb2798505ea26ceada9cd6711b8cd Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 11 Mar 2025 14:11:55 -0700 Subject: [PATCH 40/52] [Tests] NFC: Move Double/CGFloat test because it tests local type method which is printed at the end --- .../implicit_double_cgfloat_conversion.swift | 41 +++++++++++++------ 1 file changed, 28 insertions(+), 13 deletions(-) diff --git a/test/Constraints/implicit_double_cgfloat_conversion.swift b/test/Constraints/implicit_double_cgfloat_conversion.swift index 0f4266ef5fa7d..1915540fdd571 100644 --- a/test/Constraints/implicit_double_cgfloat_conversion.swift +++ b/test/Constraints/implicit_double_cgfloat_conversion.swift @@ -337,19 +337,6 @@ func test_implicit_conversion_clash_with_partial_application_check() { } } -// rdar://99352676 -// CHECK-LABEL: sil hidden [ossa] @$s34implicit_double_cgfloat_conversion20test_init_validationyyF : $@convention(thin) () -> () { -func test_init_validation() { - class Foo { - static let bar = 100.0 - - func getBar() -> CGFloat? { - return Self.bar - // CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcfC : $@convention(method) (Double, @thin CGFloat.Type) -> CGFloat - } - } -} - func test_ternary_and_nil_coalescing() { func test(_: Double?) {} @@ -389,3 +376,31 @@ 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 +} + +// rdar://99352676 +// CHECK-LABEL: sil private [ossa] @$s34implicit_double_cgfloat_conversion20test_init_validationyyF3FooL_C6getBar12CoreGraphics7CGFloatVSgyF : $@convention(method) (@guaranteed Foo) -> Optional +func test_init_validation() { + class Foo { + static let bar = 100.0 + + func getBar() -> CGFloat? { + return Self.bar + // CHECK: function_ref @$s12CoreGraphics7CGFloatVyACSdcfC : $@convention(method) (Double, @thin CGFloat.Type) -> CGFloat + } + } +} From b90fc2b8c6e68399b2ac63cc5e8beb17f939a5a9 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 13 Mar 2025 09:31:05 -0700 Subject: [PATCH 41/52] [CSOptimizer] Don't attempt to walk into literals while analyzing operator chains Most of them don't have any children but string interpolation does and that marks whole chain as unsupported. --- lib/Sema/CSOptimizer.cpp | 4 ++- .../fast/string_interpolations.swift.gyb | 26 +++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/string_interpolations.swift.gyb diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 1331406d5c9c5..e18541d80747d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -500,7 +500,9 @@ static Type inferTypeOfArithmeticOperatorChain(DeclContext *dc, ASTNode node) { } literals.insert(defaultTy); - return Action::Continue(expr); + // String interpolation expressions have `TapExpr` + // as their children, no reason to walk them. + return Action::SkipChildren(expr); } } } diff --git a/validation-test/Sema/type_checker_perf/fast/string_interpolations.swift.gyb b/validation-test/Sema/type_checker_perf/fast/string_interpolations.swift.gyb new file mode 100644 index 0000000000000..fd54e39b609c7 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/string_interpolations.swift.gyb @@ -0,0 +1,26 @@ +// RUN: %scale-test --begin 1 --end 10 --step 1 --select NumLeafScopes %s -Xfrontend=-typecheck +// REQUIRES: asserts,no_asan + +struct MyString { +} + +extension MyString: ExpressibleByStringLiteral { + init(stringLiteral value: String) {} +} + +func +(_: MyString, _: MyString) -> MyString {} + +func test(_: @autoclosure () -> String) {} +func test(_: () -> String) {} +func test(_: Character) {} + +func test(names: [String]) { + var resultString = "" + for name in names { + test("\(name)" +%for i in range(0, N): + + "\(name)" +%end + + "\(name)") + } +} From ea47e3cc24aa8662478c48275e279631a6b76156 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 14 Mar 2025 17:32:12 -0700 Subject: [PATCH 42/52] [CSOptimizer] NFC: Adopt new `Constraint` and `ConstraintGraph` APIs --- lib/Sema/CSOptimizer.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index e18541d80747d..b980124e6f0b3 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -92,11 +92,10 @@ static DeclContext *getDisjunctionDC(Constraint *disjunction) { auto *choice = disjunction->getNestedConstraints()[0]; switch (choice->getKind()) { case ConstraintKind::BindOverload: - return choice->getOverloadUseDC(); case ConstraintKind::ValueMember: case ConstraintKind::UnresolvedValueMember: case ConstraintKind::ValueWitness: - return choice->getMemberUseDC(); + return choice->getDeclContext(); default: return nullptr; } @@ -366,7 +365,7 @@ static ValueDecl *isViableOverloadChoice(ConstraintSystem &cs, // 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 (auto *useDC = constraint->getDeclContext()) { if (!useDC->isDeclImported(decl)) return nullptr; } @@ -562,7 +561,7 @@ void forEachDisjunctionChoice( Type overloadType = cs.getEffectiveOverloadType( disjunction->getLocator(), constraint->getOverloadChoice(), - /*allowMembers=*/true, constraint->getOverloadUseDC()); + /*allowMembers=*/true, constraint->getDeclContext()); if (!overloadType || !overloadType->is()) continue; From eb78e27a08b1caa698f9a33a2222a09a15327a8f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 14 Mar 2025 18:11:26 -0700 Subject: [PATCH 43/52] [CSOptimizer] Consider choices marked as `@_disfavoredOverload` These choices could be better than some other non-disfavored ones in certain situations i.e. when `async` overload is disfavored but appears in async context it's preferrable to a non-async overload choice. Note that the code that mimic old hacks still needs to filter on `@_disfavoredOverload` in few places to maintain source compatibility. --- lib/Sema/CSOptimizer.cpp | 18 +++++++++++---- .../old_hack_related_ambiguities.swift | 23 +++++++++++++++++++ 2 files changed, 36 insertions(+), 5 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index b980124e6f0b3..2723ab620289b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -371,10 +371,9 @@ static ValueDecl *isViableOverloadChoice(ConstraintSystem &cs, } } - // If disjunction choice is unavailable or disfavored we cannot + // If disjunction choice is unavailable we cannot // do anything with it. - if (decl->getAttrs().hasAttribute() || - cs.isDeclUnavailable(decl, locator)) + if (cs.isDeclUnavailable(decl, locator)) return nullptr; return decl; @@ -621,8 +620,13 @@ static void findFavoredChoicesBasedOnArity( forEachDisjunctionChoice( cs, disjunction, [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { - if (isVariadicGenericOverload(decl)) + if (decl->getAttrs().hasAttribute()) + return; + + if (isVariadicGenericOverload(decl)) { hasVariadicGenerics = true; + return; + } if (overloadType->getNumParams() == argumentList->size() || llvm::count_if(*decl->getParameterList(), [](auto *param) { @@ -661,7 +665,8 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( return false; return isa(decl->getDeclContext()) || - isVariadicGenericOverload(decl); + (!decl->getAttrs().hasAttribute() && + isVariadicGenericOverload(decl)); })) return std::nullopt; @@ -721,6 +726,9 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( cs, disjunction, [&argumentType, &favoredChoices, &argument]( Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { + if (decl->getAttrs().hasAttribute()) + return; + if (overloadType->getNumParams() != 1) return; diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index 40cb73cf45146..d94dc86cc3ae3 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -360,3 +360,26 @@ class TestFailableOnly { } } } + +do { + @_disfavoredOverload + func test(over: Int, that: String = "", block: @escaping (Int) throws -> Void) async throws {} + func test(over: Int, that: String = "", block: @escaping (Int) throws -> Void) throws {} // expected-note {{found this candidate}} + func test(over: Int, other: String = "", block: @escaping (Int) throws -> Void) throws {} // expected-note {{found this candidate}} + + func performLocal(v: Int, block: @escaping (Int) throws -> Void) async throws { + try await test(over: v, block: block) // expected-error {{ambiguous use of 'test'}} + } + + // The hack applied only to `OverloadedDeclRefExpr`s. + struct MemberTest { + @_disfavoredOverload + func test(over: Int, that: String = "", block: @escaping (Int) throws -> Void) async throws {} + func test(over: Int, that: String = "", block: @escaping (Int) throws -> Void) throws {} + func test(over: Int, other: String = "", block: @escaping (Int) throws -> Void) throws {} + + func performMember(v: Int, block: @escaping (Int) throws -> Void) async throws { + try await test(over: v, block: block) // Ok + } + } +} From 8037bbced2141d41cd7b59a6363d7186c34260ac Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sun, 20 Apr 2025 20:44:32 -0700 Subject: [PATCH 44/52] [CSOptimizer] NFC: Adopt to changes in `getParameterList()` API --- 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 2723ab620289b..0c7dd52f509ae 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -287,7 +287,7 @@ static bool isSupportedGenericOverloadChoice(ValueDecl *decl, // that use only concrete types or generic parameters directly // in their parameter positions i.e. `(T, Int)`. - auto *paramList = getParameterList(decl); + auto *paramList = decl->getParameterList(); if (!paramList) return false; From 479e61b020c3260035fd6bcc69d02b14edfaff39 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 17 May 2025 00:39:28 -0700 Subject: [PATCH 45/52] [CSOptimizer] Adopt to removal of `TypeBase::isArrayType()` by switching to `isArray()` --- 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 0c7dd52f509ae..ca9990e36657c 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1195,7 +1195,7 @@ static void determineBestChoicesInContext( // since everything else is going to increase to score. if (options.contains(MatchFlag::Literal)) { if (isUnboundArrayType(candidateType)) - return paramType->isArrayType() ? 0.3 : 0; + return paramType->isArray() ? 0.3 : 0; if (isUnboundDictionaryType(candidateType)) return cs.isDictionaryType(paramType) ? 0.3 : 0; From 2520d40575b497e7afb36fd0c1d8cae5120c57f3 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 12 Jun 2025 22:19:27 -0700 Subject: [PATCH 46/52] [CSOptimizer] Allow literal inference inside of operator chains This is helpful in situations when all of the chained operators have literal arguments because it would make sure that every operator has the same score if there is no contextual type. --- lib/Sema/CSOptimizer.cpp | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index ca9990e36657c..b5650038e7c40 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -450,7 +450,7 @@ inferTypeFromInitializerResultType(ConstraintSystem &cs, return {instanceTy, /*hasFailable=*/false}; } -/// If the given expression represents a chain of operators that only have +/// If the given expression represents a chain of operators that have /// only literals as arguments, attempt to deduce a potential type of the /// chain. For example if chain has only integral literals it's going to /// be `Int`, if there are some floating-point literals mixed in - it's going @@ -1007,13 +1007,9 @@ static void determineBestChoicesInContext( if (typeVar->getImpl().isFunctionResult()) { auto *resultLoc = typeVar->getImpl().getLocator(); - // We don't want to try and infer parts of operator - // chains. - if (!isOperator) { - if (auto type = inferTypeOfArithmeticOperatorChain( - cs.DC, resultLoc->getAnchor())) { - types.push_back({type, /*fromLiteral=*/true}); - } + if (auto type = inferTypeOfArithmeticOperatorChain( + cs.DC, resultLoc->getAnchor())) { + types.push_back({type, /*fromLiteral=*/true}); } auto binding = From ef2be0d3f575867b64e1b0b7ba8fd9cf397afa8f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 12 Jun 2025 22:23:31 -0700 Subject: [PATCH 47/52] [CSOptimizer] Rank operators with the same score based on speculative status If the scores are the same and both disjunctions are operators they could be ranked purely based on whether the candidates were speculative or not. The one with more context always wins. Consider the following situation: ```swift func test(_: Int) { ... } func test(_: String) { ... } test("a" + "b" + "c") ``` In this case we should always prefer ... + "c" over "a" + "b" because it would fail and prune the other overload if parameter type (aka contextual type) is `Int`. --- lib/Sema/CSOptimizer.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index b5650038e7c40..ad15441eb71d1 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1846,6 +1846,24 @@ ConstraintSystem::selectDisjunction() { // based on number of favored/active choices. if (*firstScore != *secondScore) return *firstScore > *secondScore; + + // If the scores are the same and both disjunctions are operators + // they could be ranked purely based on whether the candidates + // were speculative or not. The one with more context always wins. + // + // Consider the following situation: + // + // func test(_: Int) { ... } + // func test(_: String) { ... } + // + // test("a" + "b" + "c") + // + // In this case we should always prefer ... + "c" over "a" + "b" + // because it would fail and prune the other overload if parameter + // type (aka contextual type) is `Int`. + if (isFirstOperator && isSecondOperator && + isFirstSpeculative != isSecondSpeculative) + return isSecondSpeculative; } // Use favored choices only if disjunction score is higher From 979e0469e01e978abeeb4daa2a45a2b98f6f374d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 16 Jun 2025 00:13:27 -0700 Subject: [PATCH 48/52] [CSOptimizer] Rework `inferTypeOfArithmeticOperatorChain` - Expand the inference to include prefix and postfix unary operators - Recognize previously resolved declaration and member references in argument positions and record their types. - Expand reconciliation logic from Double<->Int to include other floating-point types and `CGFloat`. --- lib/Sema/CSOptimizer.cpp | 84 ++++++++++++++++++++++++++-------------- 1 file changed, 56 insertions(+), 28 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index ad15441eb71d1..5f02864239c16 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -451,20 +451,18 @@ inferTypeFromInitializerResultType(ConstraintSystem &cs, } /// If the given expression represents a chain of operators that have -/// only literals as arguments, attempt to deduce a potential type of the -/// chain. For example if chain has only integral literals it's going to -/// be `Int`, if there are some floating-point literals mixed in - it's going -/// to be `Double`. -static Type inferTypeOfArithmeticOperatorChain(DeclContext *dc, ASTNode node) { - auto binaryOp = getAsExpr(node); - if (!binaryOp) - return Type(); - +/// only declaration/member references and/or literals as arguments, +/// attempt to deduce a potential type of the chain. For example if +/// chain has only integral literals it's going to be `Int`, if there +/// are some floating-point literals mixed in - it's going to be `Double`. +static Type inferTypeOfArithmeticOperatorChain(ConstraintSystem &cs, + ASTNode node) { class OperatorChainAnalyzer : public ASTWalker { ASTContext &C; DeclContext *DC; + ConstraintSystem &CS; - llvm::SmallPtrSet literals; + llvm::SmallPtrSet, 2> candidates; bool unsupported = false; @@ -472,6 +470,9 @@ static Type inferTypeOfArithmeticOperatorChain(DeclContext *dc, ASTNode node) { if (isa(expr)) return Action::Continue(expr); + if (isa(expr) || isa(expr)) + return Action::Continue(expr); + if (isa(expr)) return Action::Continue(expr); @@ -487,17 +488,7 @@ static Type inferTypeOfArithmeticOperatorChain(DeclContext *dc, ASTNode node) { if (auto *LE = dyn_cast(expr)) { if (auto *P = TypeChecker::getLiteralProtocol(C, LE)) { if (auto defaultTy = TypeChecker::getDefaultType(P, DC)) { - if (defaultTy->isInt()) { - // Don't add `Int` if `Double` is already in the list. - if (literals.contains(C.getDoubleType())) - return Action::Continue(expr); - } else if (defaultTy->isDouble()) { - // A single use of a floating-point literal flips the - // type of the entire chain to `Double`. - (void)literals.erase(C.getIntType()); - } - - literals.insert(defaultTy); + addCandidateType(defaultTy, /*literal=*/true); // String interpolation expressions have `TapExpr` // as their children, no reason to walk them. return Action::SkipChildren(expr); @@ -505,22 +496,59 @@ static Type inferTypeOfArithmeticOperatorChain(DeclContext *dc, ASTNode node) { } } + if (auto *UDE = dyn_cast(expr)) { + auto memberTy = CS.getType(UDE); + if (!memberTy->hasTypeVariable()) { + addCandidateType(memberTy, /*literal=*/false); + return Action::SkipChildren(expr); + } + } + + if (auto *DRE = dyn_cast(expr)) { + auto declTy = CS.getType(DRE); + if (!declTy->hasTypeVariable()) { + addCandidateType(declTy, /*literal=*/false); + return Action::SkipChildren(expr); + } + } + unsupported = true; return Action::Stop(); } + void addCandidateType(Type type, bool literal) { + if (literal) { + if (type->isInt()) { + // Floating-point types always subsume Int in operator chains. + if (llvm::any_of(candidates, [](const auto &candidate) { + auto ty = candidate.getPointer(); + return isFloatType(ty) || ty->isCGFloat(); + })) + return; + } else if (isFloatType(type) || type->isCGFloat()) { + // A single use of a floating-point literal flips the + // type of the entire chain to it. + (void)candidates.erase({C.getIntType(), /*literal=*/true}); + } + } + + candidates.insert({type, literal}); + } + public: - OperatorChainAnalyzer(DeclContext *DC) : C(DC->getASTContext()), DC(DC) {} + OperatorChainAnalyzer(ConstraintSystem &CS) + : C(CS.getASTContext()), DC(CS.DC), CS(CS) {} Type chainType() const { if (unsupported) return Type(); - return literals.size() != 1 ? Type() : *literals.begin(); + return candidates.size() != 1 ? Type() + : (*candidates.begin()).getPointer(); } }; - OperatorChainAnalyzer analyzer(dc); - binaryOp->walk(analyzer); + OperatorChainAnalyzer analyzer(cs); + node.walk(analyzer); return analyzer.chainType(); } @@ -695,7 +723,7 @@ static std::optional preserveFavoringOfUnlabeledUnaryArgument( // For chains like `1 + 2 * 3` it's easy to deduce the type because // we know what literal types are preferred. if (isa(argument)) { - auto chainTy = inferTypeOfArithmeticOperatorChain(cs.DC, argument); + auto chainTy = inferTypeOfArithmeticOperatorChain(cs, argument); if (!chainTy) return DisjunctionInfo::none(); @@ -1008,7 +1036,7 @@ static void determineBestChoicesInContext( auto *resultLoc = typeVar->getImpl().getLocator(); if (auto type = inferTypeOfArithmeticOperatorChain( - cs.DC, resultLoc->getAnchor())) { + cs, resultLoc->getAnchor())) { types.push_back({type, /*fromLiteral=*/true}); } @@ -1830,7 +1858,7 @@ ConstraintSystem::selectDisjunction() { // Not all of the non-operator disjunctions are supported by the // ranking algorithm, so to prevent eager selection of operators - // when anything concrete is known about them, let's reset the score + // when nothing concrete is known about them, let's reset the score // and compare purely based on number of choices. if (isFirstOperator != isSecondOperator) { if (isFirstOperator && isFirstSpeculative) From eee40b4ecc882f4c6a985e32608860790828b759 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 23 Jun 2025 00:01:45 -0700 Subject: [PATCH 49/52] [CSGen] Make collection subscript result type inference more principled Infer result type of a subscript with Array or Dictionary base type if argument type matches the key type exactly or it's a supported literal type. This helps to maintain the existing behavior without having to resort to "favored type" computation. --- lib/Sema/CSGen.cpp | 103 ++++++++++-------- .../collection_subscript_chains.swift.gyb | 13 +++ 2 files changed, 71 insertions(+), 45 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/collection_subscript_chains.swift.gyb diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index ea9e4119faf3e..cc165c6876cbe 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -1051,6 +1051,61 @@ namespace { return tv; } + /// Attempt to infer a result type of a subscript reference where + /// the base type is either a stdlib Array or a Dictionary type. + /// This is a more principled version of the old performance hack + /// that used "favored" types deduced by the constraint optimizer + /// and is important to maintain pre-existing solver behavior. + Type inferCollectionSubscriptResultType(Type baseTy, + ArgumentList *argumentList) { + auto isLValueBase = false; + auto baseObjTy = baseTy; + if (baseObjTy->is()) { + isLValueBase = true; + baseObjTy = baseObjTy->getWithoutSpecifierType(); + } + + auto subscriptResultType = [&isLValueBase](Type valueTy, + bool isOptional) -> Type { + Type outputTy = isOptional ? OptionalType::get(valueTy) : valueTy; + return isLValueBase ? LValueType::get(outputTy) : outputTy; + }; + + if (auto *argument = argumentList->getUnlabeledUnaryExpr()) { + auto argumentTy = CS.getType(argument); + + auto elementTy = baseObjTy->getArrayElementType(); + + if (!elementTy) + elementTy = baseObjTy->getInlineArrayElementType(); + + if (elementTy) { + if (auto arraySliceTy = + dyn_cast(baseObjTy.getPointer())) { + baseObjTy = arraySliceTy->getDesugaredType(); + } + + if (argumentTy->isInt() || isExpr(argument)) + return subscriptResultType(elementTy, /*isOptional*/ false); + } else if (auto dictTy = CS.isDictionaryType(baseObjTy)) { + auto [keyTy, valueTy] = *dictTy; + + if (keyTy->isString() && + (isExpr(argument) || + isExpr(argument))) + return subscriptResultType(valueTy, /*isOptional*/ true); + + if (keyTy->isInt() && isExpr(argument)) + return subscriptResultType(valueTy, /*isOptional*/ true); + + if (keyTy->isEqual(argumentTy)) + return subscriptResultType(valueTy, /*isOptional*/ true); + } + } + + return Type(); + } + /// Add constraints for a subscript operation. Type addSubscriptConstraints( Expr *anchor, Type baseTy, ValueDecl *declOrNull, ArgumentList *argList, @@ -1074,52 +1129,10 @@ namespace { Type outputTy; - // For an integer subscript expression on an array slice type, instead of - // introducing a new type variable we can easily obtain the element type. - if (isa(anchor)) { - - auto isLValueBase = false; - auto baseObjTy = baseTy; - if (baseObjTy->is()) { - isLValueBase = true; - baseObjTy = baseObjTy->getWithoutSpecifierType(); - } - - auto elementTy = baseObjTy->getArrayElementType(); + // Attempt to infer the result type of a stdlib collection subscript. + if (isa(anchor)) + outputTy = inferCollectionSubscriptResultType(baseTy, argList); - if (!elementTy) - elementTy = baseObjTy->getInlineArrayElementType(); - - if (elementTy) { - - if (auto arraySliceTy = - dyn_cast(baseObjTy.getPointer())) { - baseObjTy = arraySliceTy->getDesugaredType(); - } - - if (argList->isUnlabeledUnary() && - isa(argList->getExpr(0))) { - - outputTy = elementTy; - - 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); - } - } - } - } - if (outputTy.isNull()) { outputTy = CS.createTypeVariable(resultLocator, TVO_CanBindToLValue | TVO_CanBindToNoEscape); diff --git a/validation-test/Sema/type_checker_perf/fast/collection_subscript_chains.swift.gyb b/validation-test/Sema/type_checker_perf/fast/collection_subscript_chains.swift.gyb new file mode 100644 index 0000000000000..b6c74599d5707 --- /dev/null +++ b/validation-test/Sema/type_checker_perf/fast/collection_subscript_chains.swift.gyb @@ -0,0 +1,13 @@ +// RUN: %scale-test --begin 1 --end 15 --step 1 --select NumLeafScopes %s --expected-exit-code 0 +// REQUIRES: asserts,no_asan + +func test(carrierDict: [String : Double]) { + var exhaustTemperature: Double + exhaustTemperature = ( + (carrierDict[""] ?? 0.0) + +%for i in range(N): + (carrierDict[""] ?? 0.0) + +%end + (carrierDict[""] ?? 0.0) + ) / 4 +} From 3efb948ad8d7d8f09e45e1629f0f47f99c921f76 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 27 Jun 2025 11:06:52 -0700 Subject: [PATCH 50/52] [ConstraintSystem] Add a option to re-enable performance hacks Instead of checking `EnableConstraintSolverPerformanceHacks` directly, let's use an option which is easier to set i.e. when a block list is implemented. --- include/swift/Sema/ConstraintSystem.h | 8 +++++--- lib/Sema/TypeCheckConstraints.cpp | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 9df425412ccd1..abb99fe9a1d21 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1964,6 +1964,9 @@ enum class ConstraintSystemFlags { /// Disable macro expansions. DisableMacroExpansions = 0x80, + + /// Enable old type-checker performance hacks. + EnablePerformanceHacks = 0x100, }; /// Options that affect the constraint system as a whole. @@ -3604,10 +3607,9 @@ class ConstraintSystem { } /// Check whether old type-checker performance hacks has been explicitly - /// enabled by a flag. + /// enabled. bool performanceHacksEnabled() const { - return getASTContext() - .TypeCheckerOpts.EnableConstraintSolverPerformanceHacks; + return Options.contains(ConstraintSystemFlags::EnablePerformanceHacks); } /// Log and record the application of the fix. Return true iff any diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 78f49d3de9ca7..c1bdf3c88bb63 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -466,6 +466,9 @@ TypeChecker::typeCheckTarget(SyntacticElementTarget &target, if (options.contains(TypeCheckExprFlags::DisableMacroExpansions)) csOptions |= ConstraintSystemFlags::DisableMacroExpansions; + if (Context.TypeCheckerOpts.EnableConstraintSolverPerformanceHacks) + csOptions |= ConstraintSystemFlags::EnablePerformanceHacks; + ConstraintSystem cs(dc, csOptions, diagnosticTransaction); if (auto *expr = target.getAsExpr()) { From c1e0e6622d2a4c9bbf7dd5a6e2e7f570e1b8c19a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 27 Jun 2025 12:04:37 -0700 Subject: [PATCH 51/52] [TypeChecker] Implement a per-module block list for disjunction optimizer Allow enabling performance hacks via a block list action - `ShouldUseTypeCheckerPerfHacks` --- include/swift/AST/TypeCheckRequests.h | 16 ++++++++++++++++ include/swift/AST/TypeCheckerTypeIDZone.def | 4 ++++ include/swift/Basic/BlockListAction.def | 1 + lib/Sema/TypeCheckConstraints.cpp | 14 +++++++++++++- .../Constraints/perf_hacks_with_block_list.swift | 16 ++++++++++++++++ 5 files changed, 50 insertions(+), 1 deletion(-) create mode 100644 test/Constraints/perf_hacks_with_block_list.swift diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 0c26d2ba2414e..d0c515ed4c418 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -5370,6 +5370,22 @@ class DefaultIsolationInSourceFileRequest bool isCached() const { return true; } }; +class ModuleHasTypeCheckerPerformanceHacksEnabledRequest + : public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + bool evaluate(Evaluator &evaluator, const ModuleDecl *module) const; + +public: + bool isCached() const { return true; } +}; + #define SWIFT_TYPEID_ZONE TypeChecker #define SWIFT_TYPEID_HEADER "swift/AST/TypeCheckerTypeIDZone.def" #include "swift/Basic/DefineTypeIDZone.h" diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index c1bed6f15c46d..9028c6e97c2f4 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -637,3 +637,7 @@ SWIFT_REQUEST(TypeChecker, SemanticAvailabilitySpecRequest, SWIFT_REQUEST(TypeChecker, DefaultIsolationInSourceFileRequest, std::optional(const SourceFile *), Cached, NoLocationInfo) + +SWIFT_REQUEST(TypeChecker, ModuleHasTypeCheckerPerformanceHacksEnabledRequest, + bool(const ModuleDecl *), + Cached, NoLocationInfo) \ No newline at end of file diff --git a/include/swift/Basic/BlockListAction.def b/include/swift/Basic/BlockListAction.def index 8d9839f019ce3..ffa570e27272f 100644 --- a/include/swift/Basic/BlockListAction.def +++ b/include/swift/Basic/BlockListAction.def @@ -26,5 +26,6 @@ BLOCKLIST_ACTION(ShouldUseLayoutStringValueWitnesses) BLOCKLIST_ACTION(ShouldDisableOwnershipVerification) BLOCKLIST_ACTION(SkipEmittingFineModuleTrace) BLOCKLIST_ACTION(SkipIndexingModule) +BLOCKLIST_ACTION(ShouldUseTypeCheckerPerfHacks) #undef BLOCKLIST_ACTION diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index c1bdf3c88bb63..6414950917ed0 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -466,7 +466,11 @@ TypeChecker::typeCheckTarget(SyntacticElementTarget &target, if (options.contains(TypeCheckExprFlags::DisableMacroExpansions)) csOptions |= ConstraintSystemFlags::DisableMacroExpansions; - if (Context.TypeCheckerOpts.EnableConstraintSolverPerformanceHacks) + if (Context.TypeCheckerOpts.EnableConstraintSolverPerformanceHacks || + evaluateOrDefault(Context.evaluator, + ModuleHasTypeCheckerPerformanceHacksEnabledRequest{ + dc->getParentModule()}, + false)) csOptions |= ConstraintSystemFlags::EnablePerformanceHacks; ConstraintSystem cs(dc, csOptions, diagnosticTransaction); @@ -2463,3 +2467,11 @@ void ConstraintSystem::forEachExpr( expr->walk(ChildWalker(*this, callback)); } + +bool ModuleHasTypeCheckerPerformanceHacksEnabledRequest::evaluate( + Evaluator &evaluator, const ModuleDecl *module) const { + auto name = module->getRealName().str(); + return module->getASTContext().blockListConfig.hasBlockListAction( + name, BlockListKeyKind::ModuleName, + BlockListAction::ShouldUseTypeCheckerPerfHacks); +} \ No newline at end of file diff --git a/test/Constraints/perf_hacks_with_block_list.swift b/test/Constraints/perf_hacks_with_block_list.swift new file mode 100644 index 0000000000000..5a965e28c1625 --- /dev/null +++ b/test/Constraints/perf_hacks_with_block_list.swift @@ -0,0 +1,16 @@ +// RUN: %empty-directory(%t) +// RUN: split-file %s %t + +// RUN: %target-swift-frontend -typecheck -module-name Test -debug-constraints -blocklist-file %t/blocklist.yaml -verify %t/main.swift 2>%t.err +// RUN: %FileCheck %t/main.swift < %t.err + +//--- blocklist.yaml +--- +ShouldUseTypeCheckerPerfHacks: + ModuleName: + - Test + +//--- main.swift +_ = 1 + 2 + 3 + +// CHECK: [favored] {{.*}} bound to decl Swift.(file).Int extension.+ : (Int.Type) -> (Int, Int) -> Int From 4591884bbc4c5b10748ea703d381dc1def99ad20 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 27 Jun 2025 23:44:36 -0700 Subject: [PATCH 52/52] [Tests] NFC: Remove `-disable-constraint-solver-performance-hacks` since hacks are now disabled by default --- test/Constraints/issue-52724.swift | 2 +- test/Constraints/warn_long_compile.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Constraints/issue-52724.swift b/test/Constraints/issue-52724.swift index 3be619ad59310..acbdc0c453619 100644 --- a/test/Constraints/issue-52724.swift +++ b/test/Constraints/issue-52724.swift @@ -1,4 +1,4 @@ -// RUN: %target-swift-frontend -typecheck -verify -disable-constraint-solver-performance-hacks %s +// RUN: %target-swift-frontend -typecheck -verify %s // https://github.com/apple/swift/issues/52724 diff --git a/test/Constraints/warn_long_compile.swift b/test/Constraints/warn_long_compile.swift index fc425058bed31..cc28934a0e155 100644 --- a/test/Constraints/warn_long_compile.swift +++ b/test/Constraints/warn_long_compile.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift -warn-long-expression-type-checking=1 -disable-constraint-solver-performance-hacks -warn-long-function-bodies=1 +// RUN: %target-typecheck-verify-swift -warn-long-expression-type-checking=1 -warn-long-function-bodies=1 @_silgen_name("generic_foo") func foo(_ x: T) -> T