From 757ca24e8a9e30e14a3c26155978310b733e18ac Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 3 Feb 2023 15:37:53 -0800 Subject: [PATCH 01/60] [ConstraintSystem] Remove `shrink` --- include/swift/Sema/ConstraintSystem.h | 78 ---- lib/Sema/CSSolver.cpp | 552 -------------------------- lib/Sema/TypeCheckConstraints.cpp | 4 - 3 files changed, 634 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 26c681869851a..37180f9ea2b85 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2424,75 +2424,6 @@ class ConstraintSystem { SynthesizedConformances; private: - /// Describe the candidate expression for partial solving. - /// This class used by shrink & solve methods which apply - /// variation of directional path consistency algorithm in attempt - /// to reduce scopes of the overload sets (disjunctions) in the system. - class Candidate { - Expr *E; - DeclContext *DC; - llvm::BumpPtrAllocator &Allocator; - - // Contextual Information. - Type CT; - ContextualTypePurpose CTP; - - public: - Candidate(ConstraintSystem &cs, Expr *expr, Type ct = Type(), - ContextualTypePurpose ctp = ContextualTypePurpose::CTP_Unused) - : E(expr), DC(cs.DC), Allocator(cs.Allocator), CT(ct), CTP(ctp) {} - - /// Return underlying expression. - Expr *getExpr() const { return E; } - - /// Try to solve this candidate sub-expression - /// and re-write it's OSR domains afterwards. - /// - /// \param shrunkExprs The set of expressions which - /// domains have been successfully shrunk so far. - /// - /// \returns true on solver failure, false otherwise. - bool solve(llvm::SmallSetVector &shrunkExprs); - - /// Apply solutions found by solver as reduced OSR sets for - /// for current and all of it's sub-expressions. - /// - /// \param solutions The solutions found by running solver on the - /// this candidate expression. - /// - /// \param shrunkExprs The set of expressions which - /// domains have been successfully shrunk so far. - void applySolutions( - llvm::SmallVectorImpl &solutions, - llvm::SmallSetVector &shrunkExprs) const; - - /// Check if attempt at solving of the candidate makes sense given - /// the current conditions - number of shrunk domains which is related - /// to the given candidate over the total number of disjunctions present. - static bool - isTooComplexGiven(ConstraintSystem *const cs, - llvm::SmallSetVector &shrunkExprs) { - SmallVector disjunctions; - cs->collectDisjunctions(disjunctions); - - unsigned unsolvedDisjunctions = disjunctions.size(); - for (auto *disjunction : disjunctions) { - auto *locator = disjunction->getLocator(); - if (!locator) - continue; - - if (auto *OSR = getAsExpr(locator->getAnchor())) { - if (shrunkExprs.count(OSR) > 0) - --unsolvedDisjunctions; - } - } - - unsigned threshold = - cs->getASTContext().TypeCheckerOpts.SolverShrinkUnsolvedThreshold; - return unsolvedDisjunctions >= threshold; - } - }; - /// Describes the current solver state. struct SolverState { SolverState(ConstraintSystem &cs, @@ -5280,15 +5211,6 @@ class ConstraintSystem { /// \returns true if an error occurred, false otherwise. bool solveSimplified(SmallVectorImpl &solutions); - /// Find reduced domains of disjunction constraints for given - /// expression, this is achieved to solving individual sub-expressions - /// and combining resolving types. Such algorithm is called directional - /// path consistency because it goes from children to parents for all - /// related sub-expressions taking union of their domains. - /// - /// \param expr The expression to find reductions for. - void shrink(Expr *expr); - /// Pick a disjunction from the InactiveConstraints list. /// /// \returns The selected disjunction. diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index b64174d9307c8..4c4902f8c5572 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -799,556 +799,6 @@ ConstraintSystem::solveSingle(FreeTypeVariableBinding allowFreeTypeVariables, return std::move(solutions[0]); } -bool ConstraintSystem::Candidate::solve( - llvm::SmallSetVector &shrunkExprs) { - // Don't attempt to solve candidate if there is closure - // expression involved, because it's handled specially - // by parent constraint system (e.g. parameter lists). - bool containsClosure = false; - E->forEachChildExpr([&](Expr *childExpr) -> Expr * { - if (isa(childExpr)) { - containsClosure = true; - return nullptr; - } - return childExpr; - }); - - if (containsClosure) - return false; - - auto cleanupImplicitExprs = [&](Expr *expr) { - expr->forEachChildExpr([&](Expr *childExpr) -> Expr * { - Type type = childExpr->getType(); - if (childExpr->isImplicit() && type && type->hasTypeVariable()) - childExpr->setType(Type()); - return childExpr; - }); - }; - - // Allocate new constraint system for sub-expression. - ConstraintSystem cs(DC, std::nullopt); - - // Set up expression type checker timer for the candidate. - cs.startExpressionTimer(E); - - // Generate constraints for the new system. - if (auto generatedExpr = cs.generateConstraints(E, DC)) { - E = generatedExpr; - } else { - // Failure to generate constraint system for sub-expression - // means we can't continue solving sub-expressions. - cleanupImplicitExprs(E); - return true; - } - - // If this candidate is too complex given the number - // of the domains we have reduced so far, let's bail out early. - if (isTooComplexGiven(&cs, shrunkExprs)) - return false; - - auto &ctx = cs.getASTContext(); - if (cs.isDebugMode()) { - auto &log = llvm::errs(); - auto indent = cs.solverState ? cs.solverState->getCurrentIndent() : 0; - log.indent(indent) << "--- Solving candidate for shrinking at "; - auto R = E->getSourceRange(); - if (R.isValid()) { - R.print(log, ctx.SourceMgr, /*PrintText=*/ false); - } else { - log << ""; - } - log << " ---\n"; - - E->dump(log, indent); - log << '\n'; - cs.print(log); - } - - // If there is contextual type present, add an explicit "conversion" - // constraint to the system. - if (!CT.isNull()) { - auto constraintKind = ConstraintKind::Conversion; - if (CTP == CTP_CallArgument) - constraintKind = ConstraintKind::ArgumentConversion; - - cs.addConstraint(constraintKind, cs.getType(E), CT, - cs.getConstraintLocator(E), /*isFavored=*/true); - } - - // Try to solve the system and record all available solutions. - llvm::SmallVector solutions; - { - SolverState state(cs, FreeTypeVariableBinding::Allow); - - // Use solve which doesn't try to filter solution list. - // Because we want the whole set of possible domain choices. - cs.solveImpl(solutions); - } - - if (cs.isDebugMode()) { - auto &log = llvm::errs(); - auto indent = cs.solverState ? cs.solverState->getCurrentIndent() : 0; - if (solutions.empty()) { - log << "\n"; - log.indent(indent) << "--- No Solutions ---\n"; - } else { - log << "\n"; - log.indent(indent) << "--- Solutions ---\n"; - for (unsigned i = 0, n = solutions.size(); i != n; ++i) { - auto &solution = solutions[i]; - log << "\n"; - log.indent(indent) << "--- Solution #" << i << " ---\n"; - solution.dump(log, indent); - } - } - } - - // Record found solutions as suggestions. - this->applySolutions(solutions, shrunkExprs); - - // Let's double-check if we have any implicit expressions - // with type variables and nullify their types. - cleanupImplicitExprs(E); - - // No solutions for the sub-expression means that either main expression - // needs salvaging or it's inconsistent (read: doesn't have solutions). - return solutions.empty(); -} - -void ConstraintSystem::Candidate::applySolutions( - llvm::SmallVectorImpl &solutions, - llvm::SmallSetVector &shrunkExprs) const { - // A collection of OSRs with their newly reduced domains, - // it's domains are sets because multiple solutions can have the same - // choice for one of the type variables, and we want no duplication. - llvm::SmallDenseMap> - domains; - for (auto &solution : solutions) { - auto &score = solution.getFixedScore(); - - // Avoid any solutions with implicit value conversions - // because they might get reverted later when more context - // becomes available. - if (score.Data[SK_ImplicitValueConversion] > 0) - continue; - - for (auto choice : solution.overloadChoices) { - // Some of the choices might not have locators. - if (!choice.getFirst()) - continue; - - auto anchor = choice.getFirst()->getAnchor(); - auto *OSR = getAsExpr(anchor); - // Anchor is not available or expression is not an overload set. - if (!OSR) - continue; - - auto overload = choice.getSecond().choice; - auto type = overload.getDecl()->getInterfaceType(); - - // One of the solutions has polymorphic type associated with one of its - // type variables. Such functions can only be properly resolved - // via complete expression, so we'll have to forget solutions - // we have already recorded. They might not include all viable overload - // choices. - if (type->is()) { - return; - } - - domains[OSR].insert(overload.getDecl()); - } - } - - // Reduce the domains. - for (auto &domain : domains) { - auto OSR = domain.getFirst(); - auto &choices = domain.getSecond(); - - // If the domain wasn't reduced, skip it. - if (OSR->getDecls().size() == choices.size()) continue; - - // Update the expression with the reduced domain. - MutableArrayRef decls( - Allocator.Allocate(choices.size()), - choices.size()); - - std::uninitialized_copy(choices.begin(), choices.end(), decls.begin()); - OSR->setDecls(decls); - - // Record successfully shrunk expression. - shrunkExprs.insert(OSR); - } -} - -void ConstraintSystem::shrink(Expr *expr) { - if (getASTContext().TypeCheckerOpts.SolverDisableShrink) - return; - - using DomainMap = llvm::SmallDenseMap>; - - // A collection of original domains of all of the expressions, - // so they can be restored in case of failure. - DomainMap domains; - - struct ExprCollector : public ASTWalker { - Expr *PrimaryExpr; - - // The primary constraint system. - ConstraintSystem &CS; - - // All of the sub-expressions which are suitable to be solved - // separately from the main system e.g. binary expressions, collections, - // function calls, coercions etc. - llvm::SmallVector Candidates; - - // Counts the number of overload sets present in the tree so far. - // Note that the traversal is depth-first. - llvm::SmallVector, 4> ApplyExprs; - - // A collection of original domains of all of the expressions, - // so they can be restored in case of failure. - DomainMap &Domains; - - ExprCollector(Expr *expr, ConstraintSystem &cs, DomainMap &domains) - : PrimaryExpr(expr), CS(cs), Domains(domains) {} - - MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Arguments; - } - - PreWalkResult walkToExprPre(Expr *expr) override { - // A dictionary expression is just a set of tuples; try to solve ones - // that have overload sets. - if (auto collectionExpr = dyn_cast(expr)) { - visitCollectionExpr(collectionExpr, - CS.getContextualType(expr, /*forConstraint=*/false), - CS.getContextualTypePurpose(expr)); - // Don't try to walk into the dictionary. - return Action::SkipNode(expr); - } - - // Let's not attempt to type-check closures or expressions - // which constrain closures, because they require special handling - // when dealing with context and parameters declarations. - if (isa(expr)) { - return Action::SkipNode(expr); - } - - // Similar to 'ClosureExpr', 'TapExpr' has a 'VarDecl' the type of which - // is determined by type checking the parent interpolated string literal. - if (isa(expr)) { - return Action::SkipNode(expr); - } - - // Same as TapExpr and ClosureExpr, we'll handle SingleValueStmtExprs - // separately. - if (isa(expr)) - return Action::SkipNode(expr); - - if (auto coerceExpr = dyn_cast(expr)) { - if (coerceExpr->isLiteralInit()) - ApplyExprs.push_back({coerceExpr, 1}); - visitCoerceExpr(coerceExpr); - return Action::SkipNode(expr); - } - - if (auto OSR = dyn_cast(expr)) { - Domains[OSR] = OSR->getDecls(); - } - - if (auto applyExpr = dyn_cast(expr)) { - auto func = applyExpr->getFn(); - // Let's record this function application for post-processing - // as well as if it contains overload set, see walkToExprPost. - ApplyExprs.push_back( - {applyExpr, isa(func) || isa(func)}); - } - - return Action::Continue(expr); - } - - /// Determine whether this is an arithmetic expression comprised entirely - /// of literals. - static bool isArithmeticExprOfLiterals(Expr *expr) { - expr = expr->getSemanticsProvidingExpr(); - - if (auto prefix = dyn_cast(expr)) - return isArithmeticExprOfLiterals(prefix->getOperand()); - - if (auto postfix = dyn_cast(expr)) - return isArithmeticExprOfLiterals(postfix->getOperand()); - - if (auto binary = dyn_cast(expr)) - return isArithmeticExprOfLiterals(binary->getLHS()) && - isArithmeticExprOfLiterals(binary->getRHS()); - - return isa(expr) || isa(expr); - } - - PostWalkResult walkToExprPost(Expr *expr) override { - auto isSrcOfPrimaryAssignment = [&](Expr *expr) -> bool { - if (auto *AE = dyn_cast(PrimaryExpr)) - return expr == AE->getSrc(); - return false; - }; - - if (expr == PrimaryExpr || isSrcOfPrimaryAssignment(expr)) { - // If this is primary expression and there are no candidates - // to be solved, let's not record it, because it's going to be - // solved regardless. - if (Candidates.empty()) - return Action::Continue(expr); - - auto contextualType = CS.getContextualType(expr, - /*forConstraint=*/false); - // If there is a contextual type set for this expression. - if (!contextualType.isNull()) { - Candidates.push_back(Candidate(CS, PrimaryExpr, contextualType, - CS.getContextualTypePurpose(expr))); - return Action::Continue(expr); - } - - // Or it's a function application or assignment with other candidates - // present. Assignment should be easy to solve because we'd get a - // contextual type from the destination expression, otherwise shrink - // might produce incorrect results without considering aforementioned - // destination type. - if (isa(expr) || isa(expr)) { - Candidates.push_back(Candidate(CS, PrimaryExpr)); - return Action::Continue(expr); - } - } - - if (!isa(expr)) - return Action::Continue(expr); - - unsigned numOverloadSets = 0; - // Let's count how many overload sets do we have. - while (!ApplyExprs.empty()) { - auto &application = ApplyExprs.back(); - auto applyExpr = application.first; - - // Add overload sets tracked by current expression. - numOverloadSets += application.second; - ApplyExprs.pop_back(); - - // We've found the current expression, so record the number of - // overloads. - if (expr == applyExpr) { - ApplyExprs.push_back({applyExpr, numOverloadSets}); - break; - } - } - - // If there are fewer than two overloads in the chain - // there is no point of solving this expression, - // because we won't be able to reduce its domain. - if (numOverloadSets > 1 && !isArithmeticExprOfLiterals(expr)) - Candidates.push_back(Candidate(CS, expr)); - - return Action::Continue(expr); - } - - private: - /// Extract type of the element from given collection type. - /// - /// \param collection The type of the collection container. - /// - /// \returns Null type, ErrorType or UnresolvedType on failure, - /// properly constructed type otherwise. - Type extractElementType(Type collection) { - auto &ctx = CS.getASTContext(); - if (!collection || collection->hasError()) - return collection; - - auto base = collection.getPointer(); - auto isInvalidType = [](Type type) -> bool { - return type.isNull() || type->hasUnresolvedType() || - type->hasError(); - }; - - // Array type. - if (auto array = dyn_cast(base)) { - auto elementType = array->getBaseType(); - // If base type is invalid let's return error type. - return elementType; - } - - // Map or Set or any other associated collection type. - if (auto boundGeneric = dyn_cast(base)) { - if (boundGeneric->hasUnresolvedType()) - return boundGeneric; - - llvm::SmallVector params; - for (auto &type : boundGeneric->getGenericArgs()) { - // One of the generic arguments in invalid or unresolved. - if (isInvalidType(type)) - return type; - - params.push_back(type); - } - - // If there is just one parameter, let's return it directly. - if (params.size() == 1) - return params[0].getType(); - - return TupleType::get(params, ctx); - } - - return Type(); - } - - bool isSuitableCollection(TypeRepr *collectionTypeRepr) { - // Only generic identifier, array or dictionary. - switch (collectionTypeRepr->getKind()) { - case TypeReprKind::UnqualifiedIdent: - return cast(collectionTypeRepr) - ->hasGenericArgList(); - - case TypeReprKind::Array: - case TypeReprKind::Dictionary: - return true; - - default: - break; - } - - return false; - } - - void visitCoerceExpr(CoerceExpr *coerceExpr) { - auto subExpr = coerceExpr->getSubExpr(); - // Coerce expression is valid only if it has sub-expression. - if (!subExpr) return; - - unsigned numOverloadSets = 0; - subExpr->forEachChildExpr([&](Expr *childExpr) -> Expr * { - if (isa(childExpr)) { - ++numOverloadSets; - return childExpr; - } - - if (auto nestedCoerceExpr = dyn_cast(childExpr)) { - visitCoerceExpr(nestedCoerceExpr); - // Don't walk inside of nested coercion expression directly, - // that is be done by recursive call to visitCoerceExpr. - return nullptr; - } - - // If sub-expression we are trying to coerce to type is a collection, - // let's allow collector discover it with assigned contextual type - // of coercion, which allows collections to be solved in parts. - if (auto collectionExpr = dyn_cast(childExpr)) { - auto *const typeRepr = coerceExpr->getCastTypeRepr(); - - if (typeRepr && isSuitableCollection(typeRepr)) { - const auto coercionType = TypeResolution::resolveContextualType( - typeRepr, CS.DC, std::nullopt, - // FIXME: Should we really be unconditionally complaining - // about unbound generics and placeholders here? For - // example: - // let foo: [Array] = [[0], [1], [2]] as [Array] - // let foo: [Array] = [[0], [1], [2]] as [Array<_>] - /*unboundTyOpener*/ nullptr, /*placeholderHandler*/ nullptr, - /*packElementOpener*/ nullptr); - - // Looks like coercion type is invalid, let's skip this sub-tree. - if (coercionType->hasError()) - return nullptr; - - // Visit collection expression inline. - visitCollectionExpr(collectionExpr, coercionType, - CTP_CoerceOperand); - } - } - - return childExpr; - }); - - // It's going to be inefficient to try and solve - // coercion in parts, so let's just make it a candidate directly, - // if it contains at least a single overload set. - - if (numOverloadSets > 0) - Candidates.push_back(Candidate(CS, coerceExpr)); - } - - void visitCollectionExpr(CollectionExpr *collectionExpr, - Type contextualType = Type(), - ContextualTypePurpose CTP = CTP_Unused) { - // If there is a contextual type set for this collection, - // let's propagate it to the candidate. - if (!contextualType.isNull()) { - auto elementType = extractElementType(contextualType); - // If we couldn't deduce element type for the collection, let's - // not attempt to solve it. - if (!elementType || - elementType->hasError() || - elementType->hasUnresolvedType()) - return; - - contextualType = elementType; - } - - for (auto element : collectionExpr->getElements()) { - unsigned numOverloads = 0; - element->walk(OverloadSetCounter(numOverloads)); - - // There are no overload sets in the element; skip it. - if (numOverloads == 0) - continue; - - // Record each of the collection elements, which passed - // number of overload sets rule, as a candidate for solving - // with contextual type of the collection. - Candidates.push_back(Candidate(CS, element, contextualType, CTP)); - } - } - }; - - ExprCollector collector(expr, *this, domains); - - // Collect all of the binary/unary and call sub-expressions - // so we can start solving them separately. - expr->walk(collector); - - llvm::SmallSetVector shrunkExprs; - for (auto &candidate : collector.Candidates) { - // If there are no results, let's forget everything we know about the - // system so far. This actually is ok, because some of the expressions - // might require manual salvaging. - if (candidate.solve(shrunkExprs)) { - // Let's restore all of the original OSR domains for this sub-expression, - // this means that we can still make forward progress with solving of the - // top sub-expressions. - candidate.getExpr()->forEachChildExpr([&](Expr *childExpr) -> Expr * { - if (auto OSR = dyn_cast(childExpr)) { - auto domain = domains.find(OSR); - if (domain == domains.end()) - return childExpr; - - OSR->setDecls(domain->getSecond()); - shrunkExprs.remove(OSR); - } - - return childExpr; - }); - } - } - - // Once "shrinking" is done let's re-allocate final version of - // the candidate list to the permanent arena, so it could - // survive even after primary constraint system is destroyed. - for (auto &OSR : shrunkExprs) { - auto choices = OSR->getDecls(); - auto decls = - getASTContext().AllocateUninitialized(choices.size()); - - std::uninitialized_copy(choices.begin(), choices.end(), decls.begin()); - OSR->setDecls(decls); - } -} - static bool debugConstraintSolverForTarget(ASTContext &C, SyntacticElementTarget target) { if (C.TypeCheckerOpts.DebugConstraintSolver) @@ -1715,8 +1165,6 @@ bool ConstraintSystem::solveForCodeCompletion( // Set up the expression type checker timer. startExpressionTimer(expr); - - shrink(expr); } if (isDebugMode()) { diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 4ddc6c32455ca..59bea09049105 100644 --- a/lib/Sema/TypeCheckConstraints.cpp +++ b/lib/Sema/TypeCheckConstraints.cpp @@ -450,10 +450,6 @@ TypeChecker::typeCheckTarget(SyntacticElementTarget &target, // diagnostics and is a hint for various performance optimizations. cs.setContextualInfo(expr, target.getExprContextualTypeInfo()); - // Try to shrink the system by reducing disjunction domains. This - // goes through every sub-expression and generate its own sub-system, to - // try to reduce the domains of those subexpressions. - cs.shrink(expr); target.setExpr(expr); } From 4432c51f57b30f1db7af9d05e0bbba747d02fa8b Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 4 Feb 2023 18:48:19 -0800 Subject: [PATCH 02/60] [CSGen] Remove ConstraintOptimizer and all favoring logic --- include/swift/Sema/ConstraintSystem.h | 5 - lib/Sema/CSGen.cpp | 791 +------------------------- lib/Sema/CSSimplify.cpp | 36 -- 3 files changed, 1 insertion(+), 831 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 37180f9ea2b85..8fe41eef14147 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5403,11 +5403,6 @@ class ConstraintSystem { bool applySolutionToBody(TapExpr *tapExpr, SyntacticElementTargetRewriter &rewriter); - /// Reorder the disjunctive clauses for a given expression to - /// increase the likelihood that a favored constraint will be successfully - /// resolved before any others. - void optimizeConstraints(Expr *e); - void startExpressionTimer(ExpressionTimer::AnchorType anchor); /// Determine if we've already explored too many paths in an diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index bc5d8513e1fc4..ad42de3c99273 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -43,10 +43,6 @@ using namespace swift; using namespace swift::constraints; -static bool isArithmeticOperatorDecl(ValueDecl *vd) { - return vd && vd->getBaseIdentifier().isArithmeticOperator(); -} - static bool mergeRepresentativeEquivalenceClasses(ConstraintSystem &CS, TypeVariableType* tyvar1, TypeVariableType* tyvar2) { @@ -77,698 +73,6 @@ static bool mergeRepresentativeEquivalenceClasses(ConstraintSystem &CS, } namespace { - - /// Internal struct for tracking information about types within a series - /// of "linked" expressions. (Such as a chain of binary operator invocations.) - struct LinkedTypeInfo { - bool hasLiteral = false; - - llvm::SmallSet collectedTypes; - llvm::SmallVector binaryExprs; - }; - - /// Walks an expression sub-tree, and collects information about expressions - /// whose types are mutually dependent upon one another. - class LinkedExprCollector : public ASTWalker { - - llvm::SmallVectorImpl &LinkedExprs; - - public: - LinkedExprCollector(llvm::SmallVectorImpl &linkedExprs) - : LinkedExprs(linkedExprs) {} - - MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Arguments; - } - - PreWalkResult walkToExprPre(Expr *expr) override { - if (isa(expr)) - return Action::SkipNode(expr); - - // Store top-level binary exprs for further analysis. - if (isa(expr) || - - // Literal exprs are contextually typed, so store them off as well. - isa(expr) || - - // We'd like to look at the elements of arrays and dictionaries. - isa(expr) || - isa(expr) || - - // assignment expression can involve anonymous closure parameters - // as source and destination, so it's beneficial for diagnostics if - // we look at the assignment. - isa(expr)) { - LinkedExprs.push_back(expr); - return Action::SkipNode(expr); - } - - return Action::Continue(expr); - } - - /// Ignore statements. - PreWalkResult walkToStmtPre(Stmt *stmt) override { - return Action::SkipNode(stmt); - } - - /// Ignore declarations. - PreWalkAction walkToDeclPre(Decl *decl) override { - return Action::SkipNode(); - } - - /// Ignore patterns. - PreWalkResult walkToPatternPre(Pattern *pat) override { - return Action::SkipNode(pat); - } - - /// Ignore types. - PreWalkAction walkToTypeReprPre(TypeRepr *T) override { - return Action::SkipNode(); - } - }; - - /// Given a collection of "linked" expressions, analyzes them for - /// commonalities regarding their types. This will help us compute a - /// "best common type" from the expression types. - class LinkedExprAnalyzer : public ASTWalker { - - LinkedTypeInfo <I; - ConstraintSystem &CS; - - public: - - LinkedExprAnalyzer(LinkedTypeInfo <i, ConstraintSystem &cs) : - LTI(lti), CS(cs) {} - - MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Arguments; - } - - PreWalkResult walkToExprPre(Expr *expr) override { - if (isa(expr)) { - LTI.hasLiteral = true; - return Action::SkipNode(expr); - } - - if (isa(expr)) { - return Action::Continue(expr); - } - - if (auto UDE = dyn_cast(expr)) { - - if (CS.hasType(UDE)) - LTI.collectedTypes.insert(CS.getType(UDE).getPointer()); - - // Don't recurse into the base expression. - return Action::SkipNode(expr); - } - - - if (isa(expr)) { - return Action::SkipNode(expr); - } - - if (auto FVE = dyn_cast(expr)) { - LTI.collectedTypes.insert(CS.getType(FVE).getPointer()); - return Action::SkipNode(expr); - } - - if (auto DRE = dyn_cast(expr)) { - if (auto varDecl = dyn_cast(DRE->getDecl())) { - if (CS.hasType(DRE)) { - LTI.collectedTypes.insert(CS.getType(DRE).getPointer()); - } - return Action::SkipNode(expr); - } - } - - // In the case of a function application, we would have already captured - // the return type during constraint generation, so there's no use in - // looking any further. - if (isa(expr) && - !(isa(expr) || isa(expr) || - isa(expr))) { - return Action::SkipNode(expr); - } - - if (auto *binaryExpr = dyn_cast(expr)) { - LTI.binaryExprs.push_back(binaryExpr); - } - - if (auto favoredType = CS.getFavoredType(expr)) { - LTI.collectedTypes.insert(favoredType); - - return Action::SkipNode(expr); - } - - // Optimize branches of a conditional expression separately. - if (auto IE = dyn_cast(expr)) { - CS.optimizeConstraints(IE->getCondExpr()); - CS.optimizeConstraints(IE->getThenExpr()); - CS.optimizeConstraints(IE->getElseExpr()); - return Action::SkipNode(expr); - } - - // For exprs of a tuple, avoid favoring. (We need to allow for cases like - // (Int, Int32).) - if (isa(expr)) { - return Action::SkipNode(expr); - } - - // Coercion exprs have a rigid type, so there's no use in gathering info - // about them. - if (auto *coercion = dyn_cast(expr)) { - // Let's not collect information about types initialized by - // coercions just like we don't for regular initializer calls, - // because that might lead to overly eager type variable merging. - if (!coercion->isLiteralInit()) - LTI.collectedTypes.insert(CS.getType(expr).getPointer()); - return Action::SkipNode(expr); - } - - // Don't walk into subscript expressions - to do so would risk factoring - // the index expression into edge contraction. (We don't want to do this - // if the index expression is a literal type that differs from the return - // type of the subscript operation.) - if (isa(expr) || isa(expr)) { - return Action::SkipNode(expr); - } - - // Don't walk into unresolved member expressions - we avoid merging type - // variables inside UnresolvedMemberExpr and those outside, since they - // should be allowed to behave independently in CS. - if (isa(expr)) { - return Action::SkipNode(expr); - } - - return Action::Continue(expr); - } - - /// Ignore statements. - PreWalkResult walkToStmtPre(Stmt *stmt) override { - return Action::SkipNode(stmt); - } - - /// Ignore declarations. - PreWalkAction walkToDeclPre(Decl *decl) override { - return Action::SkipNode(); - } - - /// Ignore patterns. - PreWalkResult walkToPatternPre(Pattern *pat) override { - return Action::SkipNode(pat); - } - - /// Ignore types. - PreWalkAction walkToTypeReprPre(TypeRepr *T) override { - return Action::SkipNode(); - } - }; - - /// For a given expression, given information that is global to the - /// expression, attempt to derive a favored type for it. - void computeFavoredTypeForExpr(Expr *expr, ConstraintSystem &CS) { - LinkedTypeInfo lti; - - expr->walk(LinkedExprAnalyzer(lti, CS)); - - // Check whether we can proceed with favoring. - if (llvm::any_of(lti.binaryExprs, [](const BinaryExpr *op) { - auto *ODRE = dyn_cast(op->getFn()); - if (!ODRE) - return false; - - // Attempting to favor based on operand types is wrong for - // nil-coalescing operator. - auto identifier = ODRE->getDecls().front()->getBaseIdentifier(); - return identifier.isNilCoalescingOperator(); - })) { - return; - } - - if (lti.collectedTypes.size() == 1) { - // TODO: Compute the BCT. - - // It's only useful to favor the type instead of - // binding it directly to arguments/result types, - // which means in case it has been miscalculated - // solver can still make progress. - auto favoredTy = (*lti.collectedTypes.begin())->getWithoutSpecifierType(); - CS.setFavoredType(expr, favoredTy.getPointer()); - - // If we have a chain of identical binop expressions with homogeneous - // argument types, we can directly simplify the associated constraint - // graph. - auto simplifyBinOpExprTyVars = [&]() { - // Don't attempt to do linking if there are - // literals intermingled with other inferred types. - if (lti.hasLiteral) - return; - - for (auto binExp1 : lti.binaryExprs) { - for (auto binExp2 : lti.binaryExprs) { - if (binExp1 == binExp2) - continue; - - auto fnTy1 = CS.getType(binExp1)->getAs(); - auto fnTy2 = CS.getType(binExp2)->getAs(); - - if (!(fnTy1 && fnTy2)) - return; - - auto ODR1 = dyn_cast(binExp1->getFn()); - auto ODR2 = dyn_cast(binExp2->getFn()); - - if (!(ODR1 && ODR2)) - return; - - // TODO: We currently limit this optimization to known arithmetic - // operators, but we should be able to broaden this out to - // logical operators as well. - if (!isArithmeticOperatorDecl(ODR1->getDecls()[0])) - return; - - if (ODR1->getDecls()[0]->getBaseName() != - ODR2->getDecls()[0]->getBaseName()) - return; - - // All things equal, we can merge the tyvars for the function - // types. - auto rep1 = CS.getRepresentative(fnTy1); - auto rep2 = CS.getRepresentative(fnTy2); - - if (rep1 != rep2) { - CS.mergeEquivalenceClasses(rep1, rep2, - /*updateWorkList*/ false); - } - - auto odTy1 = CS.getType(ODR1)->getAs(); - auto odTy2 = CS.getType(ODR2)->getAs(); - - if (odTy1 && odTy2) { - auto odRep1 = CS.getRepresentative(odTy1); - auto odRep2 = CS.getRepresentative(odTy2); - - // Since we'll be choosing the same overload, we can merge - // the overload tyvar as well. - if (odRep1 != odRep2) - CS.mergeEquivalenceClasses(odRep1, odRep2, - /*updateWorkList*/ false); - } - } - } - }; - - simplifyBinOpExprTyVars(); - } - } - - /// Determine whether the given parameter type and argument should be - /// "favored" because they match exactly. - bool isFavoredParamAndArg(ConstraintSystem &CS, Type paramTy, Type argTy, - Type otherArgTy = Type()) { - // Determine the argument type. - argTy = argTy->getWithoutSpecifierType(); - - // Do the types match exactly? - if (paramTy->isEqual(argTy)) - return true; - - // Don't favor narrowing conversions. - if (argTy->isDouble() && paramTy->isCGFloat()) - return false; - - llvm::SmallSetVector literalProtos; - if (auto argTypeVar = argTy->getAs()) { - auto constraints = CS.getConstraintGraph().gatherConstraints( - argTypeVar, ConstraintGraph::GatheringKind::EquivalenceClass, - [](Constraint *constraint) { - return constraint->getKind() == ConstraintKind::LiteralConformsTo; - }); - - for (auto constraint : constraints) { - literalProtos.insert(constraint->getProtocol()); - } - } - - // Dig out the second argument type. - if (otherArgTy) - otherArgTy = otherArgTy->getWithoutSpecifierType(); - - for (auto literalProto : literalProtos) { - // If there is another, concrete argument, check whether it's type - // conforms to the literal protocol and test against it directly. - // This helps to avoid 'widening' the favored type to the default type for - // the literal. - if (otherArgTy && otherArgTy->getAnyNominal()) { - if (otherArgTy->isEqual(paramTy) && - CS.lookupConformance(otherArgTy, literalProto)) { - return true; - } - } else if (Type defaultType = - TypeChecker::getDefaultType(literalProto, CS.DC)) { - // If there is a default type for the literal protocol, check whether - // it is the same as the parameter type. - // Check whether there is a default type to compare against. - if (paramTy->isEqual(defaultType) || - (defaultType->isDouble() && paramTy->isCGFloat())) - return true; - } - } - - return false; - } - - /// Favor certain overloads in a call based on some basic analysis - /// of the overload set and call arguments. - /// - /// \param expr The application. - /// \param isFavored Determine whether the given overload is favored, passing - /// it the "effective" overload type when it's being called. - /// \param mustConsider If provided, a function to detect the presence of - /// overloads which inhibit any overload from being favored. - void favorCallOverloads(ApplyExpr *expr, - ConstraintSystem &CS, - llvm::function_ref isFavored, - std::function - mustConsider = nullptr) { - // Find the type variable associated with the function, if any. - auto tyvarType = CS.getType(expr->getFn())->getAs(); - if (!tyvarType || CS.getFixedType(tyvarType)) - return; - - // This type variable is only currently associated with the function - // being applied, and the only constraint attached to it should - // be the disjunction constraint for the overload group. - auto disjunction = CS.getUnboundBindOverloadDisjunction(tyvarType); - if (!disjunction) - return; - - // Find the favored constraints and mark them. - SmallVector newlyFavoredConstraints; - unsigned numFavoredConstraints = 0; - Constraint *firstFavored = nullptr; - for (auto constraint : disjunction->getNestedConstraints()) { - auto *decl = constraint->getOverloadChoice().getDeclOrNull(); - if (!decl) - continue; - - if (mustConsider && mustConsider(decl)) { - // Roll back any constraints we favored. - for (auto favored : newlyFavoredConstraints) - favored->setFavored(false); - - return; - } - - Type overloadType = CS.getEffectiveOverloadType( - constraint->getLocator(), constraint->getOverloadChoice(), - /*allowMembers=*/true, CS.DC); - if (!overloadType) - continue; - - if (!CS.isDeclUnavailable(decl, constraint->getLocator()) && - !decl->getAttrs().hasAttribute() && - isFavored(decl, overloadType)) { - // If we might need to roll back the favored constraints, keep - // track of those we are favoring. - if (mustConsider && !constraint->isFavored()) - newlyFavoredConstraints.push_back(constraint); - - constraint->setFavored(); - ++numFavoredConstraints; - if (!firstFavored) - firstFavored = constraint; - } - } - - // If there was one favored constraint, set the favored type based on its - // result type. - if (numFavoredConstraints == 1) { - auto overloadChoice = firstFavored->getOverloadChoice(); - auto overloadType = CS.getEffectiveOverloadType( - firstFavored->getLocator(), overloadChoice, /*allowMembers=*/true, - CS.DC); - auto resultType = overloadType->castTo()->getResult(); - if (!resultType->hasTypeParameter()) - CS.setFavoredType(expr, resultType.getPointer()); - } - } - - /// Return a pair, containing the total parameter count of a function, coupled - /// with the number of non-default parameters. - std::pair getParamCount(ValueDecl *VD) { - auto fTy = VD->getInterfaceType()->castTo(); - - size_t nOperands = fTy->getParams().size(); - size_t nNoDefault = 0; - - if (auto AFD = dyn_cast(VD)) { - assert(!AFD->hasImplicitSelfDecl()); - for (auto param : *AFD->getParameters()) { - if (!param->isDefaultArgument()) - ++nNoDefault; - } - } else { - nNoDefault = nOperands; - } - - return { nOperands, nNoDefault }; - } - - bool hasContextuallyFavorableResultType(AnyFunctionType *choice, - Type contextualTy) { - // No restrictions of what result could be. - if (!contextualTy) - return true; - - auto resultTy = choice->getResult(); - // Result type of the call matches expected contextual type. - return contextualTy->isEqual(resultTy); - } - - /// Favor unary operator constraints where we have exact matches - /// for the operand and contextual type. - void favorMatchingUnaryOperators(ApplyExpr *expr, - ConstraintSystem &CS) { - auto *unaryArg = expr->getArgs()->getUnaryExpr(); - assert(unaryArg); - - // Determine whether the given declaration is favored. - auto isFavoredDecl = [&](ValueDecl *value, Type type) -> bool { - auto fnTy = type->getAs(); - if (!fnTy) - return false; - - auto params = fnTy->getParams(); - if (params.size() != 1) - return false; - - auto paramTy = params[0].getPlainType(); - auto argTy = CS.getType(unaryArg); - - // There are no CGFloat overloads on some of the unary operators, so - // in order to preserve current behavior, let's not favor overloads - // which would result in conversion from CGFloat to Double; otherwise - // it would lead to ambiguities. - if (argTy->isCGFloat() && paramTy->isDouble()) - return false; - - return isFavoredParamAndArg(CS, paramTy, argTy) && - hasContextuallyFavorableResultType( - fnTy, - CS.getContextualType(expr, /*forConstraint=*/false)); - }; - - favorCallOverloads(expr, CS, isFavoredDecl); - } - - void favorMatchingOverloadExprs(ApplyExpr *expr, - ConstraintSystem &CS) { - // Find the argument type. - size_t nArgs = expr->getArgs()->size(); - auto fnExpr = expr->getFn(); - - auto mustConsiderVariadicGenericOverloads = [&](ValueDecl *overload) { - if (overload->getAttrs().hasAttribute()) - return false; - - auto genericContext = overload->getAsGenericContext(); - if (!genericContext) - return false; - - auto *GPL = genericContext->getGenericParams(); - if (!GPL) - return false; - - return llvm::any_of(GPL->getParams(), - [&](const GenericTypeParamDecl *GP) { - return GP->isParameterPack(); - }); - }; - - // Check to ensure that we have an OverloadedDeclRef, and that we're not - // favoring multiple overload constraints. (Otherwise, in this case - // favoring is useless. - if (auto ODR = dyn_cast(fnExpr)) { - bool haveMultipleApplicableOverloads = false; - - for (auto VD : ODR->getDecls()) { - if (VD->getInterfaceType()->is()) { - auto nParams = getParamCount(VD); - - if (nArgs == nParams.first) { - if (haveMultipleApplicableOverloads) { - return; - } else { - haveMultipleApplicableOverloads = true; - } - } - } - } - - // Determine whether the given declaration is favored. - auto isFavoredDecl = [&](ValueDecl *value, Type type) -> bool { - // We want to consider all options for calls that might contain the code - // completion location, as missing arguments after the completion - // location are valid (since it might be that they just haven't been - // written yet). - if (CS.isForCodeCompletion()) - return false; - - if (!type->is()) - return false; - - auto paramCount = getParamCount(value); - - return nArgs == paramCount.first || - nArgs == paramCount.second; - }; - - favorCallOverloads(expr, CS, isFavoredDecl, - mustConsiderVariadicGenericOverloads); - } - - // We only currently perform favoring for unary args. - auto *unaryArg = expr->getArgs()->getUnlabeledUnaryExpr(); - if (!unaryArg) - return; - - if (auto favoredTy = CS.getFavoredType(unaryArg)) { - // Determine whether the given declaration is favored. - auto isFavoredDecl = [&](ValueDecl *value, Type type) -> bool { - auto fnTy = type->getAs(); - if (!fnTy || fnTy->getParams().size() != 1) - return false; - - return favoredTy->isEqual(fnTy->getParams()[0].getPlainType()); - }; - - // This is a hack to ensure we always consider the protocol requirement - // itself when calling something that has a default implementation in an - // extension. Otherwise, the extension method might be favored if we're - // inside an extension context, since any archetypes in the parameter - // list could match exactly. - auto mustConsider = [&](ValueDecl *value) -> bool { - return isa(value->getDeclContext()) || - mustConsiderVariadicGenericOverloads(value); - }; - - favorCallOverloads(expr, CS, isFavoredDecl, mustConsider); - } - } - - /// Favor binary operator constraints where we have exact matches - /// for the operands and contextual type. - void favorMatchingBinaryOperators(ApplyExpr *expr, ConstraintSystem &CS) { - // If we're generating constraints for a binary operator application, - // there are two special situations to consider: - // 1. If the type checker has any newly created functions with the - // operator's name. If it does, the overloads were created after the - // associated overloaded id expression was created, and we'll need to - // add a new disjunction constraint for the new set of overloads. - // 2. If any component argument expressions (nested or otherwise) are - // literals, we can favor operator overloads whose argument types are - // identical to the literal type, or whose return types are identical - // to any contextual type associated with the application expression. - - // Find the argument types. - auto *args = expr->getArgs(); - auto *lhs = args->getExpr(0); - auto *rhs = args->getExpr(1); - - auto firstArgTy = CS.getType(lhs); - auto secondArgTy = CS.getType(rhs); - - auto isOptionalWithMatchingObjectType = [](Type optional, - Type object) -> bool { - if (auto objTy = optional->getRValueType()->getOptionalObjectType()) - return objTy->getRValueType()->isEqual(object->getRValueType()); - - return false; - }; - - auto isPotentialForcingOpportunity = [&](Type first, Type second) -> bool { - return isOptionalWithMatchingObjectType(first, second) || - isOptionalWithMatchingObjectType(second, first); - }; - - // Determine whether the given declaration is favored. - auto isFavoredDecl = [&](ValueDecl *value, Type type) -> bool { - auto fnTy = type->getAs(); - if (!fnTy) - return false; - - auto firstFavoredTy = CS.getFavoredType(lhs); - auto secondFavoredTy = CS.getFavoredType(rhs); - - auto favoredExprTy = CS.getFavoredType(expr); - - if (isArithmeticOperatorDecl(value)) { - // If the parent has been favored on the way down, propagate that - // information to its children. - // TODO: This is only valid for arithmetic expressions. - if (!firstFavoredTy) { - CS.setFavoredType(lhs, favoredExprTy); - firstFavoredTy = favoredExprTy; - } - - if (!secondFavoredTy) { - CS.setFavoredType(rhs, favoredExprTy); - secondFavoredTy = favoredExprTy; - } - } - - auto params = fnTy->getParams(); - if (params.size() != 2) - return false; - - auto firstParamTy = params[0].getOldType(); - auto secondParamTy = params[1].getOldType(); - - auto contextualTy = CS.getContextualType(expr, /*forConstraint=*/false); - - // Avoid favoring overloads that would require narrowing conversion - // to match the arguments. - { - if (firstArgTy->isDouble() && firstParamTy->isCGFloat()) - return false; - - if (secondArgTy->isDouble() && secondParamTy->isCGFloat()) - return false; - } - - return (isFavoredParamAndArg(CS, firstParamTy, firstArgTy, secondArgTy) || - isFavoredParamAndArg(CS, secondParamTy, secondArgTy, - firstArgTy)) && - firstParamTy->isEqual(secondParamTy) && - !isPotentialForcingOpportunity(firstArgTy, secondArgTy) && - hasContextuallyFavorableResultType(fnTy, contextualTy); - }; - - favorCallOverloads(expr, CS, isFavoredDecl); - } - /// If \p expr is a call and that call contains the code completion token, /// add the expressions of all arguments after the code completion token to /// \p ignoredArguments. @@ -799,62 +103,6 @@ namespace { } } } - - class ConstraintOptimizer : public ASTWalker { - ConstraintSystem &CS; - - public: - - ConstraintOptimizer(ConstraintSystem &cs) : - CS(cs) {} - - MacroWalking getMacroWalkingBehavior() const override { - return MacroWalking::Arguments; - } - - PreWalkResult walkToExprPre(Expr *expr) override { - if (CS.isArgumentIgnoredForCodeCompletion(expr)) { - return Action::SkipNode(expr); - } - - if (auto applyExpr = dyn_cast(expr)) { - if (isa(applyExpr) || - isa(applyExpr)) { - favorMatchingUnaryOperators(applyExpr, CS); - } else if (isa(applyExpr)) { - favorMatchingBinaryOperators(applyExpr, CS); - } else { - favorMatchingOverloadExprs(applyExpr, CS); - } - } - - // If the paren expr has a favored type, and the subExpr doesn't, - // propagate downwards. Otherwise, propagate upwards. - if (auto parenExpr = dyn_cast(expr)) { - if (!CS.getFavoredType(parenExpr->getSubExpr())) { - CS.setFavoredType(parenExpr->getSubExpr(), - CS.getFavoredType(parenExpr)); - } else if (!CS.getFavoredType(parenExpr)) { - CS.setFavoredType(parenExpr, - CS.getFavoredType(parenExpr->getSubExpr())); - } - } - - if (isa(expr)) - return Action::SkipNode(expr); - - return Action::Continue(expr); - } - /// Ignore statements. - PreWalkResult walkToStmtPre(Stmt *stmt) override { - return Action::SkipNode(stmt); - } - - /// Ignore declarations. - PreWalkAction walkToDeclPre(Decl *decl) override { - return Action::SkipNode(); - } - }; } // end anonymous namespace void TypeVarRefCollector::inferTypeVars(Decl *D) { @@ -1100,18 +348,6 @@ namespace { if (isLValueBase) outputTy = LValueType::get(outputTy); } - } else if (auto dictTy = CS.isDictionaryType(baseObjTy)) { - auto keyTy = dictTy->first; - auto valueTy = dictTy->second; - - if (argList->isUnlabeledUnary()) { - auto argTy = CS.getType(argList->getExpr(0)); - if (isFavoredParamAndArg(CS, keyTy, argTy)) { - outputTy = OptionalType::get(valueTy); - if (isLValueBase) - outputTy = LValueType::get(outputTy); - } - } } } @@ -4385,12 +3621,7 @@ static Expr *generateConstraintsFor(ConstraintSystem &cs, Expr *expr, ConstraintGenerator cg(cs, DC); ConstraintWalker cw(cg); - Expr *result = expr->walk(cw); - - if (result) - cs.optimizeConstraints(result); - - return result; + return expr->walk(cw); } bool ConstraintSystem::generateWrappedPropertyTypeConstraints( @@ -5141,26 +4372,6 @@ ConstraintSystem::applyPropertyWrapperToParameter( return getTypeMatchSuccess(); } -void ConstraintSystem::optimizeConstraints(Expr *e) { - if (getASTContext().TypeCheckerOpts.DisableConstraintSolverPerformanceHacks) - return; - - SmallVector linkedExprs; - - // Collect any linked expressions. - LinkedExprCollector collector(linkedExprs); - e->walk(collector); - - // Favor types, as appropriate. - for (auto linkedExpr : linkedExprs) { - computeFavoredTypeForExpr(linkedExpr, *this); - } - - // Optimize the constraints. - ConstraintOptimizer optimizer(*this); - e->walk(optimizer); -} - struct ResolvedMemberResult::Implementation { llvm::SmallVector AllDecls; unsigned ViableStartIdx; diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index fa526dc79b3f8..3973f16cf6b66 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -9760,7 +9760,6 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, // If this is true, we're using type construction syntax (Foo()) rather // than an explicit call to `init` (Foo.init()). bool isImplicitInit = false; - TypeBase *favoredType = nullptr; if (memberName.isSimpleName(DeclBaseName::createConstructor())) { SmallVector parts; if (auto anchor = memberLocator->getAnchor()) { @@ -9768,17 +9767,6 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, if (!path.empty()) if (path.back().getKind() == ConstraintLocator::ConstructorMember) isImplicitInit = true; - - if (auto *applyExpr = getAsExpr(anchor)) { - if (auto *argExpr = applyExpr->getArgs()->getUnlabeledUnaryExpr()) { - favoredType = getFavoredType(argExpr); - - if (!favoredType) { - optimizeConstraints(argExpr); - favoredType = getFavoredType(argExpr); - } - } - } } } @@ -9887,30 +9875,6 @@ performMemberLookup(ConstraintKind constraintKind, DeclNameRef memberName, hasInstanceMethods = true; } - // If the invocation's argument expression has a favored type, - // use that information to determine whether a specific overload for - // the candidate should be favored. - if (isa(decl) && favoredType && - result.FavoredChoice == ~0U) { - auto *ctor = cast(decl); - - // Only try and favor monomorphic unary initializers. - if (!ctor->isGenericContext()) { - if (!ctor->getMethodInterfaceType()->hasError()) { - // The constructor might have an error type because we don't skip - // invalid decls for code completion - auto args = ctor->getMethodInterfaceType() - ->castTo() - ->getParams(); - if (args.size() == 1 && !args[0].hasLabel() && - args[0].getPlainType()->isEqual(favoredType)) { - if (!isDeclUnavailable(decl, memberLocator)) - result.FavoredChoice = result.ViableCandidates.size(); - } - } - } - } - const auto isUnsupportedExistentialMemberAccess = [&] { // We may not be able to derive a well defined type for an existential // member access if the member's signature references 'Self'. From b5f08a4009a094463e8fe138b33448b817272e44 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 4 Feb 2023 19:01:34 -0800 Subject: [PATCH 03/60] [ConstraintSystem] Add skeleton of constraint optimizer --- lib/Sema/CMakeLists.txt | 1 + lib/Sema/CSOptimizer.cpp | 25 +++++++++++++++++++++++++ lib/Sema/CSSolver.cpp | 2 ++ 3 files changed, 28 insertions(+) create mode 100644 lib/Sema/CSOptimizer.cpp diff --git a/lib/Sema/CMakeLists.txt b/lib/Sema/CMakeLists.txt index 9868e51098076..652cd39cd502e 100644 --- a/lib/Sema/CMakeLists.txt +++ b/lib/Sema/CMakeLists.txt @@ -13,6 +13,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..0bac2925d1f53 --- /dev/null +++ b/lib/Sema/CSOptimizer.cpp @@ -0,0 +1,25 @@ +//===--- CSOptimizer.cpp - Constraint Optimizer ---------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2014 - 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// +// +// This file implements disjunction and other constraint optimizations. +// +//===----------------------------------------------------------------------===// + +#include "swift/Sema/ConstraintSystem.h" +#include "llvm/ADT/SmallVector.h" + +using namespace swift; +using namespace constraints; + +void ConstraintSystem::optimizeDisjunctions( + SmallVectorImpl &disjunctions) { +} diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 4c4902f8c5572..0cee95e848118 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1835,6 +1835,8 @@ Constraint *ConstraintSystem::selectDisjunction() { if (disjunctions.empty()) return nullptr; + optimizeDisjunctions(disjunctions); + if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return disjunction; From 672ae3d252b6030c83a179fcc2e67704c0a758d8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 9 Feb 2023 17:20:42 -0800 Subject: [PATCH 04/60] [CSOptimizer] Initial implementation of disjunction choice favoring algorithm This algorithm attempts to ensure that the solver always picks a disjunction it knows the most about given the previously deduced type information. For example in chains of operators like: `let _: (Double) -> Void = { 1 * 2 + $0 - 5 }` The solver is going to start from `2 + $0` because `$0` is known to be `Double` and then proceed to `1 * ...` and only after that to `... - 5`. The algorithm is pretty simple: - Collect "candidate" types for each argument - If argument is bound then the set going to be represented by just one type - Otherwise: - Collect all the possible bindings - Add default literal type (if any) - Collect "candidate" types for result - For each disjunction in the current scope: - Compute a favoring score for each viable* overload choice: - Compute score for each parameter: - Match parameter flags to argument flags - Match parameter types to a set of candidate argument types - If it's an exact match - Concrete type: score = 1.0 - Literal default: score = 0.3 - Highest scored candidate type wins. - If none of the candidates match and they are all non-literal remove overload choice from consideration. - Average the score by dividing it by the number of parameters to avoid disfavoring disjunctions with fewer arguments. - Match result type to a set of candidates; add 1 to the score if one of the candidate types matches exactly. - The best choice score becomes a disjunction score - Compute disjunction scores for all of the disjunctions in scope. - Pick disjunction with the best overall score and favor choices with the best local candidate scores (if some candidates have equal scores). - Viable overloads include: - non-disfavored - non-disabled - available - non-generic (with current exception to SIMD) --- lib/Sema/CSOptimizer.cpp | 412 +++++++++++++++++- lib/Sema/CSSolver.cpp | 113 ----- test/Constraints/common_type.swift | 2 + test/Constraints/diag_ambiguities.swift | 6 +- .../{slow => fast}/rdar22770433.swift | 1 - .../{slow => fast}/rdar23682605.swift | 1 - .../{slow => fast}/rdar31742586.swift | 1 - .../{slow => fast}/rdar35213699.swift | 2 - .../rdar46713933_literal_arg.swift | 1 - .../{slow => fast}/rdar91310777.swift | 1 - 10 files changed, 415 insertions(+), 125 deletions(-) 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 (66%) 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%) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 0bac2925d1f53..9c64f8a08b46a 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -14,12 +14,420 @@ // //===----------------------------------------------------------------------===// +#include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" +#include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" +#include "llvm/ADT/TinyPtrVector.h" +#include "llvm/Support/raw_ostream.h" +#include +#include using namespace swift; using namespace constraints; -void ConstraintSystem::optimizeDisjunctions( - SmallVectorImpl &disjunctions) { +namespace { + +NullablePtr getApplicableFnConstraint(ConstraintGraph &CG, + Constraint *disjunction) { + auto *boundVar = disjunction->getNestedConstraints()[0] + ->getFirstType() + ->getAs(); + if (!boundVar) + return nullptr; + + auto constraints = CG.gatherConstraints( + boundVar, ConstraintGraph::GatheringKind::EquivalenceClass, + [](Constraint *constraint) { + return constraint->getKind() == ConstraintKind::ApplicableFunction; + }); + + if (constraints.size() != 1) + return nullptr; + + auto *applicableFn = constraints.front(); + // Unapplied disjunction could appear as a argument to applicable function, + // we are not interested in that. + return applicableFn->getSecondType()->isEqual(boundVar) ? applicableFn + : nullptr; +} + +void forEachDisjunctionChoice( + ConstraintSystem &cs, Constraint *disjunction, + llvm::function_ref + callback) { + for (auto constraint : disjunction->getNestedConstraints()) { + if (constraint->isDisabled()) + continue; + + if (constraint->getKind() != ConstraintKind::BindOverload) + continue; + + auto choice = constraint->getOverloadChoice(); + auto *decl = choice.getDeclOrNull(); + if (!decl) + continue; + + // If disjunction choice is unavailable or disfavored we cannot + // do anything with it. + if (decl->getAttrs().hasAttribute() || + cs.isDeclUnavailable(decl, disjunction->getLocator())) + continue; + + Type overloadType = + cs.getEffectiveOverloadType(disjunction->getLocator(), choice, + /*allowMembers=*/true, cs.DC); + + if (!overloadType || !overloadType->is()) + continue; + + callback(constraint, decl, overloadType->castTo()); + } +} + +} // end anonymous namespace + +/// Given a set of disjunctions, attempt to determine +/// favored choices in the current context. +static void determineBestChoicesInContext( + ConstraintSystem &cs, SmallVectorImpl &disjunctions, + llvm::DenseMap> + &favorings) { + double bestOverallScore = 0.0; + // Tops scores across all of the disjunctions. + llvm::DenseMap disjunctionScores; + llvm::DenseMap> + favoredChoicesPerDisjunction; + + for (auto *disjunction : disjunctions) { + auto applicableFn = + getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); + + if (applicableFn.isNull()) + continue; + + auto argFuncType = + applicableFn.get()->getFirstType()->getAs(); + + SmallVector, 2>, 2> + candidateArgumentTypes; + candidateArgumentTypes.resize(argFuncType->getNumParams()); + + llvm::TinyPtrVector resultTypes; + + for (unsigned i = 0, n = argFuncType->getNumParams(); i != n; ++i) { + const auto ¶m = argFuncType->getParams()[i]; + auto argType = cs.simplifyType(param.getPlainType()); + + SmallVector, 2> types; + if (auto *typeVar = argType->getAs()) { + auto bindingSet = cs.getBindingsFor(typeVar, /*finalize=*/true); + + for (const auto &binding : bindingSet.Bindings) { + types.push_back({binding.BindingType, /*fromLiteral=*/false}); + } + + for (const auto &literal : bindingSet.Literals) { + if (literal.second.hasDefaultType()) { + // Add primary default type + types.push_back( + {literal.second.getDefaultType(), /*fromLiteral=*/true}); + } + } + } else { + types.push_back({argType, /*fromLiteral=*/false}); + } + + candidateArgumentTypes[i].append(types); + } + + auto resultType = cs.simplifyType(argFuncType->getResult()); + if (auto *typeVar = resultType->getAs()) { + auto bindingSet = cs.getBindingsFor(typeVar, /*finalize=*/true); + + for (const auto &binding : bindingSet.Bindings) { + resultTypes.push_back(binding.BindingType); + } + } else { + resultTypes.push_back(resultType); + } + + // The choice with the best score. + double bestScore = 0.0; + SmallVector, 2> favoredChoices; + + forEachDisjunctionChoice( + cs, disjunction, + [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { + // Don't consider generic overloads because we need conformance + // checking functionality to determine best favoring, preferring + // such overloads based only on concrete types leads to subpar + // choices due to missed information. + if (decl->getInterfaceType()->is()) + return; + + if (overloadType->getNumParams() != argFuncType->getNumParams()) + return; + + double score = 0.0; + for (unsigned i = 0, n = overloadType->getNumParams(); i != n; ++i) { + if (candidateArgumentTypes[i].empty()) + continue; + + const auto ¶m = overloadType->getParams()[i]; + const auto paramFlags = param.getParameterFlags(); + + // If parameter is variadic we cannot compare because we don't know + // real arity. + if (paramFlags.isVariadic()) + continue; + + auto paramType = param.getPlainType(); + + // FIXME: Let's skip matching function types for now + // because they have special rules for e.g. Concurrency + // (around @Sendable) and @convention(c). + if (paramType->is()) + continue; + + double argScore = 0.0; + for (auto const &candidate : candidateArgumentTypes[i]) { + auto candidateType = candidate.first; + + // `inout` parameter accepts only l-value argument. + if (paramFlags.isInOut() && !candidateType->is()) + continue; + + // The specifier only matters for `inout` check. + candidateType = candidateType->getWithoutSpecifierType(); + // Exact match on one of the candidate bindings. + if (candidateType->isEqual(paramType)) { + argScore = std::max( + argScore, /*fromLiteral=*/candidate.second ? 0.3 : 1.0); + } + } + + score += argScore; + } + + // Average the score to avoid disfavoring disjunctions with fewer + // parameters. + score /= overloadType->getNumParams(); + + // If one of the result types matches exactly, that's a good + // indication that overload choice should be favored. + // + // If nothing is known about the arguments it's only safe to + // check result for operators (except to standard comparison + // ones that all have the same result type), regular + // functions/methods and especially initializers could end up + // with a lot of favored overloads because on the result type alone. + if (score > 0 || + (decl->isOperator() && + !decl->getBaseIdentifier().isStandardComparisonOperator())) { + if (llvm::any_of( + resultTypes, [&overloadType](const Type candidateResultTy) { + auto overloadResultTy = overloadType->getResult(); + return candidateResultTy->isEqual(overloadResultTy); + })) { + score += 1.0; + } + } + + if (score > 0) { + favoredChoices.push_back({choice, score}); + bestScore = std::max(bestScore, score); + } + }); + + if (cs.isDebugMode()) { + PrintOptions PO; + PO.PrintTypesForDebugging = true; + + llvm::errs().indent(cs.solverState->getCurrentIndent()) + << "<<< Disjunction " + << disjunction->getNestedConstraints()[0]->getFirstType()->getString( + PO) + << " with score " << bestScore << "\n"; + } + + // No matching overload choices to favor. + if (bestScore == 0.0) + continue; + + bestOverallScore = std::max(bestOverallScore, bestScore); + + disjunctionScores[disjunction] = bestScore; + for (const auto &choice : favoredChoices) { + if (choice.second == bestScore) + favoredChoicesPerDisjunction[disjunction].push_back(choice.first); + } + } + + if (cs.isDebugMode() && bestOverallScore > 0) { + PrintOptions PO; + PO.PrintTypesForDebugging = true; + + auto getLogger = [&](unsigned extraIndent = 0) -> llvm::raw_ostream & { + return llvm::errs().indent(cs.solverState->getCurrentIndent() + + extraIndent); + }; + + { + auto &log = getLogger(); + log << "(Optimizing disjunctions: ["; + + interleave( + disjunctions, + [&](const auto *disjunction) { + log << disjunction->getNestedConstraints()[0] + ->getFirstType() + ->getString(PO); + }, + [&]() { log << ", "; }); + + log << "]\n"; + } + + getLogger(/*extraIndent=*/4) + << "Best overall score = " << bestOverallScore << '\n'; + + for (const auto &entry : disjunctionScores) { + getLogger(/*extraIndent=*/4) + << "[Disjunction '" + << entry.first->getNestedConstraints()[0]->getFirstType()->getString( + PO) + << "' with score = " << entry.second << '\n'; + + for (const auto *choice : favoredChoicesPerDisjunction[entry.first]) { + auto &log = getLogger(/*extraIndent=*/6); + + log << "- "; + choice->print(log, &cs.getASTContext().SourceMgr); + log << '\n'; + } + + getLogger(/*extraIdent=*/4) << "]\n"; + } + + getLogger() << ")\n"; + } + + for (auto &entry : disjunctionScores) { + if (entry.second != bestOverallScore) + continue; + + for (auto *choice : favoredChoicesPerDisjunction[entry.first]) + favorings[entry.first].push_back(choice); + } +} + +// Attempt to find a disjunction of bind constraints where all options +// in the disjunction are binding the same type variable. +// +// Prefer disjunctions where the bound type variable is also the +// right-hand side of a conversion constraint, since having a concrete +// type that we're converting to can make it possible to split the +// constraint system into multiple ones. +static Constraint * +selectBestBindingDisjunction(ConstraintSystem &cs, + SmallVectorImpl &disjunctions) { + + if (disjunctions.empty()) + return nullptr; + + auto getAsTypeVar = [&cs](Type type) { + return cs.simplifyType(type)->getRValueType()->getAs(); + }; + + Constraint *firstBindDisjunction = nullptr; + for (auto *disjunction : disjunctions) { + auto choices = disjunction->getNestedConstraints(); + assert(!choices.empty()); + + auto *choice = choices.front(); + if (choice->getKind() != ConstraintKind::Bind) + continue; + + // We can judge disjunction based on the single choice + // because all of choices (of bind overload set) should + // have the same left-hand side. + // Only do this for simple type variable bindings, not for + // bindings like: ($T1) -> $T2 bind String -> Int + auto *typeVar = getAsTypeVar(choice->getFirstType()); + if (!typeVar) + continue; + + if (!firstBindDisjunction) + firstBindDisjunction = disjunction; + + auto constraints = cs.getConstraintGraph().gatherConstraints( + typeVar, ConstraintGraph::GatheringKind::EquivalenceClass, + [](Constraint *constraint) { + return constraint->getKind() == ConstraintKind::Conversion; + }); + + for (auto *constraint : constraints) { + if (typeVar == getAsTypeVar(constraint->getSecondType())) + return disjunction; + } + } + + // If we had any binding disjunctions, return the first of + // those. These ensure that we attempt to bind types earlier than + // trying the elements of other disjunctions, which can often mean + // we fail faster. + return firstBindDisjunction; +} + +Constraint *ConstraintSystem::selectDisjunction() { + SmallVector disjunctions; + + collectDisjunctions(disjunctions); + if (disjunctions.empty()) + return nullptr; + + if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) + return disjunction; + + llvm::DenseMap> favorings; + determineBestChoicesInContext(*this, disjunctions, favorings); + + // Pick the disjunction with the smallest number of favored, then active + // choices. + auto bestDisjunction = std::min_element( + disjunctions.begin(), disjunctions.end(), + [&](Constraint *first, Constraint *second) -> bool { + unsigned firstActive = first->countActiveNestedConstraints(); + unsigned secondActive = second->countActiveNestedConstraints(); + unsigned firstFavored = favorings[first].size(); + unsigned secondFavored = favorings[second].size(); + + // Everything else equal, choose the disjunction with the greatest + // number of resolved argument types. The number of resolved argument + // types is always zero for disjunctions that don't represent applied + // overloads. + if (firstFavored == secondFavored) { + if (firstActive != secondActive) + return firstActive < secondActive; + + return first->countResolvedArgumentTypes(*this) > + second->countResolvedArgumentTypes(*this); + } + + firstFavored = firstFavored ? firstFavored : firstActive; + secondFavored = secondFavored ? secondFavored : secondActive; + return firstFavored < secondFavored; + }); + + if (bestDisjunction != disjunctions.end()) { + // If selected disjunction has any choices that should be favored + // let's record them now. + for (auto *choice : favorings[*bestDisjunction]) + favorConstraint(choice); + + return *bestDisjunction; + } + + return nullptr; } diff --git a/lib/Sema/CSSolver.cpp b/lib/Sema/CSSolver.cpp index 0cee95e848118..aeabc988e9cd4 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1297,62 +1297,6 @@ ConstraintSystem::filterDisjunction( return SolutionKind::Unsolved; } -// Attempt to find a disjunction of bind constraints where all options -// in the disjunction are binding the same type variable. -// -// Prefer disjunctions where the bound type variable is also the -// right-hand side of a conversion constraint, since having a concrete -// type that we're converting to can make it possible to split the -// constraint system into multiple ones. -static Constraint *selectBestBindingDisjunction( - ConstraintSystem &cs, SmallVectorImpl &disjunctions) { - - if (disjunctions.empty()) - return nullptr; - - auto getAsTypeVar = [&cs](Type type) { - return cs.simplifyType(type)->getRValueType()->getAs(); - }; - - Constraint *firstBindDisjunction = nullptr; - for (auto *disjunction : disjunctions) { - auto choices = disjunction->getNestedConstraints(); - assert(!choices.empty()); - - auto *choice = choices.front(); - if (choice->getKind() != ConstraintKind::Bind) - continue; - - // We can judge disjunction based on the single choice - // because all of choices (of bind overload set) should - // have the same left-hand side. - // Only do this for simple type variable bindings, not for - // bindings like: ($T1) -> $T2 bind String -> Int - auto *typeVar = getAsTypeVar(choice->getFirstType()); - if (!typeVar) - continue; - - if (!firstBindDisjunction) - firstBindDisjunction = disjunction; - - auto constraints = cs.getConstraintGraph().gatherConstraints( - typeVar, ConstraintGraph::GatheringKind::EquivalenceClass, - [](Constraint *constraint) { - return constraint->getKind() == ConstraintKind::Conversion; - }); - - for (auto *constraint : constraints) { - if (typeVar == getAsTypeVar(constraint->getSecondType())) - return disjunction; - } - } - - // If we had any binding disjunctions, return the first of - // those. These ensure that we attempt to bind types earlier than - // trying the elements of other disjunctions, which can often mean - // we fail faster. - return firstBindDisjunction; -} std::optional> ConstraintSystem::findConstraintThroughOptionals( @@ -1828,63 +1772,6 @@ void DisjunctionChoiceProducer::partitionDisjunction( assert(Ordering.size() == Choices.size()); } -Constraint *ConstraintSystem::selectDisjunction() { - SmallVector disjunctions; - - collectDisjunctions(disjunctions); - if (disjunctions.empty()) - return nullptr; - - optimizeDisjunctions(disjunctions); - - if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) - return disjunction; - - // Pick the disjunction with the smallest number of favored, then active choices. - auto cs = this; - auto minDisjunction = std::min_element(disjunctions.begin(), disjunctions.end(), - [&](Constraint *first, Constraint *second) -> bool { - unsigned firstActive = first->countActiveNestedConstraints(); - unsigned secondActive = second->countActiveNestedConstraints(); - unsigned firstFavored = first->countFavoredNestedConstraints(); - unsigned secondFavored = second->countFavoredNestedConstraints(); - - if (!isOperatorDisjunction(first) || !isOperatorDisjunction(second)) - return firstActive < secondActive; - - if (firstFavored == secondFavored) { - // Look for additional choices that are "favored" - SmallVector firstExisting; - SmallVector secondExisting; - - existingOperatorBindingsForDisjunction(*cs, first->getNestedConstraints(), firstExisting); - firstFavored += firstExisting.size(); - existingOperatorBindingsForDisjunction(*cs, second->getNestedConstraints(), secondExisting); - secondFavored += secondExisting.size(); - } - - // Everything else equal, choose the disjunction with the greatest - // number of resolved argument types. The number of resolved argument - // types is always zero for disjunctions that don't represent applied - // overloads. - if (firstFavored == secondFavored) { - if (firstActive != secondActive) - return firstActive < secondActive; - - return (first->countResolvedArgumentTypes(*this) > second->countResolvedArgumentTypes(*this)); - } - - firstFavored = firstFavored ? firstFavored : firstActive; - secondFavored = secondFavored ? secondFavored : secondActive; - return firstFavored < secondFavored; - }); - - if (minDisjunction != disjunctions.end()) - return *minDisjunction; - - return nullptr; -} - Constraint *ConstraintSystem::selectConjunction() { SmallVector conjunctions; for (auto &constraint : InactiveConstraints) { diff --git a/test/Constraints/common_type.swift b/test/Constraints/common_type.swift index 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/diag_ambiguities.swift b/test/Constraints/diag_ambiguities.swift index cb328fb0d5ed4..d6b358609c771 100644 --- a/test/Constraints/diag_ambiguities.swift +++ b/test/Constraints/diag_ambiguities.swift @@ -29,11 +29,11 @@ C(g) // expected-error{{ambiguous use of 'g'}} func h(_ x: T) -> () {} _ = C(h) // OK - init(_: (Int) -> ()) -func rdar29691909_callee(_ o: AnyObject?) -> Any? { return o } // expected-note {{found this candidate}} -func rdar29691909_callee(_ o: AnyObject) -> Any { return o } // expected-note {{found this candidate}} +func rdar29691909_callee(_ o: AnyObject?) -> Any? { return o } +func rdar29691909_callee(_ o: AnyObject) -> Any { return o } func rdar29691909(o: AnyObject) -> Any? { - return rdar29691909_callee(o) // expected-error{{ambiguous use of 'rdar29691909_callee'}} + return rdar29691909_callee(o) } func rdar29907555(_ value: Any!) -> String { diff --git a/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 66% rename from validation-test/Sema/type_checker_perf/slow/rdar35213699.swift rename to validation-test/Sema/type_checker_perf/fast/rdar35213699.swift index 5d89ab1541a89..ea333f993b6a3 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar35213699.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar35213699.swift @@ -3,6 +3,4 @@ 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) } } From c2f7451c7b21dd74af34f5a0b5e081634b3e3cdb Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 10 Feb 2023 10:50:03 -0800 Subject: [PATCH 05/60] [CSOptimizer] Favor SIMD related arithmetic operator choices if argument is SIMD type --- lib/Sema/CSOptimizer.cpp | 61 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 56 insertions(+), 5 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 9c64f8a08b46a..6b45983ab7254 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -85,6 +85,34 @@ void forEachDisjunctionChoice( } } +static bool isSIMDType(Type type) { + auto *NTD = dyn_cast_or_null(type->getAnyNominal()); + if (!NTD) + return false; + + auto typeName = NTD->getName().str(); + if (!typeName.startswith("SIMD")) + return false; + + return NTD->getParentModule()->getName().is("Swift"); +} + +static bool isArithmeticOperatorOnSIMDProtocol(ValueDecl *decl) { + if (!isSIMDOperator(decl)) + return false; + + if (!decl->getBaseIdentifier().isArithmeticOperator()) + return false; + + auto *DC = decl->getDeclContext(); + if (auto *P = DC->getSelfProtocolDecl()) { + if (auto knownKind = P->getKnownProtocolKind()) + return *knownKind == KnownProtocolKind::SIMD; + } + + return false; +} + } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -152,6 +180,23 @@ static void determineBestChoicesInContext( resultTypes.push_back(resultType); } + auto isViableOverload = [&](ValueDecl *decl) { + // Allow standard arithmetic operator overloads on SIMD protocol + // to be considered because we can favor them when then argument + // is a known SIMD type. + if (isArithmeticOperatorOnSIMDProtocol(decl)) + return true; + + // Don't consider generic overloads because we need conformance + // checking functionality to determine best favoring, preferring + // such overloads based only on concrete types leads to subpar + // choices due to missed information. + if (decl->getInterfaceType()->is()) + return false; + + return true; + }; + // The choice with the best score. double bestScore = 0.0; SmallVector, 2> favoredChoices; @@ -159,11 +204,7 @@ static void determineBestChoicesInContext( forEachDisjunctionChoice( cs, disjunction, [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { - // Don't consider generic overloads because we need conformance - // checking functionality to determine best favoring, preferring - // such overloads based only on concrete types leads to subpar - // choices due to missed information. - if (decl->getInterfaceType()->is()) + if (!isViableOverload(decl)) return; if (overloadType->getNumParams() != argFuncType->getNumParams()) @@ -204,6 +245,16 @@ static void determineBestChoicesInContext( if (candidateType->isEqual(paramType)) { argScore = std::max( argScore, /*fromLiteral=*/candidate.second ? 0.3 : 1.0); + continue; + } + + // If argument is SIMD type i.e. SIMD1<...> it's appropriate + // to favor of the overloads that are declared on SIMD protocol + // and expect a particular `Scalar` if it's known. + if (isSIMDType(candidateType) && + isArithmeticOperatorOnSIMDProtocol(decl)) { + argScore = std::max(argScore, 1.0); + continue; } } From a094c3ebb061ba25e501ade2e6fd010043666a39 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 10 Feb 2023 11:33:37 -0800 Subject: [PATCH 06/60] [CSOptimizer] Keep track of mismatches while evaluating candidates --- lib/Sema/CSOptimizer.cpp | 53 ++++++++++++++++++++++++++++++++++------ 1 file changed, 45 insertions(+), 8 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 6b45983ab7254..ec40bab9294a8 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -16,6 +16,7 @@ #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" @@ -231,20 +232,45 @@ static void determineBestChoicesInContext( if (paramType->is()) continue; - double argScore = 0.0; - for (auto const &candidate : candidateArgumentTypes[i]) { - auto candidateType = candidate.first; + if (candidateArgumentTypes[i].empty()) + continue; + + // The idea here is to match the parameter type against + // all of the argument candidate types and pick the best + // match (i.e. exact equality one). + // + // If none of the candidates match exactly and they are + // all bound concrete types, we consider this is mismatch + // at this parameter position and remove the overload choice + // from consideration. + + double bestCandidateScore = 0; + llvm::BitVector mismatches(candidateArgumentTypes[i].size()); + + for (unsigned candidateIdx : indices(candidateArgumentTypes[i])) { + // If one of the candidates matched exactly there is no reason + // to continue checking. + if (bestCandidateScore == 1) + break; + + Type candidateType; + bool isLiteralDefault; + + std::tie(candidateType, isLiteralDefault) = + candidateArgumentTypes[i][candidateIdx]; // `inout` parameter accepts only l-value argument. - if (paramFlags.isInOut() && !candidateType->is()) + if (paramFlags.isInOut() && !candidateType->is()) { + mismatches.set(candidateIdx); continue; + } // The specifier only matters for `inout` check. candidateType = candidateType->getWithoutSpecifierType(); // Exact match on one of the candidate bindings. if (candidateType->isEqual(paramType)) { - argScore = std::max( - argScore, /*fromLiteral=*/candidate.second ? 0.3 : 1.0); + bestCandidateScore = + std::max(bestCandidateScore, isLiteralDefault ? 0.3 : 1.0); continue; } @@ -253,12 +279,23 @@ static void determineBestChoicesInContext( // and expect a particular `Scalar` if it's known. if (isSIMDType(candidateType) && isArithmeticOperatorOnSIMDProtocol(decl)) { - argScore = std::max(argScore, 1.0); + bestCandidateScore = 1.0; continue; } + + // Only established arguments could be considered mismatches, + // literal default types should be regarded as holes if they + // didn't match. + if (!isLiteralDefault && !candidateType->hasTypeVariable()) + mismatches.set(candidateIdx); } - score += argScore; + // If none of the candidates for this parameter matched, let's + // drop this overload from any further consideration. + if (mismatches.all()) + return; + + score += bestCandidateScore; } // Average the score to avoid disfavoring disjunctions with fewer From e404ed722a99f880e3dbcf163283ba31da06c4e5 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 10 Feb 2023 14:54:06 -0800 Subject: [PATCH 07/60] [CSStep] Don't favor choices until the disjunction is picked --- include/swift/Sema/ConstraintSystem.h | 18 +++++++++++++----- lib/Sema/CSOptimizer.cpp | 19 +++++++------------ lib/Sema/CSStep.cpp | 9 +++++---- lib/Sema/CSStep.h | 10 +++++++++- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 8fe41eef14147..2289cd8b195bc 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5213,8 +5213,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. + Optional>> + selectDisjunction(); /// Pick a conjunction from the InactiveConstraints list. /// @@ -6143,7 +6144,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), @@ -6153,6 +6155,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); } @@ -6197,8 +6204,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/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index ec40bab9294a8..c9920a3201c28 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -468,15 +468,16 @@ selectBestBindingDisjunction(ConstraintSystem &cs, return firstBindDisjunction; } -Constraint *ConstraintSystem::selectDisjunction() { +Optional>> +ConstraintSystem::selectDisjunction() { SmallVector disjunctions; collectDisjunctions(disjunctions); if (disjunctions.empty()) - return nullptr; + return None; if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) - return disjunction; + return std::make_pair(disjunction, llvm::TinyPtrVector()); llvm::DenseMap> favorings; determineBestChoicesInContext(*this, disjunctions, favorings); @@ -508,14 +509,8 @@ Constraint *ConstraintSystem::selectDisjunction() { return firstFavored < secondFavored; }); - if (bestDisjunction != disjunctions.end()) { - // If selected disjunction has any choices that should be favored - // let's record them now. - for (auto *choice : favorings[*bestDisjunction]) - favorConstraint(choice); - - return *bestDisjunction; - } + if (bestDisjunction != disjunctions.end()) + return std::make_pair(*bestDisjunction, favorings[*bestDisjunction]); - return nullptr; + return None; } diff --git a/lib/Sema/CSStep.cpp b/lib/Sema/CSStep.cpp index d186a4960040b..c30640a993b7e 100644 --- a/lib/Sema/CSStep.cpp +++ b/lib/Sema/CSStep.cpp @@ -360,7 +360,7 @@ StepResult ComponentStep::take(bool prevFailed) { } }); - auto *disjunction = CS.selectDisjunction(); + auto disjunction = CS.selectDisjunction(); auto *conjunction = CS.selectConjunction(); if (CS.isDebugMode()) { @@ -403,7 +403,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)) @@ -426,9 +427,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 3b8c39e4a015d..e446b07b5dd29 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -677,9 +677,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 7c1c46d4e4480d78c1dc3ef5447fd616411a5dfe Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 11 Feb 2023 00:28:42 -0800 Subject: [PATCH 08/60] [CSOptimizer] Make sure that all parameters without arguments are defaulted --- lib/Sema/CSOptimizer.cpp | 20 +++++++++++++++++--- test/Constraints/ranking.swift | 8 ++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c9920a3201c28..a7f3a462d5c3b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -208,15 +208,29 @@ static void determineBestChoicesInContext( if (!isViableOverload(decl)) return; - if (overloadType->getNumParams() != argFuncType->getNumParams()) - return; + ParameterListInfo paramListInfo( + overloadType->getParams(), decl, + hasAppliedSelf(cs, choice->getOverloadChoice())); double score = 0.0; for (unsigned i = 0, n = overloadType->getNumParams(); i != n; ++i) { + const auto ¶m = overloadType->getParams()[i]; + + if (i >= candidateArgumentTypes.size()) { + // If parameter has a default - continue matching, + // all of the subsequence parameters should have defaults + // as well, if they don't the overload choice in not viable. + if (paramListInfo.hasDefaultArgument(i)) + continue; + + // Early return because this overload choice is not viable + // without default value for the current parameter. + return; + } + if (candidateArgumentTypes[i].empty()) continue; - const auto ¶m = overloadType->getParams()[i]; const auto paramFlags = param.getParameterFlags(); // If parameter is variadic we cannot compare because we don't know diff --git a/test/Constraints/ranking.swift b/test/Constraints/ranking.swift index 82d29791e6f14..74e3b6842f02e 100644 --- a/test/Constraints/ranking.swift +++ b/test/Constraints/ranking.swift @@ -450,3 +450,11 @@ struct HasIntInit { func compare_solutions_with_bindings(x: UInt8, y: UInt8) -> HasIntInit { return .init(Int(x / numericCast(y))) } + +// Test to make sure that previous favoring behavior is maintained and @autoclosure makes a difference. +func test_no_ambiguity_with_autoclosure(x: Int) { + func test(_ condition: Bool, file: StaticString = #file, line: UInt = #line) {} + func test(_ condition: @autoclosure () -> Bool, file: StaticString = #file, line: UInt = #line) {} + + test(x >= 0) // Ok +} From bc5f70a9a3b2b0f3664e8a1fcb20acc693217cc0 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 13 Feb 2023 13:14:17 -0800 Subject: [PATCH 09/60] [CSOptimizer] Allow generic operator overloads without associated type parameters --- lib/Sema/CSOptimizer.cpp | 117 +++++++++--------- .../{slow => fast}/rdar17170728.swift | 1 - 2 files changed, 59 insertions(+), 59 deletions(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar17170728.swift (74%) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index a7f3a462d5c3b..b1634d916589d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -14,6 +14,8 @@ // //===----------------------------------------------------------------------===// +#include "TypeChecker.h" +#include "swift/AST/GenericSignature.h" #include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" #include "llvm/ADT/BitVector.h" @@ -86,34 +88,6 @@ void forEachDisjunctionChoice( } } -static bool isSIMDType(Type type) { - auto *NTD = dyn_cast_or_null(type->getAnyNominal()); - if (!NTD) - return false; - - auto typeName = NTD->getName().str(); - if (!typeName.startswith("SIMD")) - return false; - - return NTD->getParentModule()->getName().is("Swift"); -} - -static bool isArithmeticOperatorOnSIMDProtocol(ValueDecl *decl) { - if (!isSIMDOperator(decl)) - return false; - - if (!decl->getBaseIdentifier().isArithmeticOperator()) - return false; - - auto *DC = decl->getDeclContext(); - if (auto *P = DC->getSelfProtocolDecl()) { - if (auto knownKind = P->getKnownProtocolKind()) - return *knownKind == KnownProtocolKind::SIMD; - } - - return false; -} - } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -181,23 +155,6 @@ static void determineBestChoicesInContext( resultTypes.push_back(resultType); } - auto isViableOverload = [&](ValueDecl *decl) { - // Allow standard arithmetic operator overloads on SIMD protocol - // to be considered because we can favor them when then argument - // is a known SIMD type. - if (isArithmeticOperatorOnSIMDProtocol(decl)) - return true; - - // Don't consider generic overloads because we need conformance - // checking functionality to determine best favoring, preferring - // such overloads based only on concrete types leads to subpar - // choices due to missed information. - if (decl->getInterfaceType()->is()) - return false; - - return true; - }; - // The choice with the best score. double bestScore = 0.0; SmallVector, 2> favoredChoices; @@ -205,8 +162,21 @@ static void determineBestChoicesInContext( forEachDisjunctionChoice( cs, disjunction, [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { - if (!isViableOverload(decl)) - return; + GenericSignature genericSig; + { + if (auto *GF = dyn_cast(decl)) { + genericSig = GF->getGenericSignature(); + } else if (auto *SD = dyn_cast(decl)) { + genericSig = SD->getGenericSignature(); + } + + // Let's not consider non-operator generic overloads because we + // need conformance checking functionality to determine best + // favoring, preferring such overloads based on concrete types + // alone leads to subpar choices due to missed information. + if (genericSig && !decl->isOperator()) + return; + } ParameterListInfo paramListInfo( overloadType->getParams(), decl, @@ -249,6 +219,23 @@ static void determineBestChoicesInContext( if (candidateArgumentTypes[i].empty()) continue; + // Check protocol requirement(s) if this parameter is a + // generic parameter type. + GenericSignature::RequiredProtocols protocolRequirements; + if (genericSig) { + if (auto *GP = paramType->getAs()) { + protocolRequirements = genericSig->getRequiredProtocols(GP); + // It's a generic parameter which might be connected via + // same-type constraints to other generic parameters but + // we cannot check that here, so let's ignore it. + if (protocolRequirements.empty()) + continue; + } + + if (paramType->getAs()) + return; + } + // The idea here is to match the parameter type against // all of the argument candidate types and pick the best // match (i.e. exact equality one). @@ -281,22 +268,36 @@ static void determineBestChoicesInContext( // The specifier only matters for `inout` check. candidateType = candidateType->getWithoutSpecifierType(); - // Exact match on one of the candidate bindings. - if (candidateType->isEqual(paramType)) { + + // We don't check generic requirements against literal default + // types because it creates more noise than signal for operators. + if (!protocolRequirements.empty() && !isLiteralDefault) { + if (llvm::all_of( + protocolRequirements, [&](ProtocolDecl *protocol) { + return TypeChecker::conformsToProtocol( + candidateType, protocol, cs.DC->getParentModule(), + /*allowMissing=*/false); + })) { + // Score is lower here because we still prefer concrete + // overloads over the generic ones when possible. + bestCandidateScore = std::max(bestCandidateScore, 0.7); + continue; + } + } else if (paramType->hasTypeParameter()) { + // i.e. Array or Optional as a parameter. + // This is slightly better than all of the conformances matching + // because the parameter is concrete and could split the graph. + if (paramType->getAnyNominal() == candidateType->getAnyNominal()) { + bestCandidateScore = std::max(bestCandidateScore, 0.8); + continue; + } + } else if (candidateType->isEqual(paramType)) { + // Exact match on one of the candidate bindings. bestCandidateScore = std::max(bestCandidateScore, isLiteralDefault ? 0.3 : 1.0); continue; } - // If argument is SIMD type i.e. SIMD1<...> it's appropriate - // to favor of the overloads that are declared on SIMD protocol - // and expect a particular `Scalar` if it's known. - if (isSIMDType(candidateType) && - isArithmeticOperatorOnSIMDProtocol(decl)) { - bestCandidateScore = 1.0; - continue; - } - // Only established arguments could be considered mismatches, // literal default types should be regarded as holes if they // didn't match. diff --git a/validation-test/Sema/type_checker_perf/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)) } From 14e2a16fced65b6e4b2130af91957556b0efbfb8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 15 Feb 2023 11:56:33 -0800 Subject: [PATCH 10/60] [CSOptimizer] Don't attempt to optimize calls with code completion token(s) in argument position --- lib/Sema/CSOptimizer.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index b1634d916589d..90f528e622c7d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -112,6 +112,10 @@ static void determineBestChoicesInContext( auto argFuncType = applicableFn.get()->getFirstType()->getAs(); + auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); + if (!argumentList || cs.containsIDEInspectionTarget(argumentList)) + return; + SmallVector, 2>, 2> candidateArgumentTypes; candidateArgumentTypes.resize(argFuncType->getNumParams()); From cb1cb2018dc7662cd8af08633c17b876b187b377 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 15 Feb 2023 15:11:24 -0800 Subject: [PATCH 11/60] [CSOptimizer] Use `matchCallArguments` to establish argument-to-parameter relationships This helps to find defaulted parameter positions as well. --- lib/Sema/CSOptimizer.cpp | 77 ++++++++++++++++++++++++++++------------ 1 file changed, 55 insertions(+), 22 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 90f528e622c7d..76de2d72197bf 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -116,6 +116,13 @@ static void determineBestChoicesInContext( if (!argumentList || cs.containsIDEInspectionTarget(argumentList)) return; + SmallVector argsWithLabels; + { + argsWithLabels.append(argFuncType->getParams().begin(), + argFuncType->getParams().end()); + FunctionType::relabelParams(argsWithLabels, argumentList); + } + SmallVector, 2>, 2> candidateArgumentTypes; candidateArgumentTypes.resize(argFuncType->getNumParams()); @@ -159,6 +166,27 @@ static void determineBestChoicesInContext( resultTypes.push_back(resultType); } + // Match arguments to the given overload choice. + auto matchArguments = + [&](OverloadChoice choice, + FunctionType *overloadType) -> Optional { + auto *decl = choice.getDeclOrNull(); + assert(decl); + + auto hasAppliedSelf = + decl->hasCurriedSelf() && + doesMemberRefApplyCurriedSelf(choice.getBaseType(), decl); + + ParameterListInfo paramListInfo(overloadType->getParams(), decl, + hasAppliedSelf); + + MatchCallArgumentListener listener; + return matchCallArguments(argsWithLabels, overloadType->getParams(), + paramListInfo, + argumentList->getFirstTrailingClosureIndex(), + /*allow fixes*/ false, listener, None); + }; + // The choice with the best score. double bestScore = 0.0; SmallVector, 2> favoredChoices; @@ -182,27 +210,35 @@ static void determineBestChoicesInContext( return; } - ParameterListInfo paramListInfo( - overloadType->getParams(), decl, - hasAppliedSelf(cs, choice->getOverloadChoice())); + auto matchings = + matchArguments(choice->getOverloadChoice(), overloadType); + if (!matchings) + return; double score = 0.0; - for (unsigned i = 0, n = overloadType->getNumParams(); i != n; ++i) { - const auto ¶m = overloadType->getParams()[i]; - - if (i >= candidateArgumentTypes.size()) { - // If parameter has a default - continue matching, - // all of the subsequence parameters should have defaults - // as well, if they don't the overload choice in not viable. - if (paramListInfo.hasDefaultArgument(i)) - continue; + for (unsigned paramIdx = 0, n = overloadType->getNumParams(); + paramIdx != n; ++paramIdx) { + const auto ¶m = overloadType->getParams()[paramIdx]; + + auto argIndices = matchings->parameterBindings[paramIdx]; + switch (argIndices.size()) { + case 0: + // Current parameter is defaulted. + continue; + + case 1: + // One-to-one match between argument and parameter. + break; - // Early return because this overload choice is not viable - // without default value for the current parameter. + default: + // Cannot deal with multiple possible matchings at the moment. return; } - if (candidateArgumentTypes[i].empty()) + auto argIdx = argIndices.front(); + + // Looks like there is nothing know about the argument. + if (candidateArgumentTypes[argIdx].empty()) continue; const auto paramFlags = param.getParameterFlags(); @@ -220,9 +256,6 @@ static void determineBestChoicesInContext( if (paramType->is()) continue; - if (candidateArgumentTypes[i].empty()) - continue; - // Check protocol requirement(s) if this parameter is a // generic parameter type. GenericSignature::RequiredProtocols protocolRequirements; @@ -248,11 +281,11 @@ static void determineBestChoicesInContext( // all bound concrete types, we consider this is mismatch // at this parameter position and remove the overload choice // from consideration. - double bestCandidateScore = 0; - llvm::BitVector mismatches(candidateArgumentTypes[i].size()); + llvm::BitVector mismatches(candidateArgumentTypes[argIdx].size()); - for (unsigned candidateIdx : indices(candidateArgumentTypes[i])) { + for (unsigned candidateIdx : + indices(candidateArgumentTypes[argIdx])) { // If one of the candidates matched exactly there is no reason // to continue checking. if (bestCandidateScore == 1) @@ -262,7 +295,7 @@ static void determineBestChoicesInContext( bool isLiteralDefault; std::tie(candidateType, isLiteralDefault) = - candidateArgumentTypes[i][candidateIdx]; + candidateArgumentTypes[argIdx][candidateIdx]; // `inout` parameter accepts only l-value argument. if (paramFlags.isInOut() && !candidateType->is()) { From 11b897b32f92ab735915910782e2c9bab00fe3f2 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 16 Feb 2023 16:32:35 -0800 Subject: [PATCH 12/60] [CSOptimizer] Relax candidate type requirements from equality to set of no-impact conversions Candidate is viable (with some score) if: - Candidate is exactly equal to a parameter type - Candidate type differs from a parameter type only in optionality - Parameter is a generic parameter type and all conformances are matched by a candidate type - Candidate tuples matches a parameter tuple on arity - Candidate is an `Array` and parameter is an `Unsafe*Pointer` - Candidate is a subclass of a parameter class type - Candidate is a concrete type and parameter is its existential value (except Any) --- lib/Sema/CSOptimizer.cpp | 221 +++++++++++++++++++----- test/Constraints/diag_ambiguities.swift | 6 +- test/expr/expressions.swift | 8 +- 3 files changed, 180 insertions(+), 55 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 76de2d72197bf..7779f3cda3b9d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -15,13 +15,16 @@ //===----------------------------------------------------------------------===// #include "TypeChecker.h" +#include "swift/AST/ExistentialLayout.h" #include "swift/AST/GenericSignature.h" +#include "swift/Basic/OptionSet.h" #include "swift/Sema/ConstraintGraph.h" #include "swift/Sema/ConstraintSystem.h" #include "llvm/ADT/BitVector.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/TinyPtrVector.h" +#include "llvm/Support/SaveAndRestore.h" #include "llvm/Support/raw_ostream.h" #include #include @@ -187,6 +190,162 @@ static void determineBestChoicesInContext( /*allow fixes*/ false, listener, None); }; + // Determine whether the candidate type is a subclass of the superclass + // type. + std::function isSubclassOf = [&](Type candidateType, + Type superclassType) { + // Conversion from a concrete type to its existential value. + if (superclassType->isExistentialType() && !superclassType->isAny()) { + auto layout = superclassType->getExistentialLayout(); + + if (auto layoutConstraint = layout.getLayoutConstraint()) { + if (layoutConstraint->isClass() && + !(candidateType->isClassExistentialType() || + candidateType->mayHaveSuperclass())) + return false; + } + + if (layout.explicitSuperclass && + !isSubclassOf(candidateType, layout.explicitSuperclass)) + return false; + + return llvm::all_of(layout.getProtocols(), [&](ProtocolDecl *P) { + if (auto superclass = P->getSuperclass()) { + if (!isSubclassOf(candidateType, superclass)) + return false; + } + + return bool(TypeChecker::containsProtocol( + candidateType, P, cs.DC->getParentModule(), + /*skipConditionalRequirements=*/true, + /*allowMissing=*/false)); + }); + } + + auto *subclassDecl = candidateType->getClassOrBoundGenericClass(); + auto *superclassDecl = superclassType->getClassOrBoundGenericClass(); + + if (!(subclassDecl && superclassDecl)) + return false; + + return superclassDecl->isSuperclassOf(subclassDecl); + }; + + enum class MatchFlag { + OnParam = 0x01, + Literal = 0x02, + }; + + using MatchOptions = OptionSet; + + // Perform a limited set of checks to determine whether the candidate + // could possibly match the parameter type: + // + // - Equality + // - Protocol conformance(s) + // - Optional injection + // - Superclass conversion + // - Array-to-pointer conversion + // - Value to existential conversion + // - Exact match on top-level types + std::function + scoreCandidateMatch = [&](GenericSignature genericSig, + Type candidateType, Type paramType, + MatchOptions options) -> double { + // Dependent members cannot be handled here because + // they require substitution of the base type which + // could come from a different argument. + if (paramType->getAs()) + return 0; + + // Exact match between candidate and parameter types. + if (candidateType->isEqual(paramType)) + return options.contains(MatchFlag::Literal) ? 0.3 : 1; + + if (options.contains(MatchFlag::Literal)) + return 0; + + // Check whether match would require optional injection. + { + SmallVector candidateOptionals; + SmallVector paramOptionals; + + candidateType = + candidateType->lookThroughAllOptionalTypes(candidateOptionals); + paramType = paramType->lookThroughAllOptionalTypes(paramOptionals); + + if (!candidateOptionals.empty() || !paramOptionals.empty()) { + if (paramOptionals.size() >= candidateOptionals.size()) + return scoreCandidateMatch(genericSig, candidateType, paramType, + options); + + // Optionality mismatch. + return 0; + } + } + + // Candidate could be injected into optional parameter type + // or converted to a superclass. + if (isSubclassOf(candidateType, paramType)) + return 1; + + // Possible Array -> Unsafe*Pointer conversion. + if (options.contains(MatchFlag::OnParam)) { + if (candidateType->isArrayType() && + paramType->getAnyPointerElementType()) + return 1; + } + + // If both argument and parameter are tuples of the same arity, + // it's a match. + { + if (auto *candidateTuple = candidateType->getAs()) { + auto *paramTuple = paramType->getAs(); + if (paramTuple && + candidateTuple->getNumElements() == paramTuple->getNumElements()) + return 1; + } + } + + // Check protocol requirement(s) if this parameter is a + // generic parameter type. + GenericSignature::RequiredProtocols protocolRequirements; + if (genericSig) { + if (auto *GP = paramType->getAs()) { + protocolRequirements = genericSig->getRequiredProtocols(GP); + // It's a generic parameter which might be connected via + // same-type constraints to other generic parameters but + // we cannot check that here, so let's add a tiny score + // just to acknowledge that it could possibly match. + if (protocolRequirements.empty()) { + return 0.01; + } + + if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { + return TypeChecker::conformsToProtocol(candidateType, protocol, + cs.DC->getParentModule(), + /*allowMissing=*/false); + })) + return 0.7; + } + } + + // Parameter is generic, let's check whether top-level + // types match i.e. Array as a parameter. + // + // This is slightly better than all of the conformances matching + // because the parameter is concrete and could split the graph. + if (paramType->hasTypeParameter()) { + auto *candidateDecl = candidateType->getAnyNominal(); + auto *paramDecl = paramType->getAnyNominal(); + + if (candidateDecl && paramDecl && candidateDecl == paramDecl) + return 0.8; + } + + return 0; + }; + // The choice with the best score. double bestScore = 0.0; SmallVector, 2> favoredChoices; @@ -256,23 +415,6 @@ static void determineBestChoicesInContext( if (paramType->is()) continue; - // Check protocol requirement(s) if this parameter is a - // generic parameter type. - GenericSignature::RequiredProtocols protocolRequirements; - if (genericSig) { - if (auto *GP = paramType->getAs()) { - protocolRequirements = genericSig->getRequiredProtocols(GP); - // It's a generic parameter which might be connected via - // same-type constraints to other generic parameters but - // we cannot check that here, so let's ignore it. - if (protocolRequirements.empty()) - continue; - } - - if (paramType->getAs()) - return; - } - // The idea here is to match the parameter type against // all of the argument candidate types and pick the best // match (i.e. exact equality one). @@ -306,32 +448,14 @@ static void determineBestChoicesInContext( // The specifier only matters for `inout` check. candidateType = candidateType->getWithoutSpecifierType(); - // We don't check generic requirements against literal default - // types because it creates more noise than signal for operators. - if (!protocolRequirements.empty() && !isLiteralDefault) { - if (llvm::all_of( - protocolRequirements, [&](ProtocolDecl *protocol) { - return TypeChecker::conformsToProtocol( - candidateType, protocol, cs.DC->getParentModule(), - /*allowMissing=*/false); - })) { - // Score is lower here because we still prefer concrete - // overloads over the generic ones when possible. - bestCandidateScore = std::max(bestCandidateScore, 0.7); - continue; - } - } else if (paramType->hasTypeParameter()) { - // i.e. Array or Optional as a parameter. - // This is slightly better than all of the conformances matching - // because the parameter is concrete and could split the graph. - if (paramType->getAnyNominal() == candidateType->getAnyNominal()) { - bestCandidateScore = std::max(bestCandidateScore, 0.8); - continue; - } - } else if (candidateType->isEqual(paramType)) { - // Exact match on one of the candidate bindings. - bestCandidateScore = - std::max(bestCandidateScore, isLiteralDefault ? 0.3 : 1.0); + MatchOptions options(MatchFlag::OnParam); + if (isLiteralDefault) + options |= MatchFlag::Literal; + + auto score = scoreCandidateMatch(genericSig, candidateType, + paramType, options); + if (score > 0) { + bestCandidateScore = std::max(bestCandidateScore, score); continue; } @@ -365,11 +489,12 @@ static void determineBestChoicesInContext( if (score > 0 || (decl->isOperator() && !decl->getBaseIdentifier().isStandardComparisonOperator())) { - if (llvm::any_of( - resultTypes, [&overloadType](const Type candidateResultTy) { - auto overloadResultTy = overloadType->getResult(); - return candidateResultTy->isEqual(overloadResultTy); - })) { + if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) { + return scoreCandidateMatch(genericSig, + overloadType->getResult(), + candidateResultTy, + /*options=*/{}) > 0; + })) { score += 1.0; } } diff --git a/test/Constraints/diag_ambiguities.swift b/test/Constraints/diag_ambiguities.swift index d6b358609c771..cb328fb0d5ed4 100644 --- a/test/Constraints/diag_ambiguities.swift +++ b/test/Constraints/diag_ambiguities.swift @@ -29,11 +29,11 @@ C(g) // expected-error{{ambiguous use of 'g'}} func h(_ x: T) -> () {} _ = C(h) // OK - init(_: (Int) -> ()) -func rdar29691909_callee(_ o: AnyObject?) -> Any? { return o } -func rdar29691909_callee(_ o: AnyObject) -> Any { return o } +func rdar29691909_callee(_ o: AnyObject?) -> Any? { return o } // expected-note {{found this candidate}} +func rdar29691909_callee(_ o: AnyObject) -> Any { return o } // expected-note {{found this candidate}} func rdar29691909(o: AnyObject) -> Any? { - return rdar29691909_callee(o) + return rdar29691909_callee(o) // expected-error{{ambiguous use of 'rdar29691909_callee'}} } func rdar29907555(_ value: Any!) -> String { diff --git a/test/expr/expressions.swift b/test/expr/expressions.swift index d7ddc5fc0c765..4a4d8c3232ce8 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 Any.Type' to 'nil' always returns false}} -_ = nil == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns false}} -_ = Int.self != nil // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns true}} -_ = nil != Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'nil' always returns true}} +_ = Int.self == 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 Any.Type' to 'Optional.none' always returns false}} _ = .none == Int.self // expected-warning {{comparing non-optional value of type 'any Any.Type' to 'Optional.none' always returns false}} From 2c44e379488ef1476d4aa5c0f86f58ec040e6de9 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 17 Feb 2023 15:57:10 -0800 Subject: [PATCH 13/60] [CSStep] Remove disjunction pruning logic from DisjunctionStep Since old favoring mechanism has been removed this is dead code now because nothing is going to equate disjunction constraints. --- lib/Sema/CSStep.h | 46 ---------------------------------------------- 1 file changed, 46 deletions(-) diff --git a/lib/Sema/CSStep.h b/lib/Sema/CSStep.h index e446b07b5dd29..a3e75b93f0136 100644 --- a/lib/Sema/CSStep.h +++ b/lib/Sema/CSStep.h @@ -671,8 +671,6 @@ class TypeVariableStep final : public BindingStep { class DisjunctionStep final : public BindingStep { Constraint *Disjunction; - SmallVector DisabledChoices; - std::optional BestNonGenericScore; std::optional> LastSolvedChoice; @@ -689,16 +687,12 @@ class DisjunctionStep final : public BindingStep { : BindingStep(cs, {cs, disjunction, favoredChoices}, solutions), Disjunction(disjunction) { assert(Disjunction->getKind() == ConstraintKind::Disjunction); - pruneOverloadSet(Disjunction); ++cs.solverState->NumDisjunctions; } ~DisjunctionStep() override { // Rewind back any changes left after attempting last choice. ActiveChoice.reset(); - // Re-enable previously disabled overload choices. - for (auto *choice : DisabledChoices) - choice->setEnabled(); } StepResult resume(bool prevFailed) override; @@ -751,46 +745,6 @@ class DisjunctionStep final : public BindingStep { /// simplified further, false otherwise. bool attempt(const DisjunctionChoice &choice) override; - // Check if selected disjunction has a representative - // this might happen when there are multiple binary operators - // chained together. If so, disable choices which differ - // from currently selected representative. - void pruneOverloadSet(Constraint *disjunction) { - auto *choice = disjunction->getNestedConstraints().front(); - if (choice->getKind() != ConstraintKind::BindOverload) - return; - - auto *typeVar = choice->getFirstType()->getAs(); - if (!typeVar) - return; - - auto *repr = typeVar->getImpl().getRepresentative(nullptr); - if (!repr || repr == typeVar) - return; - - for (auto overload : CS.getResolvedOverloads()) { - auto resolved = overload.second; - if (!resolved.boundType->isEqual(repr)) - continue; - - auto &representative = resolved.choice; - if (!representative.isDecl()) - return; - - // Disable all of the overload choices which are different from - // the one which is currently picked for representative. - for (auto *constraint : disjunction->getNestedConstraints()) { - auto choice = constraint->getOverloadChoice(); - if (!choice.isDecl() || choice.getDecl() == representative.getDecl()) - continue; - - constraint->setDisabled(); - DisabledChoices.push_back(constraint); - } - break; - } - }; - // Figure out which of the solutions has the smallest score. static std::optional getBestScore(SmallVectorImpl &solutions) { From 957a5f4270ce9820f8be8d03a0c4e2f01db37d0e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 21 Feb 2023 13:29:43 -0800 Subject: [PATCH 14/60] [CSOptimizer] Treat all type parameters equally Dependent members just like generic parameter types could have associated protocol conformances which give us a glimpse into whether arguments could possibly match. --- lib/Sema/CSOptimizer.cpp | 41 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 7779f3cda3b9d..3b95edd9abbfb 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -252,12 +252,6 @@ static void determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> double { - // Dependent members cannot be handled here because - // they require substitution of the base type which - // could come from a different argument. - if (paramType->getAs()) - return 0; - // Exact match between candidate and parameter types. if (candidateType->isEqual(paramType)) return options.contains(MatchFlag::Literal) ? 0.3 : 1; @@ -309,25 +303,22 @@ static void determineBestChoicesInContext( // Check protocol requirement(s) if this parameter is a // generic parameter type. - GenericSignature::RequiredProtocols protocolRequirements; - if (genericSig) { - if (auto *GP = paramType->getAs()) { - protocolRequirements = genericSig->getRequiredProtocols(GP); - // It's a generic parameter which might be connected via - // same-type constraints to other generic parameters but - // we cannot check that here, so let's add a tiny score - // just to acknowledge that it could possibly match. - if (protocolRequirements.empty()) { - return 0.01; - } - - if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { - return TypeChecker::conformsToProtocol(candidateType, protocol, - cs.DC->getParentModule(), - /*allowMissing=*/false); - })) - return 0.7; - } + if (genericSig && paramType->isTypeParameter()) { + auto protocolRequirements = genericSig->getRequiredProtocols(paramType); + // It's a generic parameter or dependent member which might + // be connected via ame-type constraints to other generic + // parameters or dependent member but we cannot check that here, + // so let's add a tiny score just to acknowledge that it could + // possibly match. + if (protocolRequirements.empty()) + return 0.01; + + if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { + return TypeChecker::conformsToProtocol(candidateType, protocol, + cs.DC->getParentModule(), + /*allowMissing=*/false); + })) + return 0.7; } // Parameter is generic, let's check whether top-level From da65333d41d35601c9d3dc29d3441b527a516cea Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 10 Oct 2023 13:35:29 -0700 Subject: [PATCH 15/60] [CSOptimizer] NFC: Adjust conformance check to use `ConstraintSystem::lookupConformance` --- lib/Sema/CSOptimizer.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 3b95edd9abbfb..d6497eec1dd6b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -314,9 +314,7 @@ static void determineBestChoicesInContext( return 0.01; if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { - return TypeChecker::conformsToProtocol(candidateType, protocol, - cs.DC->getParentModule(), - /*allowMissing=*/false); + return bool(cs.lookupConformance(candidateType, protocol)); })) return 0.7; } From 0fc68069221dfba4dc93f479ed7941665f7dd3fa Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 10 Oct 2023 14:29:37 -0700 Subject: [PATCH 16/60] [CSOptimizer] NFC: Switch to llvm::Optional --- include/swift/Sema/ConstraintSystem.h | 2 +- lib/Sema/CSOptimizer.cpp | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 2289cd8b195bc..12cd5091104f6 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5214,7 +5214,7 @@ class ConstraintSystem { /// Pick a disjunction from the InactiveConstraints list. /// /// \returns The selected disjunction and a set of it's favored choices. - Optional>> + llvm::Optional>> selectDisjunction(); /// Pick a conjunction from the InactiveConstraints list. diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index d6497eec1dd6b..baa45eb7ef81b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -170,9 +170,8 @@ static void determineBestChoicesInContext( } // Match arguments to the given overload choice. - auto matchArguments = - [&](OverloadChoice choice, - FunctionType *overloadType) -> Optional { + auto matchArguments = [&](OverloadChoice choice, FunctionType *overloadType) + -> llvm::Optional { auto *decl = choice.getDeclOrNull(); assert(decl); @@ -187,7 +186,7 @@ static void determineBestChoicesInContext( return matchCallArguments(argsWithLabels, overloadType->getParams(), paramListInfo, argumentList->getFirstTrailingClosureIndex(), - /*allow fixes*/ false, listener, None); + /*allow fixes*/ false, listener, llvm::None); }; // Determine whether the candidate type is a subclass of the superclass @@ -634,13 +633,13 @@ selectBestBindingDisjunction(ConstraintSystem &cs, return firstBindDisjunction; } -Optional>> +llvm::Optional>> ConstraintSystem::selectDisjunction() { SmallVector disjunctions; collectDisjunctions(disjunctions); if (disjunctions.empty()) - return None; + return llvm::None; if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return std::make_pair(disjunction, llvm::TinyPtrVector()); @@ -678,5 +677,5 @@ ConstraintSystem::selectDisjunction() { if (bestDisjunction != disjunctions.end()) return std::make_pair(*bestDisjunction, favorings[*bestDisjunction]); - return None; + return llvm::None; } From 2869dff995ca5301ce33f2116ca370749dca77f6 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 11 Oct 2023 00:19:30 -0700 Subject: [PATCH 17/60] [CSOptimizer] Increase score when type matches opaque type Since erasure of a concrete type into an existential value yields score of 1, we need to bump the score in cases where a type passed to a generic parameter satisfies all of its protocol conformance requirements and that parameter represents an opaque type. --- lib/Sema/CSOptimizer.cpp | 8 +++++- test/Constraints/operator.swift | 46 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index baa45eb7ef81b..5c7ac68bbdd66 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -314,8 +314,14 @@ static void determineBestChoicesInContext( if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { return bool(cs.lookupConformance(candidateType, protocol)); - })) + })) { + if (auto *GP = paramType->getAs()) { + auto *paramDecl = GP->getDecl(); + if (paramDecl && paramDecl->isOpaqueType()) + return 1.0; + } return 0.7; + } } // Parameter is generic, let's check whether top-level diff --git a/test/Constraints/operator.swift b/test/Constraints/operator.swift index 4770481c95e30..77d07f5dd3078 100644 --- a/test/Constraints/operator.swift +++ b/test/Constraints/operator.swift @@ -329,3 +329,49 @@ enum I60954 { } init?(_ string: S) where S: StringProtocol {} // expected-note{{where 'S' = 'I60954'}} } + +infix operator <<<>>> : DefaultPrecedence + +protocol P5 { +} + +struct Expr : P6 {} + +protocol P6: P5 { +} + +extension P6 { + public static func <<<>>> (lhs: Self, rhs: (any P5)?) -> Expr { Expr() } + public static func <<<>>> (lhs: (any P5)?, rhs: Self) -> Expr { Expr() } + public static func <<<>>> (lhs: Self, rhs: some P6) -> Expr { Expr() } + + public static prefix func ! (value: Self) -> Expr { + Expr() + } +} + +extension P6 { + public static func != (lhs: Self, rhs: some P6) -> Expr { + !(lhs <<<>>> rhs) // Ok + } +} + +do { + struct Value : P6 { + } + + struct Column: P6 { + } + + func test(col: Column, val: Value) -> Expr { + col <<<>>> val // Ok + } + + func test(col: Column, val: some P6) -> Expr { + col <<<>>> val // Ok + } + + func test(col: some P6, val: Value) -> Expr { + col <<<>>> val // Ok + } +} From c429f5b9ec30adbbfd0eef1edafe1e09503d071d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 5 Aug 2024 16:53:19 -0700 Subject: [PATCH 18/60] [CSOptimizer] NFC: Switch from llvm::Optional to std::optional post-rebase --- include/swift/Sema/ConstraintSystem.h | 2 +- lib/Sema/CSOptimizer.cpp | 21 ++++++++++----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 12cd5091104f6..aa7fd79dbb0c5 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5214,7 +5214,7 @@ class ConstraintSystem { /// Pick a disjunction from the InactiveConstraints list. /// /// \returns The selected disjunction and a set of it's favored choices. - llvm::Optional>> + std::optional>> selectDisjunction(); /// Pick a conjunction from the InactiveConstraints list. diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 5c7ac68bbdd66..d2c36811430b3 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -171,7 +171,7 @@ static void determineBestChoicesInContext( // Match arguments to the given overload choice. auto matchArguments = [&](OverloadChoice choice, FunctionType *overloadType) - -> llvm::Optional { + -> std::optional { auto *decl = choice.getDeclOrNull(); assert(decl); @@ -186,7 +186,7 @@ static void determineBestChoicesInContext( return matchCallArguments(argsWithLabels, overloadType->getParams(), paramListInfo, argumentList->getFirstTrailingClosureIndex(), - /*allow fixes*/ false, listener, llvm::None); + /*allow fixes*/ false, listener, std::nullopt); }; // Determine whether the candidate type is a subclass of the superclass @@ -209,15 +209,14 @@ static void determineBestChoicesInContext( return false; return llvm::all_of(layout.getProtocols(), [&](ProtocolDecl *P) { - if (auto superclass = P->getSuperclass()) { - if (!isSubclassOf(candidateType, superclass)) + if (auto superclass = P->getSuperclassDecl()) { + if (!isSubclassOf(candidateType, + superclass->getDeclaredInterfaceType())) return false; } - return bool(TypeChecker::containsProtocol( - candidateType, P, cs.DC->getParentModule(), - /*skipConditionalRequirements=*/true, - /*allowMissing=*/false)); + return bool(TypeChecker::containsProtocol(candidateType, P, + /*allowMissing=*/false)); }); } @@ -639,13 +638,13 @@ selectBestBindingDisjunction(ConstraintSystem &cs, return firstBindDisjunction; } -llvm::Optional>> +std::optional>> ConstraintSystem::selectDisjunction() { SmallVector disjunctions; collectDisjunctions(disjunctions); if (disjunctions.empty()) - return llvm::None; + return std::nullopt; if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return std::make_pair(disjunction, llvm::TinyPtrVector()); @@ -683,5 +682,5 @@ ConstraintSystem::selectDisjunction() { if (bestDisjunction != disjunctions.end()) return std::make_pair(*bestDisjunction, favorings[*bestDisjunction]); - return llvm::None; + return std::nullopt; } From 1760bd1f1ec002885195bd154ce9d0a26c9d7879 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 6 Aug 2024 23:03:24 -0700 Subject: [PATCH 19/60] [CSOptimizer] Remove an outdated optimization to compare resolved argument types with all else equal This is already accounted for by `determineBestChoicesInContext` and reflected in the overall score, which means that we no longer need to use this in vacuum. --- include/swift/Sema/Constraint.h | 5 ----- lib/Sema/CSOptimizer.cpp | 7 ------- lib/Sema/Constraint.cpp | 11 ----------- 3 files changed, 23 deletions(-) diff --git a/include/swift/Sema/Constraint.h b/include/swift/Sema/Constraint.h index c60ae2a59fb89..89acbceeb255c 100644 --- a/include/swift/Sema/Constraint.h +++ b/include/swift/Sema/Constraint.h @@ -825,11 +825,6 @@ class Constraint final : public llvm::ilist_node, }); } - /// Returns the number of resolved argument types for an applied disjunction - /// constraint. This is always zero for disjunctions that do not represent - /// an applied overload. - unsigned countResolvedArgumentTypes(ConstraintSystem &cs) const; - /// Determine if this constraint represents explicit conversion, /// e.g. coercion constraint "as X" which forms a disjunction. bool isExplicitConversion() const; diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index d2c36811430b3..272abea810a93 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -662,16 +662,9 @@ ConstraintSystem::selectDisjunction() { unsigned firstFavored = favorings[first].size(); unsigned secondFavored = favorings[second].size(); - // Everything else equal, choose the disjunction with the greatest - // number of resolved argument types. The number of resolved argument - // types is always zero for disjunctions that don't represent applied - // overloads. if (firstFavored == secondFavored) { if (firstActive != secondActive) return firstActive < secondActive; - - return first->countResolvedArgumentTypes(*this) > - second->countResolvedArgumentTypes(*this); } firstFavored = firstFavored ? firstFavored : firstActive; diff --git a/lib/Sema/Constraint.cpp b/lib/Sema/Constraint.cpp index 1b4395b25cfc1..8261e009589ac 100644 --- a/lib/Sema/Constraint.cpp +++ b/lib/Sema/Constraint.cpp @@ -711,17 +711,6 @@ gatherReferencedTypeVars(Constraint *constraint, } } -unsigned Constraint::countResolvedArgumentTypes(ConstraintSystem &cs) const { - auto *argumentFuncType = cs.getAppliedDisjunctionArgumentFunction(this); - if (!argumentFuncType) - return 0; - - return llvm::count_if(argumentFuncType->getParams(), [&](const AnyFunctionType::Param arg) { - auto argType = cs.getFixedTypeRecursive(arg.getPlainType(), /*wantRValue=*/true); - return !argType->isTypeVariableOrMember(); - }); -} - bool Constraint::isExplicitConversion() const { assert(Kind == ConstraintKind::Disjunction); From d69b6a059494dafc3ce917fa4f34959f2cf05589 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 6 Aug 2024 23:50:04 -0700 Subject: [PATCH 20/60] [CSOptimizer] Prefer homogeneous arithmetic operator overloads when argument(s) or result match This is an opportunistic optimization based on the operator use patterns where homogeneous operators are the most heavily used ones. --- lib/Sema/CSOptimizer.cpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 272abea810a93..a8663d5b2ea2f 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -493,6 +493,24 @@ static void determineBestChoicesInContext( } if (score > 0) { + if (decl->isOperator() && + decl->getBaseIdentifier().isArithmeticOperator() && + overloadType->getNumParams() == 2) { + // Nudge the score slightly to prefer concrete homogeneous + // operators. + // + // This is an opportunistic optimization based on the operator + // use patterns where homogeneous operators are the most + // heavily used ones. + auto resultTy = overloadType->getResult(); + if (!resultTy->hasTypeParameter() && + llvm::all_of(overloadType->getParams(), + [&resultTy](const auto ¶m) { + return param.getPlainType()->isEqual(resultTy); + })) + score += 0.001; + } + favoredChoices.push_back({choice, score}); bestScore = std::max(bestScore, score); } From 670127abd65fdbbee985d46051701761d35e2983 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 23 Aug 2024 17:53:38 -0700 Subject: [PATCH 21/60] [Tests] NFC: Add a test-case for rdar://133340307 which is now fast --- .../fast/rdar133340307.swift | 134 ++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 validation-test/Sema/type_checker_perf/fast/rdar133340307.swift diff --git a/validation-test/Sema/type_checker_perf/fast/rdar133340307.swift b/validation-test/Sema/type_checker_perf/fast/rdar133340307.swift 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 + } +} From 527de22bec2b28308f8f9d292d098f923bb15c08 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 5 Sep 2024 09:59:35 -0700 Subject: [PATCH 22/60] [CSOptimizer] Emulate old behavior related to favoring of unary calls to members Preserves old behavior where for unary calls to members the solver would not consider choices that didn't match on the number of parameters (regardless of defaults) and only exact matches were favored. --- lib/Sema/CSOptimizer.cpp | 33 +++++++++++++-- .../old_hack_related_ambiguities.swift | 41 +++++++++++++++++++ 2 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 test/Constraints/old_hack_related_ambiguities.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index a8663d5b2ea2f..8cfb5618a7816 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -91,6 +91,11 @@ void forEachDisjunctionChoice( } } +static bool isOverloadedDeclRef(Constraint *disjunction) { + assert(disjunction->getKind() == ConstraintKind::Disjunction); + return disjunction->getLocator()->directlyAt(); +} + } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -232,6 +237,7 @@ static void determineBestChoicesInContext( enum class MatchFlag { OnParam = 0x01, Literal = 0x02, + ExactOnly = 0x04, }; using MatchOptions = OptionSet; @@ -250,6 +256,9 @@ static void determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> double { + if (options.contains(MatchFlag::ExactOnly)) + return candidateType->isEqual(paramType) ? 1 : 0; + // Exact match between candidate and parameter types. if (candidateType->isEqual(paramType)) return options.contains(MatchFlag::Literal) ? 0.3 : 1; @@ -267,17 +276,17 @@ static void determineBestChoicesInContext( paramType = paramType->lookThroughAllOptionalTypes(paramOptionals); if (!candidateOptionals.empty() || !paramOptionals.empty()) { - if (paramOptionals.size() >= candidateOptionals.size()) + if (paramOptionals.size() >= candidateOptionals.size()) { return scoreCandidateMatch(genericSig, candidateType, paramType, options); + } // Optionality mismatch. return 0; } } - // Candidate could be injected into optional parameter type - // or converted to a superclass. + // Candidate could be converted to a superclass. if (isSubclassOf(candidateType, paramType)) return 1; @@ -343,6 +352,8 @@ static void determineBestChoicesInContext( double bestScore = 0.0; SmallVector, 2> favoredChoices; + bool isOverloadedDeclRefDisjunction = isOverloadedDeclRef(disjunction); + forEachDisjunctionChoice( cs, disjunction, [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { @@ -367,6 +378,20 @@ static void determineBestChoicesInContext( if (!matchings) return; + bool favorExactMatchesOnly = false; + // Preserves old behavior where for unary calls to members + // the solver would not consider choices that didn't match on + // the number of parameters (regardless of defaults) and only + // exact matches were favored. + if (!isOverloadedDeclRefDisjunction && argumentList->size() == 1) { + // Old behavior completely disregarded the fact that some of + // the parameters could be defaulted. + if (overloadType->getNumParams() != 1) + return; + + favorExactMatchesOnly = true; + } + double score = 0.0; for (unsigned paramIdx = 0, n = overloadType->getNumParams(); paramIdx != n; ++paramIdx) { @@ -444,6 +469,8 @@ static void determineBestChoicesInContext( MatchOptions options(MatchFlag::OnParam); if (isLiteralDefault) options |= MatchFlag::Literal; + if (favorExactMatchesOnly) + options |= MatchFlag::ExactOnly; auto score = scoreCandidateMatch(genericSig, candidateType, paramType, options); diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift new file mode 100644 index 0000000000000..30743800d8642 --- /dev/null +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -0,0 +1,41 @@ +// RUN: %target-typecheck-verify-swift + +func entity(_: Int) -> Int { + 0 +} + +struct Test { + func test(_ v: Int) -> Int { v } + func test(_ v: Int?) -> Int? { v } +} + +func test_ternary_literal(v: Test) -> Int? { + true ? v.test(0) : nil // Ok +} + +func test_ternary(v: Test) -> Int? { + true ? v.test(entity(0)) : nil // Ok +} + +do { + struct TestFloat { + func test(_ v: Float) -> Float { v } // expected-note {{found this candidate}} + func test(_ v: Float?) -> Float? { v } // expected-note {{found this candidate}} + } + + func test_ternary_non_default_literal(v: TestFloat) -> Float? { + true ? v.test(1.0) : nil // expected-error {{ambiguous use of 'test'}} + } +} + +do { + struct Test { + init(a: Int, b: Int = 0) throws {} + init?(a: Int?) {} + } + + func test(v: Int) -> Test? { + return Test(a: v) // Ok + } +} + From cf05405eae56384f6021eaec6facfcc938b1edc0 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 6 Sep 2024 17:46:18 -0700 Subject: [PATCH 23/60] [CSOptimizer] Let `determineBestChoicesInContext` return the best disjunction if one is available If there is one overall best disjunction, let's attempt it first. --- lib/Sema/CSOptimizer.cpp | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 8cfb5618a7816..e5fc1be62785c 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -100,7 +100,7 @@ static bool isOverloadedDeclRef(Constraint *disjunction) { /// Given a set of disjunctions, attempt to determine /// favored choices in the current context. -static void determineBestChoicesInContext( +static Constraint *determineBestChoicesInContext( ConstraintSystem &cs, SmallVectorImpl &disjunctions, llvm::DenseMap> &favorings) { @@ -122,7 +122,7 @@ static void determineBestChoicesInContext( auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); if (!argumentList || cs.containsIDEInspectionTarget(argumentList)) - return; + return nullptr; SmallVector argsWithLabels; { @@ -623,6 +623,22 @@ static void determineBestChoicesInContext( for (auto *choice : favoredChoicesPerDisjunction[entry.first]) favorings[entry.first].push_back(choice); } + + if (bestOverallScore == 0) + return nullptr; + + Constraint *bestDisjunction = nullptr; + for (auto *disjunction : disjunctions) { + if (disjunctionScores[disjunction] != bestOverallScore) + continue; + + if (!bestDisjunction) + bestDisjunction = disjunction; + else // Multiple disjunctions with the same score. + return nullptr; + } + + return bestDisjunction; } // Attempt to find a disjunction of bind constraints where all options @@ -695,7 +711,9 @@ ConstraintSystem::selectDisjunction() { return std::make_pair(disjunction, llvm::TinyPtrVector()); llvm::DenseMap> favorings; - determineBestChoicesInContext(*this, disjunctions, favorings); + if (auto *bestDisjunction = + determineBestChoicesInContext(*this, disjunctions, favorings)) + return std::make_pair(bestDisjunction, favorings[bestDisjunction]); // Pick the disjunction with the smallest number of favored, then active // choices. From 3819ddfb4010ac0eaa9c8ba68e8f4d82d3ed9344 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 7 Sep 2024 16:49:04 -0700 Subject: [PATCH 24/60] [CSOptimizer] Record best scores for each disjunction and use them in `selectDisjunction` If there is no single best disjunction use best scores first to determine minimum with fallback to favored and/or active choice counts. --- lib/Sema/CSOptimizer.cpp | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index e5fc1be62785c..c93a84a3e80f7 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -102,7 +102,8 @@ static bool isOverloadedDeclRef(Constraint *disjunction) { /// favored choices in the current context. static Constraint *determineBestChoicesInContext( ConstraintSystem &cs, SmallVectorImpl &disjunctions, - llvm::DenseMap> + llvm::DenseMap>> &favorings) { double bestOverallScore = 0.0; // Tops scores across all of the disjunctions. @@ -617,11 +618,10 @@ static Constraint *determineBestChoicesInContext( } for (auto &entry : disjunctionScores) { - if (entry.second != bestOverallScore) - continue; - + TinyPtrVector favoredChoices; for (auto *choice : favoredChoicesPerDisjunction[entry.first]) - favorings[entry.first].push_back(choice); + favoredChoices.push_back(choice); + favorings[entry.first] = std::make_pair(entry.second, favoredChoices); } if (bestOverallScore == 0) @@ -710,10 +710,12 @@ ConstraintSystem::selectDisjunction() { if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return std::make_pair(disjunction, llvm::TinyPtrVector()); - llvm::DenseMap> favorings; + llvm::DenseMap>> + favorings; if (auto *bestDisjunction = determineBestChoicesInContext(*this, disjunctions, favorings)) - return std::make_pair(bestDisjunction, favorings[bestDisjunction]); + return std::make_pair(bestDisjunction, favorings[bestDisjunction].second); // Pick the disjunction with the smallest number of favored, then active // choices. @@ -722,21 +724,29 @@ ConstraintSystem::selectDisjunction() { [&](Constraint *first, Constraint *second) -> bool { unsigned firstActive = first->countActiveNestedConstraints(); unsigned secondActive = second->countActiveNestedConstraints(); - unsigned firstFavored = favorings[first].size(); - unsigned secondFavored = favorings[second].size(); - if (firstFavored == secondFavored) { + auto &[firstScore, firstFavoredChoices] = favorings[first]; + auto &[secondScore, secondFavoredChoices] = favorings[second]; + + if (firstScore > secondScore) + return true; + + unsigned numFirstFavored = firstFavoredChoices.size(); + unsigned numSecondFavored = secondFavoredChoices.size(); + + if (numFirstFavored == numSecondFavored) { if (firstActive != secondActive) return firstActive < secondActive; } - firstFavored = firstFavored ? firstFavored : firstActive; - secondFavored = secondFavored ? secondFavored : secondActive; - return firstFavored < secondFavored; + numFirstFavored = numFirstFavored ? numFirstFavored : firstActive; + numSecondFavored = numSecondFavored ? numSecondFavored : secondActive; + + return numFirstFavored < numSecondFavored; }); if (bestDisjunction != disjunctions.end()) - return std::make_pair(*bestDisjunction, favorings[*bestDisjunction]); + return std::make_pair(*bestDisjunction, favorings[*bestDisjunction].second); return std::nullopt; } From deca9b61c549dabcef8cf20dfcaa4ea9f33ae5cb Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 9 Sep 2024 21:07:45 -0700 Subject: [PATCH 25/60] [CSOptimizer] attempt to rank only standard/simd operators and fully concrete overload sets --- lib/Sema/CSOptimizer.cpp | 54 ++++++++++++++++++++++++++++++++++------ 1 file changed, 46 insertions(+), 8 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c93a84a3e80f7..299467756f307 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -34,6 +34,48 @@ using namespace constraints; namespace { +static bool isSupportedOperator(Constraint *disjunction) { + if (!isOperatorDisjunction(disjunction)) + return false; + + auto choices = disjunction->getNestedConstraints(); + auto *decl = getOverloadChoiceDecl(choices.front()); + + auto name = decl->getBaseIdentifier(); + if (name.isArithmeticOperator() || name.isStandardComparisonOperator() || + name.is("^")) { + return true; + } + + // Operators like &<<, &>>, &+, .== etc. + if (llvm::any_of(choices, [](Constraint *choice) { + return isSIMDOperator(getOverloadChoiceDecl(choice)); + })) { + return true; + } + + return false; +} + +static bool isSupportedDisjunction(Constraint *disjunction) { + auto choices = disjunction->getNestedConstraints(); + + if (isSupportedOperator(disjunction)) + return true; + + // Non-operator disjunctions are supported only if they don't + // have any generic choices. + return llvm::all_of(choices, [&](Constraint *choice) { + if (choice->getKind() != ConstraintKind::BindOverload) + return false; + + if (auto *decl = getOverloadChoiceDecl(choice)) + return decl->getInterfaceType()->is(); + + return false; + }); +} + NullablePtr getApplicableFnConstraint(ConstraintGraph &CG, Constraint *disjunction) { auto *boundVar = disjunction->getNestedConstraints()[0] @@ -112,6 +154,9 @@ static Constraint *determineBestChoicesInContext( favoredChoicesPerDisjunction; for (auto *disjunction : disjunctions) { + if (!isSupportedDisjunction(disjunction)) + continue; + auto applicableFn = getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); @@ -337,7 +382,7 @@ static Constraint *determineBestChoicesInContext( // types match i.e. Array as a parameter. // // This is slightly better than all of the conformances matching - // because the parameter is concrete and could split the graph. + // because the parameter is concrete and could split the graph. if (paramType->hasTypeParameter()) { auto *candidateDecl = candidateType->getAnyNominal(); auto *paramDecl = paramType->getAnyNominal(); @@ -365,13 +410,6 @@ static Constraint *determineBestChoicesInContext( } else if (auto *SD = dyn_cast(decl)) { genericSig = SD->getGenericSignature(); } - - // Let's not consider non-operator generic overloads because we - // need conformance checking functionality to determine best - // favoring, preferring such overloads based on concrete types - // alone leads to subpar choices due to missed information. - if (genericSig && !decl->isOperator()) - return; } auto matchings = From 8a918e23692c5a0dd4dde3da20e77f8c9165541b Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 11 Sep 2024 10:17:20 -0700 Subject: [PATCH 26/60] [CSOptimizer] Don't optimize (implicit) calls with code completion arguments `containsIDEInspectionTarget` doesn't work for implicit argument lists, but luckily implicit code completion expressions cannot be injected into arguments, so we can check whether whether they appear at an argument position and prevent optimization. --- lib/Sema/CSOptimizer.cpp | 11 ++++++++++- test/IDE/complete_operators.swift | 2 +- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 299467756f307..4efda9f6460c9 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -167,9 +167,18 @@ static Constraint *determineBestChoicesInContext( applicableFn.get()->getFirstType()->getAs(); auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); - if (!argumentList || cs.containsIDEInspectionTarget(argumentList)) + if (!argumentList) return nullptr; + for (const auto &argument : *argumentList) { + if (auto *expr = argument.getExpr()) { + // Directly `<#...#>` or has one inside. + if (isa(expr) || + cs.containsIDEInspectionTarget(expr)) + return nullptr; + } + } + SmallVector argsWithLabels; { argsWithLabels.append(argFuncType->getParams().begin(), diff --git a/test/IDE/complete_operators.swift b/test/IDE/complete_operators.swift index a349fd4ecf8da..483d3cc6225d8 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^# From 66981364fedcefde9018a83c3ae5b4913d930e85 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 11 Sep 2024 13:41:00 -0700 Subject: [PATCH 27/60] [Tests] NFC: Adjust a couple of improved tests --- validation-test/stdlib/FixedPointDiagnostics.swift.gyb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/validation-test/stdlib/FixedPointDiagnostics.swift.gyb b/validation-test/stdlib/FixedPointDiagnostics.swift.gyb index e091805af22b7..2b2df2812c0d2 100644 --- a/validation-test/stdlib/FixedPointDiagnostics.swift.gyb +++ b/validation-test/stdlib/FixedPointDiagnostics.swift.gyb @@ -53,8 +53,8 @@ func testMixedSignArithmetic() { Stride(1) - ${T}(0) // expected-error {{}} expected-note {{}} var y: Stride = 0 - y += ${T}(1) // expected-error {{cannot convert value of type '${T}' to expected argument type 'Int'}} {{10-10=Int(}} - y -= ${T}(1) // expected-error {{cannot convert value of type '${T}' to expected argument type 'Int'}} {{10-10=Int(}} + y += ${T}(1) // expected-error {{cannot convert value of type '${T}' to expected argument type 'Stride' (aka 'Int')}} {{10-10=Stride(}} + y -= ${T}(1) // expected-error {{cannot convert value of type '${T}' to expected argument type 'Stride' (aka 'Int')}} {{10-10=Stride(}} } % end } From 23589add740de550c0de8a9cd9c887d66ad1ca72 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 11 Sep 2024 14:57:44 -0700 Subject: [PATCH 28/60] [CSOptimizer] Average score should reflect number of defaulted parameters --- lib/Sema/CSOptimizer.cpp | 11 ++++++++-- .../old_hack_related_ambiguities.swift | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 4efda9f6460c9..ff46bed30011f 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -441,6 +441,7 @@ static Constraint *determineBestChoicesInContext( } double score = 0.0; + unsigned numDefaulted = 0; for (unsigned paramIdx = 0, n = overloadType->getNumParams(); paramIdx != n; ++paramIdx) { const auto ¶m = overloadType->getParams()[paramIdx]; @@ -448,7 +449,8 @@ static Constraint *determineBestChoicesInContext( auto argIndices = matchings->parameterBindings[paramIdx]; switch (argIndices.size()) { case 0: - // Current parameter is defaulted. + // Current parameter is defaulted, mark and continue. + ++numDefaulted; continue; case 1: @@ -542,9 +544,14 @@ static Constraint *determineBestChoicesInContext( score += bestCandidateScore; } + // An overload whether all of the parameters are defaulted + // that's called without arguments. + if (numDefaulted == overloadType->getNumParams()) + return; + // Average the score to avoid disfavoring disjunctions with fewer // parameters. - score /= overloadType->getNumParams(); + score /= (overloadType->getNumParams() - numDefaulted); // If one of the result types matches exactly, that's a good // indication that overload choice should be favored. diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index 30743800d8642..d2db5a2e36632 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -39,3 +39,25 @@ do { } } +// error: initializer for conditional binding must have Optional type, not 'S' +do { + struct S { + let n: Int + } + + func f(_: String, _ p: Bool = false) -> S? { + nil + } + + func f(_ x: String) -> S { + fatalError() + } + + func g(_ x: String) -> Int? { + guard let y = f(x) else { + return nil + } + return y.n + } +} + From d0ff6c81b818a76e78463bec2613354498a4af76 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 11 Sep 2024 15:01:13 -0700 Subject: [PATCH 29/60] [Tests] NFC: Add more test-cases that were previously solved due to old hacks behavior --- .../old_hack_related_ambiguities.swift | 117 ++++++++++++++++++ 1 file changed, 117 insertions(+) diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index d2db5a2e36632..67feaabab5836 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -61,3 +61,120 @@ do { } } +// ambiguities related to ~= +protocol _Error: Error {} + +extension _Error { + public static func ~=(lhs: Self, rhs: Self) -> Bool { + false + } + + public static func ~=(lhs: Error, rhs: Self) -> Bool { + false + } + + public static func ~=(lhs: Self, rhs: Error) -> Bool { + false + } +} + +enum CustomError { + case A +} + +extension CustomError: _Error {} + +func f(e: CustomError) { + if e ~= CustomError.A {} +} + +// Generic overload should be preferred over concrete one because the latter is non-default literal +struct Pattern {} + +func ~= (pattern: Pattern, value: String) -> Bool { + return false +} + +extension Pattern: ExpressibleByStringLiteral { + init(stringLiteral value: String) {} +} + +func test_default_tilda(v: String) { + _ = "hi" ~= v // Ok +} + +struct UUID {} + +struct LogKey { + init(base: some CustomStringConvertible, foo: Int = 0) { + } + + init(base: UUID, foo: Int = 0) { + } +} + +@available(swift 99) +extension LogKey { + init(base: String?) { + } + + init(base: UUID?) { + } +} + +func test_that_unavailable_init_is_not_used(x: String?) { + _ = LogKey(base: x ?? "??") +} + +// error: value of optional type 'UID?' must be unwrapped to a value of type 'UID' +struct S: Comparable { + static func <(lhs: Self, rhs: Self) -> Bool { + false + } +} + +func max(_ a: S?, _ b: S?) -> S? { + nil +} + +func test_stdlib_max_selection(s: S) -> S { + let new = max(s, s) + return new // Ok +} + +// error: initializer for conditional binding must have Optional type, not 'UnsafeMutablePointer' +do { + struct TestPointerConversions { + var p: UnsafeMutableRawPointer { get { fatalError() } } + + func f(_ p: UnsafeMutableRawPointer) { + guard let x = UnsafeMutablePointer(OpaquePointer(self.p)) else { + return + } + _ = x + + guard let x = UnsafeMutablePointer(OpaquePointer(p)) else { + return + } + _ = x + } + } +} + +// error: initializer 'init(_:)' requires that 'T' conform to 'BinaryInteger' +do { + struct Config { + subscript(_ key: String) -> T? { nil } + subscript(_ key: String) -> Any? { nil } + } + + struct S { + init(maxQueueDepth: UInt) {} + } + + func f(config: Config) { + let maxQueueDepth = config["hi"] ?? 256 + _ = S(maxQueueDepth: UInt(maxQueueDepth)) + } +} + From 3996b25fbdc70566990c33a6f431ca645336cac7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 16 Sep 2024 15:18:49 -0700 Subject: [PATCH 30/60] [CSOptimizer] Rank results of operators regardless of whether anything is known about parameters When operators are chained it's possible that we don't know anything about parameter(s) but result is known from the context, we should use that information. --- lib/Sema/CSOptimizer.cpp | 28 +++++++++++-------- ...or_chain_with_hetergeneous_arguments.swift | 1 - 2 files changed, 17 insertions(+), 12 deletions(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/operator_chain_with_hetergeneous_arguments.swift (90%) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index ff46bed30011f..fd35da211b84b 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -57,6 +57,15 @@ static bool isSupportedOperator(Constraint *disjunction) { return false; } +static bool isStandardComparisonOperator(ValueDecl *decl) { + return decl->isOperator() && + decl->getBaseIdentifier().isStandardComparisonOperator(); +} + +static bool isArithmeticOperator(ValueDecl *decl) { + return decl->isOperator() && decl->getBaseIdentifier().isArithmeticOperator(); +} + static bool isSupportedDisjunction(Constraint *disjunction) { auto choices = disjunction->getNestedConstraints(); @@ -561,9 +570,7 @@ static Constraint *determineBestChoicesInContext( // ones that all have the same result type), regular // functions/methods and especially initializers could end up // with a lot of favored overloads because on the result type alone. - if (score > 0 || - (decl->isOperator() && - !decl->getBaseIdentifier().isStandardComparisonOperator())) { + if (decl->isOperator() && !isStandardComparisonOperator(decl)) { if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) { return scoreCandidateMatch(genericSig, overloadType->getResult(), @@ -575,15 +582,14 @@ static Constraint *determineBestChoicesInContext( } if (score > 0) { - if (decl->isOperator() && - decl->getBaseIdentifier().isArithmeticOperator() && + // 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) { - // Nudge the score slightly to prefer concrete homogeneous - // operators. - // - // This is an opportunistic optimization based on the operator - // use patterns where homogeneous operators are the most - // heavily used ones. auto resultTy = overloadType->getResult(); if (!resultTy->hasTypeParameter() && llvm::all_of(overloadType->getParams(), diff --git a/validation-test/Sema/type_checker_perf/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}} } From 8818d399f9b14dd0fd8bc325f4427246a660c824 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 17 Sep 2024 09:27:48 -0700 Subject: [PATCH 31/60] [CSOptimizer] Rank disjunctions based on score only if both sides are supported --- lib/Sema/CSOptimizer.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index fd35da211b84b..73482062e5ee2 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -677,6 +677,9 @@ static Constraint *determineBestChoicesInContext( getLogger() << ")\n"; } + if (bestOverallScore == 0) + return nullptr; + for (auto &entry : disjunctionScores) { TinyPtrVector favoredChoices; for (auto *choice : favoredChoicesPerDisjunction[entry.first]) @@ -684,9 +687,6 @@ static Constraint *determineBestChoicesInContext( favorings[entry.first] = std::make_pair(entry.second, favoredChoices); } - if (bestOverallScore == 0) - return nullptr; - Constraint *bestDisjunction = nullptr; for (auto *disjunction : disjunctions) { if (disjunctionScores[disjunction] != bestOverallScore) @@ -788,8 +788,13 @@ ConstraintSystem::selectDisjunction() { auto &[firstScore, firstFavoredChoices] = favorings[first]; auto &[secondScore, secondFavoredChoices] = favorings[second]; - if (firstScore > secondScore) - return true; + // Rank based on scores only if both disjunctions are supported. + if (isSupportedDisjunction(first) && isSupportedDisjunction(second)) { + // If both disjunctions have the same score they should be ranked + // based on number of favored/active choices. + if (firstScore != secondScore) + return firstScore > secondScore; + } unsigned numFirstFavored = firstFavoredChoices.size(); unsigned numSecondFavored = secondFavoredChoices.size(); From 6fb6d1cf90c94865df46fc1557990e0640a1164e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 17 Sep 2024 13:31:21 -0700 Subject: [PATCH 32/60] [CSOptimizer] Enable ranking of `Int*`, `Float{80}` and `Double` initializers --- lib/Sema/CSOptimizer.cpp | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 73482062e5ee2..c96c6fee5fd43 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -34,6 +34,20 @@ using namespace constraints; namespace { +// TODO: both `isIntegerType` and `isFloatType` should be available on Type +// as `isStdlib{Integer, Float}Type`. + +static bool isIntegerType(Type type) { + return type->isInt() || type->isInt8() || type->isInt16() || + type->isInt32() || type->isInt64() || type->isUInt() || + type->isUInt8() || type->isUInt16() || type->isUInt32() || + type->isUInt64(); +} + +static bool isFloatType(Type type) { + return type->isFloat() || type->isDouble() || type->isFloat80(); +} + static bool isSupportedOperator(Constraint *disjunction) { if (!isOperatorDisjunction(disjunction)) return false; @@ -57,6 +71,16 @@ static bool isSupportedOperator(Constraint *disjunction) { return false; } +static bool isSupportedSpecialConstructor(ConstructorDecl *ctor) { + if (auto *selfDecl = ctor->getImplicitSelfDecl()) { + auto selfTy = selfDecl->getInterfaceType(); + /// Support `Int*`, `Float*` and `Double` initializers since their generic + /// overloads are not too complicated. + return selfTy && (isIntegerType(selfTy) || isFloatType(selfTy)); + } + return false; +} + static bool isStandardComparisonOperator(ValueDecl *decl) { return decl->isOperator() && decl->getBaseIdentifier().isStandardComparisonOperator(); @@ -72,6 +96,12 @@ static bool isSupportedDisjunction(Constraint *disjunction) { if (isSupportedOperator(disjunction)) return true; + if (auto *ctor = dyn_cast_or_null( + getOverloadChoiceDecl(choices.front()))) { + if (isSupportedSpecialConstructor(ctor)) + return true; + } + // Non-operator disjunctions are supported only if they don't // have any generic choices. return llvm::all_of(choices, [&](Constraint *choice) { From 59109c2d602e7ea48d0c41cb9918d961dba16211 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 18 Sep 2024 13:03:52 -0700 Subject: [PATCH 33/60] [CSOptimizer] Score all of the overload choices matching on literals uniformly Regardless of whether an overload choice matched on one or more literal candidate types assign it a fixed score to make sure that we always prefer outmost disjunction in cases like `test(1 + 2 + 3)` since it gives us a better chance to prune once there is a contextual type. --- lib/Sema/CSOptimizer.cpp | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c96c6fee5fd43..2eb7c56d6e5d1 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -268,6 +268,19 @@ static Constraint *determineBestChoicesInContext( resultTypes.push_back(resultType); } + // Determine whether all of the argument candidates are inferred from literals. + // This information is going to be used later on when we need to decide how to + // score a matching choice. + bool onlyLiteralCandidates = + argFuncType->getNumParams() > 0 && + llvm::none_of( + indices(argFuncType->getParams()), [&](const unsigned argIdx) { + auto &candidates = candidateArgumentTypes[argIdx]; + return llvm::any_of(candidates, [&](const auto &candidate) { + return !candidate.second; + }); + }); + // Match arguments to the given overload choice. auto matchArguments = [&](OverloadChoice choice, FunctionType *overloadType) -> std::optional { @@ -592,6 +605,18 @@ static Constraint *determineBestChoicesInContext( // parameters. score /= (overloadType->getNumParams() - numDefaulted); + // Make sure that the score is uniform for all disjunction + // choices that match on literals only, this would make sure that + // in operator chains that consist purely of literals we'd + // always prefer outermost disjunction instead of innermost + // one. + // + // Preferring outer disjunction first works better in situations + // when contextual type for the whole chain becomes available at + // some point during solving at it would allow for faster pruning. + if (score > 0 && onlyLiteralCandidates) + score = 0.1; + // If one of the result types matches exactly, that's a good // indication that overload choice should be favored. // From f2a6677a6d42dbda51d70e54c38a3c101985b4e6 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 19 Sep 2024 17:44:06 -0700 Subject: [PATCH 34/60] [CSOptimizer] Infer argument candidates from calls to `Double` and CGFloat constructors 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. Helps situations like `1 + CGFloat(...)` by inferring a type for the second operand of `+` based on a type being constructed. Currently limited to known integer, floating-point and CGFloat types since we know how they are structured. --- include/swift/Sema/ConstraintSystem.h | 4 ++++ lib/Sema/CSOptimizer.cpp | 17 +++++++++++++++++ lib/Sema/TypeCheckConstraints.cpp | 4 ++++ .../implicit_double_cgfloat_conversion.swift | 19 +++++++++++++++++++ 4 files changed, 44 insertions(+) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index aa7fd79dbb0c5..7d0dbc28ef025 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -497,6 +497,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. /// diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 2eb7c56d6e5d1..62c712ebb7bd5 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -250,6 +250,23 @@ static Constraint *determineBestChoicesInContext( {literal.second.getDefaultType(), /*fromLiteral=*/true}); } } + + // Helps situations like `1 + {Double, CGFloat}(...)` by inferring + // a type for the second operand of `+` based on a type being constructed. + // + // Currently limited to Double and CGFloat only since we need to + // support implicit `Double<->CGFloat` conversion. + if (typeVar->getImpl().isFunctionResult() && + isOperatorDisjunction(disjunction)) { + auto resultLoc = typeVar->getImpl().getLocator(); + if (auto *call = getAsExpr(resultLoc->getAnchor())) { + if (auto *typeExpr = dyn_cast(call->getFn())) { + auto instanceTy = cs.getType(typeExpr)->getMetatypeInstanceType(); + if (instanceTy->isDouble() || instanceTy->isCGFloat()) + types.push_back({instanceTy, /*fromLiteral=*/false}); + } + } + } } else { types.push_back({argType, /*fromLiteral=*/false}); } diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index 59bea09049105..bf88e66bafc5c 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); diff --git a/test/Constraints/implicit_double_cgfloat_conversion.swift b/test/Constraints/implicit_double_cgfloat_conversion.swift index 32126db0864d0..22011f0019214 100644 --- a/test/Constraints/implicit_double_cgfloat_conversion.swift +++ b/test/Constraints/implicit_double_cgfloat_conversion.swift @@ -342,3 +342,22 @@ func test_init_validation() { } } } + +do { + struct G { + init(_: T) {} + } + + func round(_: Double) -> Double {} + func round(_: T) -> T {} + + func test_cgfloat_over_double(withColors colors: Int, size: CGSize) -> G { + let g = G(1.0 / CGFloat(colors)) + return g // Ok + } + + func test_no_ambiguity(width: Int, height: Int) -> CGFloat { + let v = round(CGFloat(width / height) * 10) / 10.0 + return v // Ok + } +} From 8d5cb112efdc86d736f8cc7fda9b3e43a23d2fdb Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Sat, 21 Sep 2024 20:50:27 -0700 Subject: [PATCH 35/60] [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. --- include/swift/Sema/ConstraintSystem.h | 4 ++++ lib/Sema/CSSolver.cpp | 21 +++++++++++++++++++ lib/Sema/TypeCheckConstraints.cpp | 4 ++++ .../old_hack_related_ambiguities.swift | 9 ++++++++ 4 files changed, 38 insertions(+) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 7d0dbc28ef025..720bb16e6b828 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -497,6 +497,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 aeabc988e9cd4..ae85683d84125 100644 --- a/lib/Sema/CSSolver.cpp +++ b/lib/Sema/CSSolver.cpp @@ -1402,6 +1402,27 @@ tryOptimizeGenericDisjunction(ConstraintSystem &cs, Constraint *disjunction, return nullptr; } + // Don't attempt this optimization if call has number literals. + // This is intended to narrowly fix situations like: + // + // func test(_: T) { ... } + // func test(_: T) { ... } + // + // test(42) + // + // The call should use `` overload even though the + // `` is a more specialized version because + // selecting `` doesn't introduce non-default literal + // types. + if (auto *argFnType = cs.getAppliedDisjunctionArgumentFunction(disjunction)) { + if (llvm::any_of( + argFnType->getParams(), [](const AnyFunctionType::Param ¶m) { + auto *typeVar = param.getPlainType()->getAs(); + return typeVar && typeVar->getImpl().isNumberLiteralType(); + })) + return nullptr; + } + llvm::SmallVector choices; for (auto *choice : constraints) { if (choices.size() > 2) diff --git a/lib/Sema/TypeCheckConstraints.cpp b/lib/Sema/TypeCheckConstraints.cpp index bf88e66bafc5c..0f45f3b37bfaa 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 67feaabab5836..f30bfddfaf313 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -178,3 +178,12 @@ do { } } +// `tryOptimizeGenericDisjunction` is too aggressive sometimes, make sure that `` +// overload is _not_ selected in this case. +do { + func test(_ expression1: @autoclosure () throws -> T, accuracy: T) -> T {} + func test(_ expression1: @autoclosure () throws -> T, accuracy: T) -> T {} + + let result = test(10, accuracy: 1) + let _: Int = result +} From 802f5cd105f3d9ab3132056ecd48a458dfbe7547 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 23 Sep 2024 11:53:00 -0700 Subject: [PATCH 36/60] [CSOptimizer] Desugar types before checking for equality --- lib/Sema/CSOptimizer.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 62c712ebb7bd5..c86df5e535bcf 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -380,11 +380,15 @@ static Constraint *determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> double { + auto areEqual = [](Type a, Type b) { + return a->getDesugaredType()->isEqual(b->getDesugaredType()); + }; + if (options.contains(MatchFlag::ExactOnly)) - return candidateType->isEqual(paramType) ? 1 : 0; + return areEqual(candidateType, paramType) ? 1 : 0; // Exact match between candidate and parameter types. - if (candidateType->isEqual(paramType)) + if (areEqual(candidateType, paramType)) return options.contains(MatchFlag::Literal) ? 0.3 : 1; if (options.contains(MatchFlag::Literal)) From a3a3ec4fe054c07e3d004b62fb4d0733a8bbd97d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 23 Sep 2024 17:52:04 -0700 Subject: [PATCH 37/60] [CSOptimizer] Restore old hack behavior which used to favor overloads based on arity matches 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. This is important to maintain source compatibility. --- lib/Sema/CSOptimizer.cpp | 84 +++++++++++++++++-- .../old_hack_related_ambiguities.swift | 50 +++++++++++ test/Constraints/ranking.swift | 8 -- 3 files changed, 129 insertions(+), 13 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c86df5e535bcf..c58ec1f58218a 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -172,9 +172,67 @@ void forEachDisjunctionChoice( } } -static bool isOverloadedDeclRef(Constraint *disjunction) { +static OverloadedDeclRefExpr *isOverloadedDeclRef(Constraint *disjunction) { assert(disjunction->getKind() == ConstraintKind::Disjunction); - return disjunction->getLocator()->directlyAt(); + + auto *locator = disjunction->getLocator(); + if (locator->getPath().empty()) + return getAsExpr(locator->getAnchor()); + return nullptr; +} + +/// This maintains an "old hack" behavior where overloads of some +/// `OverloadedDeclRef` calls were favored purely based on number of +/// argument and (non-defaulted) parameters matching. +static void findFavoredChoicesBasedOnArity( + ConstraintSystem &cs, Constraint *disjunction, ArgumentList *argumentList, + llvm::function_ref favoredChoice) { + auto *ODRE = isOverloadedDeclRef(disjunction); + if (!ODRE) + return; + + if (llvm::count_if(ODRE->getDecls(), [&argumentList](auto *choice) { + if (auto *paramList = getParameterList(choice)) + return argumentList->size() == paramList->size(); + return false; + }) > 1) + return; + + auto isVariadicGenericOverload = [&](ValueDecl *choice) { + auto genericContext = choice->getAsGenericContext(); + if (!genericContext) + return false; + + auto *GPL = genericContext->getGenericParams(); + if (!GPL) + return false; + + return llvm::any_of(GPL->getParams(), [&](const GenericTypeParamDecl *GP) { + return GP->isParameterPack(); + }); + }; + + bool hasVariadicGenerics = false; + SmallVector favored; + + forEachDisjunctionChoice( + cs, disjunction, + [&](Constraint *choice, ValueDecl *decl, FunctionType *overloadType) { + if (isVariadicGenericOverload(decl)) + hasVariadicGenerics = true; + + if (overloadType->getNumParams() == argumentList->size() || + llvm::count_if(*getParameterList(decl), [](auto *param) { + return !param->isDefaultArgument(); + }) == argumentList->size()) + favored.push_back(choice); + }); + + if (hasVariadicGenerics) + return; + + for (auto *choice : favored) + favoredChoice(choice); } } // end anonymous namespace @@ -193,9 +251,6 @@ static Constraint *determineBestChoicesInContext( favoredChoicesPerDisjunction; for (auto *disjunction : disjunctions) { - if (!isSupportedDisjunction(disjunction)) - continue; - auto applicableFn = getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); @@ -218,6 +273,25 @@ static Constraint *determineBestChoicesInContext( } } + // This maintains an "old hack" behavior where overloads + // of `OverloadedDeclRef` calls were favored purely + // based on arity of arguments and parameters matching. + { + findFavoredChoicesBasedOnArity( + cs, disjunction, argumentList, [&](Constraint *choice) { + favoredChoicesPerDisjunction[disjunction].push_back(choice); + }); + + if (!favoredChoicesPerDisjunction[disjunction].empty()) { + disjunctionScores[disjunction] = 0.01; + bestOverallScore = std::max(bestOverallScore, 0.01); + continue; + } + } + + if (!isSupportedDisjunction(disjunction)) + continue; + SmallVector argsWithLabels; { argsWithLabels.append(argFuncType->getParams().begin(), diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index f30bfddfaf313..d1dc81b514fab 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -187,3 +187,53 @@ do { let result = test(10, accuracy: 1) let _: Int = result } + +// swift-distributed-tracing snippet that relies on old hack behavior. +protocol TracerInstant { +} + +extension Int: TracerInstant {} + +do { + enum SpanKind { + case `internal` + } + + func withSpan( + _ operationName: String, + at instant: @autoclosure () -> Instant, + context: @autoclosure () -> Int = 0, + ofKind kind: SpanKind = .internal + ) {} + + func withSpan( + _ operationName: String, + context: @autoclosure () -> Int = 0, + ofKind kind: SpanKind = .internal, + at instant: @autoclosure () -> some TracerInstant = 42 + ) {} + + withSpan("", at: 0) // Ok +} + +protocol ForAssert { + var isEmpty: Bool { get } +} + +extension ForAssert { + var isEmpty: Bool { false } +} + +do { + func assert(_ condition: @autoclosure () -> Bool, _ message: @autoclosure () -> String = String(), file: StaticString = #file, line: UInt = #line) {} + func assert(_ condition: Bool, _ message: @autoclosure () -> String, file: StaticString = #file, line: UInt = #line) {} + func assert(_ condition: Bool, file: StaticString = #fileID, line: UInt = #line) {} + + struct S : ForAssert { + var isEmpty: Bool { false } + } + + func test(s: S) { + assert(s.isEmpty, "") // Ok + } +} diff --git a/test/Constraints/ranking.swift b/test/Constraints/ranking.swift index 74e3b6842f02e..82d29791e6f14 100644 --- a/test/Constraints/ranking.swift +++ b/test/Constraints/ranking.swift @@ -450,11 +450,3 @@ struct HasIntInit { func compare_solutions_with_bindings(x: UInt8, y: UInt8) -> HasIntInit { return .init(Int(x / numericCast(y))) } - -// Test to make sure that previous favoring behavior is maintained and @autoclosure makes a difference. -func test_no_ambiguity_with_autoclosure(x: Int) { - func test(_ condition: Bool, file: StaticString = #file, line: UInt = #line) {} - func test(_ condition: @autoclosure () -> Bool, file: StaticString = #file, line: UInt = #line) {} - - test(x >= 0) // Ok -} From e30587bda4c24afa56cd1e91f0d1407c41253a0a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 24 Sep 2024 12:31:31 -0700 Subject: [PATCH 38/60] [CSOptimizer] A more comprehensive generic overload checking when candidates are fully resolved Use `checkGenericRequirements` to check whether all of the requirements placed on a particular parameter match, that gives us a lot of confidence that the overload should be favored. --- lib/Sema/CSOptimizer.cpp | 79 ++++++++++++++++++++++++++++++---------- 1 file changed, 59 insertions(+), 20 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c58ec1f58218a..7bcf91249b94d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -479,8 +479,13 @@ static Constraint *determineBestChoicesInContext( if (!candidateOptionals.empty() || !paramOptionals.empty()) { if (paramOptionals.size() >= candidateOptionals.size()) { - return scoreCandidateMatch(genericSig, candidateType, paramType, - options); + auto score = scoreCandidateMatch(genericSig, candidateType, + paramType, options); + // Injection lowers the score slightly to comply with + // old behavior where exact matches on operator parameter + // types were always preferred. + return score == 1 && isOperatorDisjunction(disjunction) ? 0.9 + : score; } // Optionality mismatch. @@ -512,26 +517,60 @@ static Constraint *determineBestChoicesInContext( // Check protocol requirement(s) if this parameter is a // generic parameter type. - if (genericSig && paramType->isTypeParameter()) { - auto protocolRequirements = genericSig->getRequiredProtocols(paramType); - // It's a generic parameter or dependent member which might - // be connected via ame-type constraints to other generic - // parameters or dependent member but we cannot check that here, - // so let's add a tiny score just to acknowledge that it could - // possibly match. - if (protocolRequirements.empty()) - return 0.01; - - if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { - return bool(cs.lookupConformance(candidateType, protocol)); - })) { - if (auto *GP = paramType->getAs()) { - auto *paramDecl = GP->getDecl(); - if (paramDecl && paramDecl->isOpaqueType()) - return 1.0; + if (genericSig && paramType->is()) { + // If candidate is not fully resolved, check conformances only + // and lower the score. + if (candidateType->hasTypeVariable()) { + auto protocolRequirements = + genericSig->getRequiredProtocols(paramType); + if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { + return bool(cs.lookupConformance(candidateType, protocol)); + })) { + if (auto *GP = paramType->getAs()) { + auto *paramDecl = GP->getDecl(); + if (paramDecl && paramDecl->isOpaqueType()) + return 1.0; + } + return 0.7; } - return 0.7; + + return 0; + } + + // If the candidate type is fully resolved, let's check all of + // the requirements that are associated with the corresponding + // parameter, if all of them are satisfied this candidate is + // an exact match. + + auto isParameterType = [¶mType](Type type) { + return type->isEqual(paramType); + }; + + SmallVector requirements; + for (const auto &requirement : genericSig.getRequirements()) { + if (requirement.getFirstType().findIf(isParameterType) || + (requirement.getKind() != RequirementKind::Layout && + requirement.getSecondType().findIf(isParameterType))) + requirements.push_back(requirement); } + + auto result = checkRequirements( + requirements, + [¶mType, &candidateType](SubstitutableType *type) -> Type { + if (type->isEqual(paramType)) + return candidateType; + return ErrorType::get(type); + }, + SubstOptions(std::nullopt)); + + // Concrete operator overloads are always more preferable to + // generic ones if there are exact or subtype matches, for + // everything else the solver should try both concrete and + // generic and disambiguate during ranking. + if (result == CheckRequirementsResult::Success) + return isOperatorDisjunction(disjunction) ? 0.9 : 1.0; + + return 0; } // Parameter is generic, let's check whether top-level From 6caf1ccbb2671dbd628ef81877e7fd6ad1def4f7 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 7 Oct 2024 19:57:50 +0900 Subject: [PATCH 39/60] [CSOptimizer] Fix Double<->CGFloat implicit conversion support when arguments are literals Passing Double to CGFloat as well as vice versa constitutes an exact match. --- lib/Sema/CSOptimizer.cpp | 16 +++++++++++++--- .../implicit_double_cgfloat_conversion.swift | 5 +++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 7bcf91249b94d..305a91feacd02 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -454,7 +454,15 @@ static Constraint *determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> double { - auto areEqual = [](Type a, Type b) { + auto areEqual = [&options](Type a, Type b) { + // Double<->CGFloat implicit conversion support for literals + // only since in this case the conversion might not result in + // score penalty. + if (options.contains(MatchFlag::Literal) && + ((a->isDouble() && b->isCGFloat()) || + (a->isCGFloat() && b->isDouble()))) + return true; + return a->getDesugaredType()->isEqual(b->getDesugaredType()); }; @@ -462,11 +470,13 @@ static Constraint *determineBestChoicesInContext( return areEqual(candidateType, paramType) ? 1 : 0; // Exact match between candidate and parameter types. - if (areEqual(candidateType, paramType)) + if (areEqual(candidateType, paramType)) { return options.contains(MatchFlag::Literal) ? 0.3 : 1; + } - if (options.contains(MatchFlag::Literal)) + if (options.contains(MatchFlag::Literal)) { return 0; + } // Check whether match would require optional injection. { diff --git a/test/Constraints/implicit_double_cgfloat_conversion.swift b/test/Constraints/implicit_double_cgfloat_conversion.swift index 22011f0019214..83c6955851b58 100644 --- a/test/Constraints/implicit_double_cgfloat_conversion.swift +++ b/test/Constraints/implicit_double_cgfloat_conversion.swift @@ -361,3 +361,8 @@ do { return v // Ok } } + +func test_cgfloat_operator_is_attempted_with_literal_arguments(v: CGFloat?) { + let ratio = v ?? (2.0 / 16.0) + let _: CGFloat = ratio // Ok +} From 9b62c84a4f44b9ecdd9bbb8df642b04b9d2d4ddb Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 8 Oct 2024 21:25:37 +0900 Subject: [PATCH 40/60] [CSOptimizer] Adjust `scoreCandidateMatch` to indicate when match cannot be decided It is not always possible to match candidate types against corresponding parameter types for certain overloads i.e. when parameter is an associated type. `scoreCandidateMatch` needs to be able to indicate that to the caller to allow it to move to the next parameter (if any) without failing the overload choice when the match cannot be established. --- lib/Sema/CSOptimizer.cpp | 41 ++++++++++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 12 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 305a91feacd02..c8cd3bfc9ce2a 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -450,10 +450,16 @@ static Constraint *determineBestChoicesInContext( // - Array-to-pointer conversion // - Value to existential conversion // - Exact match on top-level types - std::function - scoreCandidateMatch = [&](GenericSignature genericSig, - Type candidateType, Type paramType, - MatchOptions options) -> double { + // + // In situations when it's not possible to determine whether a candidate + // type matches a parameter type (i.e. when partially resolved generic + // types are matched) this function is going to produce \c std::nullopt + // instead of `0` that indicates "not a match". + std::function(GenericSignature, Type, Type, + MatchOptions)> + scoreCandidateMatch = + [&](GenericSignature genericSig, Type candidateType, Type paramType, + MatchOptions options) -> std::optional { auto areEqual = [&options](Type a, Type b) { // Double<->CGFloat implicit conversion support for literals // only since in this case the conversion might not result in @@ -527,10 +533,12 @@ static Constraint *determineBestChoicesInContext( // Check protocol requirement(s) if this parameter is a // generic parameter type. - if (genericSig && paramType->is()) { - // If candidate is not fully resolved, check conformances only - // and lower the score. - if (candidateType->hasTypeVariable()) { + if (genericSig && paramType->isTypeParameter()) { + // If candidate is not fully resolved or is matched against a + // dependent member type (i.e. `Self.T`), let's check conformances + // only and lower the score. + if (candidateType->hasTypeVariable() || + paramType->is()) { auto protocolRequirements = genericSig->getRequiredProtocols(paramType); if (llvm::all_of(protocolRequirements, [&](ProtocolDecl *protocol) { @@ -547,6 +555,10 @@ static Constraint *determineBestChoicesInContext( return 0; } + // Cannot match anything but generic type parameters here. + if (!paramType->is()) + return std::nullopt; + // If the candidate type is fully resolved, let's check all of // the requirements that are associated with the corresponding // parameter, if all of them are satisfied this candidate is @@ -718,10 +730,15 @@ static Constraint *determineBestChoicesInContext( if (favorExactMatchesOnly) options |= MatchFlag::ExactOnly; - auto score = scoreCandidateMatch(genericSig, candidateType, - paramType, options); - if (score > 0) { - bestCandidateScore = std::max(bestCandidateScore, score); + auto candidateScore = scoreCandidateMatch( + genericSig, candidateType, paramType, options); + + if (!candidateScore) + continue; + + if (candidateScore > 0) { + bestCandidateScore = + std::max(bestCandidateScore, candidateScore.value()); continue; } From 2fdd4b6c35ae5c7763307cd16b335030ae537d84 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 8 Oct 2024 23:49:42 +0900 Subject: [PATCH 41/60] [CSOptimizer] Allow literal arguments to match parameters that conform to `ExpressibleBy{Integer, Float}Literal` `scoreCandidateMatch` needs to reflect the fact that literal can assume any type that conforms to `ExpressibleBy{Integer, Float}Literal` protocol but since such bindings are non-default and alway produce a worse solution vs. default literal types if all of the arguments are literals let's use only exact matches otherwise there is a chance of "over-favoring" and creating more work for ranking. --- lib/Sema/CSOptimizer.cpp | 23 +++++++++++++++++-- .../old_hack_related_ambiguities.swift | 9 ++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c8cd3bfc9ce2a..cff09dbee7103 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -481,6 +481,22 @@ static Constraint *determineBestChoicesInContext( } if (options.contains(MatchFlag::Literal)) { + // Integer and floating-point literals can match any parameter + // type that conforms to `ExpressibleBy{Integer, Float}Literal` + // protocol but since that would constitute a non-default binding + // the score has to be slightly lowered. + if (!paramType->hasTypeParameter()) { + if (candidateType->isInt() && + TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByIntegerLiteral)) + return 0.2; + + if (candidateType->isDouble() && + TypeChecker::conformsToKnownProtocol( + paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) + return 0.2; + } + return 0; } @@ -634,7 +650,10 @@ static Constraint *determineBestChoicesInContext( if (!matchings) return; - bool favorExactMatchesOnly = false; + // 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; // Preserves old behavior where for unary calls to members // the solver would not consider choices that didn't match on // the number of parameters (regardless of defaults) and only @@ -812,7 +831,7 @@ static Constraint *determineBestChoicesInContext( [&resultTy](const auto ¶m) { return param.getPlainType()->isEqual(resultTy); })) - score += 0.001; + score += 0.1; } favoredChoices.push_back({choice, score}); diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index d1dc81b514fab..36c1c99c3b1ed 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -237,3 +237,12 @@ do { assert(s.isEmpty, "") // Ok } } + +extension Double { + public static func * (left: Float, right: Double) -> Double { 0 } +} + +func test_non_default_literal_use(arg: Float) { + let v = arg * 2.0 // shouldn't use `(Float, Double) -> Double` overload + let _: Float = v // Ok +} From 8bd288447f378034d68168f3eccd3433c206940c Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 16 Oct 2024 11:06:15 -0700 Subject: [PATCH 42/60] [CSGen] NFC: Remove obsolete `ConstraintSystem::{get, set}FavoredType` --- include/swift/Sema/ConstraintSystem.h | 13 ------------- lib/Sema/CSGen.cpp | 10 ---------- 2 files changed, 23 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 720bb16e6b828..74ec4e58acac2 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -2211,10 +2211,6 @@ class ConstraintSystem { llvm::SetVector TypeVariables; - /// Maps expressions to types for choosing a favored overload - /// type in a disjunction constraint. - llvm::DenseMap FavoredTypes; - /// Maps discovered closures to their types inferred /// from declared parameters/result and body. /// @@ -2955,15 +2951,6 @@ class ConstraintSystem { return nullptr; } - TypeBase* getFavoredType(Expr *E) { - assert(E != nullptr); - return this->FavoredTypes[E]; - } - void setFavoredType(Expr *E, TypeBase *T) { - assert(E != nullptr); - this->FavoredTypes[E] = T; - } - /// Set the type in our type map for the given node, and record the change /// in the trail. /// diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index ad42de3c99273..76cdb2c4eed1f 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -403,7 +403,6 @@ namespace { Type fixedOutputType = CS.getFixedTypeRecursive(outputTy, /*wantRValue=*/false); if (!fixedOutputType->isTypeVariableOrMember()) { - CS.setFavoredType(anchor, fixedOutputType.getPointer()); outputTy = fixedOutputType; } @@ -807,11 +806,6 @@ namespace { return eltType; } } - - if (!knownType->hasPlaceholder()) { - // Set the favored type for this expression to the known type. - CS.setFavoredType(E, knownType.getPointer()); - } } } @@ -1290,9 +1284,6 @@ namespace { CS.getASTContext()); } - if (auto favoredTy = CS.getFavoredType(expr->getSubExpr())) { - CS.setFavoredType(expr, favoredTy); - } return CS.getType(expr->getSubExpr()); } @@ -2567,7 +2558,6 @@ namespace { Type fixedType = CS.getFixedTypeRecursive(resultType, /*wantRvalue=*/true); if (!fixedType->isTypeVariableOrMember()) { - CS.setFavoredType(expr, fixedType.getPointer()); resultType = fixedType; } From 28396a6dce2eaff21eb6eaffb34275dfff6e4d35 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 21 Oct 2024 13:30:26 -0700 Subject: [PATCH 43/60] [Tests] NFC: Move simd related test-case from `slow` to `fast` --- .../{slow => fast}/simd_scalar_multiple.swift.gyb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/simd_scalar_multiple.swift.gyb (61%) 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 61% 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 30d886028d7b5..6d189530e690a 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: no_asan From ff8663ff161cc96cc92e897618dd56c529915038 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 31 Oct 2024 14:41:45 -0700 Subject: [PATCH 44/60] [Tests] NFC: Update a couple of type-checker tests --- .../{slow => fast}/nil_coalescing.swift.gyb | 2 +- .../{fast => slow}/borderline_flat_map_operator_mix.swift | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/nil_coalescing.swift.gyb (58%) rename validation-test/Sema/type_checker_perf/{fast => slow}/borderline_flat_map_operator_mix.swift (50%) 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/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)] From 9fb73143f6e6462bc9664a00fc00c72e9f70a241 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 1 Nov 2024 09:42:06 -0700 Subject: [PATCH 45/60] [CSOptimizer] Limit "old" behavior compatibility to unlabeled unary arguments `favorMatchingOverloadExprs` used `getUnlabeledUnaryExpr` and we need to mimic that exactly because there is code that relies on this behavior now. --- lib/Sema/CSOptimizer.cpp | 33 ++++++++++++------- .../old_hack_related_ambiguities.swift | 11 +++++++ 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index cff09dbee7103..1acdfb995186f 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -181,6 +181,15 @@ static OverloadedDeclRefExpr *isOverloadedDeclRef(Constraint *disjunction) { return nullptr; } +static unsigned numOverloadChoicesMatchingOnArity(OverloadedDeclRefExpr *ODRE, + ArgumentList *arguments) { + return llvm::count_if(ODRE->getDecls(), [&arguments](auto *choice) { + if (auto *paramList = getParameterList(choice)) + return arguments->size() == paramList->size(); + return false; + }); +} + /// This maintains an "old hack" behavior where overloads of some /// `OverloadedDeclRef` calls were favored purely based on number of /// argument and (non-defaulted) parameters matching. @@ -191,11 +200,7 @@ static void findFavoredChoicesBasedOnArity( if (!ODRE) return; - if (llvm::count_if(ODRE->getDecls(), [&argumentList](auto *choice) { - if (auto *paramList = getParameterList(choice)) - return argumentList->size() == paramList->size(); - return false; - }) > 1) + if (numOverloadChoicesMatchingOnArity(ODRE, argumentList) > 1) return; auto isVariadicGenericOverload = [&](ValueDecl *choice) { @@ -631,7 +636,16 @@ static Constraint *determineBestChoicesInContext( double bestScore = 0.0; SmallVector, 2> favoredChoices; - bool isOverloadedDeclRefDisjunction = isOverloadedDeclRef(disjunction); + // 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, @@ -654,11 +668,8 @@ static Constraint *determineBestChoicesInContext( // matches to filter out non-default literal bindings which otherwise // could cause "over-favoring". bool favorExactMatchesOnly = onlyLiteralCandidates; - // Preserves old behavior where for unary calls to members - // the solver would not consider choices that didn't match on - // the number of parameters (regardless of defaults) and only - // exact matches were favored. - if (!isOverloadedDeclRefDisjunction && argumentList->size() == 1) { + + if (preserveFavoringOfUnlabeledUnaryArgument) { // Old behavior completely disregarded the fact that some of // the parameters could be defaulted. if (overloadType->getNumParams() != 1) diff --git a/test/Constraints/old_hack_related_ambiguities.swift b/test/Constraints/old_hack_related_ambiguities.swift index 36c1c99c3b1ed..da8da45cc76dc 100644 --- a/test/Constraints/old_hack_related_ambiguities.swift +++ b/test/Constraints/old_hack_related_ambiguities.swift @@ -43,6 +43,17 @@ do { do { struct S { let n: Int + + func test(v: String) -> Int { } + func test(v: String, flag: Bool = false) -> Int? { } + + + func verify(v: String) -> Int? { + guard let _ = test(v: v) else { // Ok + return nil + } + return 0 + } } func f(_: String, _ p: Bool = false) -> S? { From c2a55886f05da8c2b5e422d497ce46fe0dde7259 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 4 Nov 2024 00:21:42 -0800 Subject: [PATCH 46/60] [CSOptimizer] Fix `selectDisjunction` to use favored choices even if disjunction was not optimized Some disjunctions e.g. explicit coercions, compositions of restrictions, and implicit CGFloat initializers have favored choices set independently from optimization algorithm, `selectDisjunction` should account for such situations. --- lib/Sema/CSOptimizer.cpp | 38 ++++++++----------- test/Constraints/casts_swift6.swift | 6 +-- ...rals_and_operators_cast_to_float.swift.gyb | 10 +++++ 3 files changed, 28 insertions(+), 26 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/array_of_literals_and_operators_cast_to_float.swift.gyb diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 1acdfb995186f..d3e84d1982d3e 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -244,7 +244,7 @@ static void findFavoredChoicesBasedOnArity( /// Given a set of disjunctions, attempt to determine /// favored choices in the current context. -static Constraint *determineBestChoicesInContext( +static void determineBestChoicesInContext( ConstraintSystem &cs, SmallVectorImpl &disjunctions, llvm::DenseMap>> @@ -267,14 +267,14 @@ static Constraint *determineBestChoicesInContext( auto argumentList = cs.getArgumentList(applicableFn.get()->getLocator()); if (!argumentList) - return nullptr; + return; for (const auto &argument : *argumentList) { if (auto *expr = argument.getExpr()) { // Directly `<#...#>` or has one inside. if (isa(expr) || cs.containsIDEInspectionTarget(expr)) - return nullptr; + return; } } @@ -924,7 +924,7 @@ static Constraint *determineBestChoicesInContext( } if (bestOverallScore == 0) - return nullptr; + return; for (auto &entry : disjunctionScores) { TinyPtrVector favoredChoices; @@ -932,19 +932,6 @@ static Constraint *determineBestChoicesInContext( favoredChoices.push_back(choice); favorings[entry.first] = std::make_pair(entry.second, favoredChoices); } - - Constraint *bestDisjunction = nullptr; - for (auto *disjunction : disjunctions) { - if (disjunctionScores[disjunction] != bestOverallScore) - continue; - - if (!bestDisjunction) - bestDisjunction = disjunction; - else // Multiple disjunctions with the same score. - return nullptr; - } - - return bestDisjunction; } // Attempt to find a disjunction of bind constraints where all options @@ -1019,9 +1006,7 @@ ConstraintSystem::selectDisjunction() { llvm::DenseMap>> favorings; - if (auto *bestDisjunction = - determineBestChoicesInContext(*this, disjunctions, favorings)) - return std::make_pair(bestDisjunction, favorings[bestDisjunction].second); + determineBestChoicesInContext(*this, disjunctions, favorings); // Pick the disjunction with the smallest number of favored, then active // choices. @@ -1034,16 +1019,23 @@ ConstraintSystem::selectDisjunction() { auto &[firstScore, firstFavoredChoices] = favorings[first]; auto &[secondScore, secondFavoredChoices] = favorings[second]; + bool isFirstSupported = isSupportedDisjunction(first); + bool isSecondSupported = isSupportedDisjunction(second); + // Rank based on scores only if both disjunctions are supported. - if (isSupportedDisjunction(first) && isSupportedDisjunction(second)) { + if (isFirstSupported && isSecondSupported) { // If both disjunctions have the same score they should be ranked // based on number of favored/active choices. if (firstScore != secondScore) return firstScore > secondScore; } - unsigned numFirstFavored = firstFavoredChoices.size(); - unsigned numSecondFavored = secondFavoredChoices.size(); + unsigned numFirstFavored = isFirstSupported + ? firstFavoredChoices.size() + : first->countFavoredNestedConstraints(); + unsigned numSecondFavored = + isSecondSupported ? secondFavoredChoices.size() + : second->countFavoredNestedConstraints(); if (numFirstFavored == numSecondFavored) { if (firstActive != secondActive) 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/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] From 15c773b9d7765467e562e03ca1a0d8b8b57b6f62 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 11 Nov 2024 09:55:09 -0800 Subject: [PATCH 47/60] [CSOptimizer] Make a light-weight generic overload check if some requirements are unsatisfiable If some of the requirements of a generic overload reference other generic parameters, the optimizer won't be able to satisfy them because it only has candidates for one (current) parameter. In cases like that, let's fallback to a light-weight protocol conformance check instead of skipping an overload choice altogether. --- lib/Sema/CSOptimizer.cpp | 67 ++++++++++++++----- .../collections_chained_with_plus.swift.gyb | 18 +++++ 2 files changed, 68 insertions(+), 17 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/slow/collections_chained_with_plus.swift.gyb diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index d3e84d1982d3e..c15d8ec773361 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -555,11 +555,9 @@ static void determineBestChoicesInContext( // Check protocol requirement(s) if this parameter is a // generic parameter type. if (genericSig && paramType->isTypeParameter()) { - // If candidate is not fully resolved or is matched against a - // dependent member type (i.e. `Self.T`), let's check conformances - // only and lower the score. - if (candidateType->hasTypeVariable() || - paramType->is()) { + // 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) { @@ -574,29 +572,64 @@ static void determineBestChoicesInContext( } return 0; + }; + + // If candidate is not fully resolved or is matched against a + // dependent member type (i.e. `Self.T`), let's check conformances + // only and lower the score. + if (candidateType->hasTypeVariable() || + paramType->is()) { + return checkProtocolRequirementsOnly(); } // Cannot match anything but generic type parameters here. if (!paramType->is()) return std::nullopt; - // If the candidate type is fully resolved, let's check all of - // the requirements that are associated with the corresponding - // parameter, if all of them are satisfied this candidate is - // an exact match. - - auto isParameterType = [¶mType](Type type) { - return type->isEqual(paramType); - }; - + bool hasUnsatisfiableRequirements = false; SmallVector requirements; + for (const auto &requirement : genericSig.getRequirements()) { - if (requirement.getFirstType().findIf(isParameterType) || - (requirement.getKind() != RequirementKind::Layout && - requirement.getSecondType().findIf(isParameterType))) + 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 { 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 +} From 867e64182f3dc847ca8cb1ba865a81d1d20ece8f Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 11 Nov 2024 11:46:18 -0800 Subject: [PATCH 48/60] [CSOptimizer] Mark compiler synthesized disjunctions as optimized If a disjunction has favored choices, let's mark it as optimized with a high score to make sure that such disjunctions are prioritized since only disjunctions that could have their choices fovored independently from the optimization algorithm are compiler synthesized ones for things like IUO references, explicit coercions etc. --- lib/Sema/CSOptimizer.cpp | 111 +++++++++++++++++++++------------------ 1 file changed, 60 insertions(+), 51 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c15d8ec773361..f16f9e0a8105d 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -34,6 +34,19 @@ 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`. @@ -246,16 +259,30 @@ static void findFavoredChoicesBasedOnArity( /// favored choices in the current context. static void determineBestChoicesInContext( ConstraintSystem &cs, SmallVectorImpl &disjunctions, - llvm::DenseMap>> - &favorings) { + llvm::DenseMap &result) { double bestOverallScore = 0.0; - // Tops scores across all of the disjunctions. - llvm::DenseMap disjunctionScores; - llvm::DenseMap> - favoredChoicesPerDisjunction; + + 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); @@ -282,14 +309,14 @@ static void determineBestChoicesInContext( // of `OverloadedDeclRef` calls were favored purely // based on arity of arguments and parameters matching. { - findFavoredChoicesBasedOnArity( - cs, disjunction, argumentList, [&](Constraint *choice) { - favoredChoicesPerDisjunction[disjunction].push_back(choice); - }); - - if (!favoredChoicesPerDisjunction[disjunction].empty()) { - disjunctionScores[disjunction] = 0.01; - bestOverallScore = std::max(bestOverallScore, 0.01); + llvm::TinyPtrVector favoredChoices; + findFavoredChoicesBasedOnArity(cs, disjunction, argumentList, + [&favoredChoices](Constraint *choice) { + favoredChoices.push_back(choice); + }); + + if (!favoredChoices.empty()) { + recordResult(disjunction, {/*score=*/0.01, favoredChoices}); continue; } } @@ -894,17 +921,16 @@ static void determineBestChoicesInContext( << " with score " << bestScore << "\n"; } - // No matching overload choices to favor. - if (bestScore == 0.0) - continue; - bestOverallScore = std::max(bestOverallScore, bestScore); - disjunctionScores[disjunction] = bestScore; + DisjunctionInfo info(/*score=*/bestScore); + for (const auto &choice : favoredChoices) { if (choice.second == bestScore) - favoredChoicesPerDisjunction[disjunction].push_back(choice.first); + info.FavoredChoices.push_back(choice.first); } + + recordResult(disjunction, std::move(info)); } if (cs.isDebugMode() && bestOverallScore > 0) { @@ -935,14 +961,15 @@ static void determineBestChoicesInContext( getLogger(/*extraIndent=*/4) << "Best overall score = " << bestOverallScore << '\n'; - for (const auto &entry : disjunctionScores) { + for (auto *disjunction : disjunctions) { + auto &entry = result[disjunction]; getLogger(/*extraIndent=*/4) << "[Disjunction '" - << entry.first->getNestedConstraints()[0]->getFirstType()->getString( + << disjunction->getNestedConstraints()[0]->getFirstType()->getString( PO) - << "' with score = " << entry.second << '\n'; + << "' with score = " << entry.Score.value_or(0) << '\n'; - for (const auto *choice : favoredChoicesPerDisjunction[entry.first]) { + for (const auto *choice : entry.FavoredChoices) { auto &log = getLogger(/*extraIndent=*/6); log << "- "; @@ -955,16 +982,6 @@ static void determineBestChoicesInContext( getLogger() << ")\n"; } - - if (bestOverallScore == 0) - return; - - for (auto &entry : disjunctionScores) { - TinyPtrVector favoredChoices; - for (auto *choice : favoredChoicesPerDisjunction[entry.first]) - favoredChoices.push_back(choice); - favorings[entry.first] = std::make_pair(entry.second, favoredChoices); - } } // Attempt to find a disjunction of bind constraints where all options @@ -1036,9 +1053,7 @@ ConstraintSystem::selectDisjunction() { if (auto *disjunction = selectBestBindingDisjunction(*this, disjunctions)) return std::make_pair(disjunction, llvm::TinyPtrVector()); - llvm::DenseMap>> - favorings; + llvm::DenseMap favorings; determineBestChoicesInContext(*this, disjunctions, favorings); // Pick the disjunction with the smallest number of favored, then active @@ -1052,23 +1067,16 @@ ConstraintSystem::selectDisjunction() { auto &[firstScore, firstFavoredChoices] = favorings[first]; auto &[secondScore, secondFavoredChoices] = favorings[second]; - bool isFirstSupported = isSupportedDisjunction(first); - bool isSecondSupported = isSupportedDisjunction(second); - // Rank based on scores only if both disjunctions are supported. - if (isFirstSupported && isSecondSupported) { + if (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; + if (*firstScore != *secondScore) + return *firstScore > *secondScore; } - unsigned numFirstFavored = isFirstSupported - ? firstFavoredChoices.size() - : first->countFavoredNestedConstraints(); - unsigned numSecondFavored = - isSecondSupported ? secondFavoredChoices.size() - : second->countFavoredNestedConstraints(); + unsigned numFirstFavored = firstFavoredChoices.size(); + unsigned numSecondFavored = secondFavoredChoices.size(); if (numFirstFavored == numSecondFavored) { if (firstActive != secondActive) @@ -1082,7 +1090,8 @@ ConstraintSystem::selectDisjunction() { }); if (bestDisjunction != disjunctions.end()) - return std::make_pair(*bestDisjunction, favorings[*bestDisjunction].second); + return std::make_pair(*bestDisjunction, + favorings[*bestDisjunction].FavoredChoices); return std::nullopt; } From 87cd5f873364890ac76a6ef115a3283d2cde8363 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 12 Nov 2024 09:48:38 -0800 Subject: [PATCH 49/60] [CSOptimizer] Add support for chained members without arguments If disjunction represents a member reference that has no arguments applied, let's score that as `1` to indicate that it should be priorized. This helps in situations like `a.b + 1` where resolving `a.b` member chain helps to establish context for `+`. --- lib/Sema/CSOptimizer.cpp | 40 ++++++++++++++++++- ...ns_and_operators_with_iou_base_types.swift | 35 ++++++++++++++++ 2 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index f16f9e0a8105d..c9d433196e762 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -253,6 +253,37 @@ static void findFavoredChoicesBasedOnArity( favoredChoice(choice); } +/// Determine whether the given disjunction serves as a base of +/// another member reference i.e. `x.y` where `x` could be overloaded. +static bool isPartOfMemberChain(ConstraintSystem &CS, Constraint *disjunction) { + if (isOperatorDisjunction(disjunction)) + return false; + + auto &CG = CS.getConstraintGraph(); + + TypeVariableType *typeVar = nullptr; + + // If disjunction is applied, the member is chained on the result. + if (auto appliedFn = CS.getAppliedDisjunctionArgumentFunction(disjunction)) { + typeVar = appliedFn->getResult()->getAs(); + } else { + typeVar = disjunction->getNestedConstraints()[0] + ->getFirstType() + ->getAs(); + } + + if (!typeVar) + return false; + + return llvm::any_of( + CG[typeVar].getConstraints(), [&typeVar](Constraint *constraint) { + if (constraint->getKind() != ConstraintKind::ValueMember) + return false; + + return constraint->getFirstType()->isEqual(typeVar); + }); +} + } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -286,8 +317,15 @@ static void determineBestChoicesInContext( auto applicableFn = getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); - if (applicableFn.isNull()) + if (applicableFn.isNull()) { + // If this is a chained member reference it could be prioritized since + // it helps to establish context for other calls i.e. `a.b + 2` if + // `a` is a disjunction it should be preferred over `+`. + if (isPartOfMemberChain(cs, disjunction)) + recordResult(disjunction, {/*score=*/1.0}); + continue; + } auto argFuncType = applicableFn.get()->getFirstType()->getAs(); diff --git a/validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift b/validation-test/Sema/type_checker_perf/fast/member_chains_and_operators_with_iou_base_types.swift 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) +} From b7e749307644bd2f5cc5daf243a0edfdf67741e0 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 22 Nov 2024 17:25:45 -0800 Subject: [PATCH 50/60] [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. --- lib/Sema/CSBindings.cpp | 32 +++++++++++++++++++++++++++++++- 1 file changed, 31 insertions(+), 1 deletion(-) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index 0b67bbf022ac3..dda17fb35915a 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -31,6 +31,12 @@ using namespace swift; using namespace constraints; using namespace inference; +/// 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); @@ -1209,7 +1215,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 cb876cbd9e4123cc6ba9a989b0cd91e5f79dc36a Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 25 Nov 2024 20:30:04 -0800 Subject: [PATCH 51/60] [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?`. --- lib/Sema/CSSimplify.cpp | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 3973f16cf6b66..1016216eab701 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -14690,9 +14690,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()) { From bf8ae3bc1b6ad57d300d8a9207283d89482bb968 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 25 Nov 2024 21:55:57 -0800 Subject: [PATCH 52/60] [CSOptimizer] Allow only widening CGFloat->Double conversions while matching candidate arguments 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. --- lib/Sema/CSOptimizer.cpp | 39 +++++++++++++------ ...l_with_operators_and_double_literals.swift | 31 +++++++++++++++ 2 files changed, 59 insertions(+), 11 deletions(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index c9d433196e762..3936df099d005 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -530,18 +530,22 @@ static void determineBestChoicesInContext( scoreCandidateMatch = [&](GenericSignature genericSig, Type candidateType, Type paramType, MatchOptions options) -> std::optional { - auto areEqual = [&options](Type a, Type b) { - // Double<->CGFloat implicit conversion support for literals - // only since in this case the conversion might not result in - // score penalty. - if (options.contains(MatchFlag::Literal) && - ((a->isDouble() && b->isCGFloat()) || - (a->isCGFloat() && b->isDouble()))) - return true; - + auto areEqual = [&](Type a, Type b) { return a->getDesugaredType()->isEqual(b->getDesugaredType()); }; + // Allow CGFloat -> Double widening conversions between + // candidate argument types and parameter types. This would + // make sure that Double is always preferred over CGFloat + // when using literals and ranking supported disjunction + // choices. Narrowing conversion (Double -> CGFloat) should + // be delayed as much as possible. + if (options.contains(MatchFlag::OnParam)) { + if (candidateType->isCGFloat() && paramType->isDouble()) { + return options.contains(MatchFlag::Literal) ? 0.2 : 0.9; + } + } + if (options.contains(MatchFlag::ExactOnly)) return areEqual(candidateType, paramType) ? 1 : 0; @@ -559,12 +563,12 @@ static void determineBestChoicesInContext( if (candidateType->isInt() && TypeChecker::conformsToKnownProtocol( paramType, KnownProtocolKind::ExpressibleByIntegerLiteral)) - return 0.2; + return paramType->isDouble() ? 0.2 : 0.3; if (candidateType->isDouble() && TypeChecker::conformsToKnownProtocol( paramType, KnownProtocolKind::ExpressibleByFloatLiteral)) - return 0.2; + return 0.3; } return 0; @@ -916,6 +920,19 @@ static void determineBestChoicesInContext( // with a lot of favored overloads because on the result type alone. if (decl->isOperator() && !isStandardComparisonOperator(decl)) { if (llvm::any_of(resultTypes, [&](const Type candidateResultTy) { + // Avoid increasing weight based on CGFloat result type + // match because that could require narrowing conversion + // in the arguments and that is always detrimental. + // + // For example, `has_CGFloat_param(1.0 + 2.0)` should use + // `+(_: Double, _: Double) -> Double` instead of + // `+(_: CGFloat, _: CGFloat) -> CGFloat` which would match + // parameter of `has_CGFloat_param` exactly but use a + // narrowing conversion for both literals. + if (candidateResultTy->lookThroughAllOptionalTypes() + ->isCGFloat()) + return false; + return scoreCandidateMatch(genericSig, overloadType->getResult(), candidateResultTy, diff --git a/validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift b/validation-test/Sema/type_checker_perf/fast/array_literal_with_operators_and_double_literals.swift 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) +] From 56d6635e46e9c2b88c40f2fabad3a452d6748f9d Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 28 Nov 2024 00:48:04 -0800 Subject: [PATCH 53/60] [CSOptimizer] Implement special prioritization rules for result builder contexts 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. --- lib/Sema/CSOptimizer.cpp | 78 +++++++++++++++++++ .../complex_swiftui_padding_conditions.swift | 17 ++++ 2 files changed, 95 insertions(+) create mode 100644 validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 3936df099d005..03ac05fab72e3 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -1097,6 +1097,79 @@ selectBestBindingDisjunction(ConstraintSystem &cs, return firstBindDisjunction; } +/// Prioritize `build{Block, Expression, ...}` and any chained +/// members that are connected to individual builder elements +/// i.e. `ForEach(...) { ... }.padding(...)`, once `ForEach` +/// is resolved, `padding` should be prioritized because its +/// requirements can help prune the solution space before the +/// body is checked. +static Constraint * +selectDisjunctionInResultBuilderContext(ConstraintSystem &cs, + ArrayRef disjunctions) { + auto context = AnyFunctionRef::fromDeclContext(cs.DC); + if (!context) + return nullptr; + + if (!cs.getAppliedResultBuilderTransform(context.value())) + return nullptr; + + std::pair best{nullptr, 0}; + for (auto *disjunction : disjunctions) { + auto *member = + getAsExpr(disjunction->getLocator()->getAnchor()); + if (!member) + continue; + + // Attempt `build{Block, Expression, ...} first because they + // provide contextual information for the inner calls. + if (isResultBuilderMethodReference(cs.getASTContext(), member)) + return disjunction; + + Expr *curr = member; + bool disqualified = false; + // Walk up the parent expression chain and check whether this + // disjunction represents one of the members in a chain that + // leads up to `buildExpression` (if defined by the builder) + // or to a pattern binding for `$__builderN` (the walk won't + // find any argument position locations in that case). + while (auto parent = cs.getParentExpr(curr)) { + if (!(isExpr(parent) || isExpr(parent))) { + disqualified = true; + break; + } + + if (auto *call = getAsExpr(parent)) { + // The current parent appears in an argument position. + if (call->getFn() != curr) { + // Allow expressions that appear in a argument position to + // `build{Expression, Block, ...} methods. + if (auto *UDE = getAsExpr(call->getFn())) { + disqualified = + !isResultBuilderMethodReference(cs.getASTContext(), UDE); + } else { + disqualified = true; + } + } + } + + if (disqualified) + break; + + curr = parent; + } + + if (disqualified) + continue; + + if (auto depth = cs.getExprDepth(member)) { + if (!best.first || best.second > depth) + best = std::make_pair(disjunction, depth.value()); + } + } + + return best.first; +} + std::optional>> ConstraintSystem::selectDisjunction() { SmallVector disjunctions; @@ -1111,6 +1184,11 @@ ConstraintSystem::selectDisjunction() { llvm::DenseMap favorings; determineBestChoicesInContext(*this, disjunctions, favorings); + if (auto *disjunction = + selectDisjunctionInResultBuilderContext(*this, disjunctions)) { + return std::make_pair(disjunction, favorings[disjunction].FavoredChoices); + } + // Pick the disjunction with the smallest number of favored, then active // choices. auto bestDisjunction = std::min_element( diff --git a/validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift b/validation-test/Sema/type_checker_perf/fast/complex_swiftui_padding_conditions.swift 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) + } +} From 40a41c82d94a5474e9193600ba5c60776d46a234 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 5 Dec 2024 09:30:23 -0800 Subject: [PATCH 54/60] [CSBindings] Don't attempt to strip optional for closure result types Let's not perform $T? -> $T for closure result types to avoid having to re-discover solutions that differ only in location of optional injection. The pattern with such type variables is: ``` $T_body $T_result $T_contextual_result ``` When `$T_contextual_result` is `Optional<$U>`, the optional injection can either happen from `$T_body` or from `$T_result` (if `return` expression is non-optional), if we allow both the solver would find two solutions that differ only in location of optional injection. --- lib/Sema/CSBindings.cpp | 38 ++++++++++++++----- test/Constraints/argument_matching.swift | 4 ++ .../{slow => fast}/rdar22079400.swift | 2 +- 3 files changed, 33 insertions(+), 11 deletions(-) rename validation-test/Sema/type_checker_perf/{slow => fast}/rdar22079400.swift (74%) diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index dda17fb35915a..913ab01db052e 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -2447,17 +2447,35 @@ bool TypeVarBindingProducer::computeNext() { if (binding.Kind == BindingKind::Subtypes || CS.shouldAttemptFixes()) { // If we were unsuccessful solving for T?, try solving for T. if (auto objTy = type->getOptionalObjectType()) { - // If T is a type variable, only attempt this if both the - // type variable we are trying bindings for, and the type - // variable we will attempt to bind, both have the same - // polarity with respect to being able to bind lvalues. - if (auto otherTypeVar = objTy->getAs()) { - if (TypeVar->getImpl().canBindToLValue() == - otherTypeVar->getImpl().canBindToLValue()) { - addNewBinding(binding.withSameSource(objTy, binding.Kind)); + // TODO: This could be generalized in the future to cover all patterns + // that have an intermediate type variable in subtype/conversion chain. + // + // Let's not perform $T? -> $T for closure result types to avoid having + // to re-discover solutions that differ only in location of optional + // injection. + // + // The pattern with such type variables is: + // + // $T_body $T_result $T_contextual_result + // + // When $T_contextual_result is Optional<$U>, the optional injection + // can either happen from $T_body or from $T_result (if `return` + // expression is non-optional), if we allow both the solver would + // find two solutions that differ only in location of optional + // injection. + if (!TypeVar->getImpl().isClosureResultType()) { + // If T is a type variable, only attempt this if both the + // type variable we are trying bindings for, and the type + // variable we will attempt to bind, both have the same + // polarity with respect to being able to bind lvalues. + if (auto otherTypeVar = objTy->getAs()) { + if (TypeVar->getImpl().canBindToLValue() == + otherTypeVar->getImpl().canBindToLValue()) { + addNewBinding(binding.withType(objTy)); + } + } else { + addNewBinding(binding.withType(objTy)); } - } else { - addNewBinding(binding.withSameSource(objTy, binding.Kind)); } } } diff --git a/test/Constraints/argument_matching.swift b/test/Constraints/argument_matching.swift index 2b78c083000c4..57f173eb0d19d 100644 --- a/test/Constraints/argument_matching.swift +++ b/test/Constraints/argument_matching.swift @@ -1488,15 +1488,19 @@ func trailingclosure4(f: () -> Int) {} trailingclosure4 { 5 } func trailingClosure5(_ file: String = #file, line: UInt = #line, expression: () -> T?) { } +// expected-note@-1 {{in call to function 'trailingClosure5(_:line:expression:)'}} func trailingClosure6(value: Int, expression: () -> T?) { } +// expected-note@-1 {{in call to function 'trailingClosure6(value:expression:)'}} trailingClosure5(file: "hello", line: 17) { // expected-error{{extraneous argument label 'file:' in call}}{{18-24=}} + // expected-error@-1 {{generic parameter 'T' could not be inferred}} return Optional.Some(5) // expected-error@-1 {{enum type 'Optional' has no case 'Some'; did you mean 'some'?}} {{19-23=some}} // expected-error@-2 {{generic parameter 'Wrapped' could not be inferred}} // expected-note@-3 {{explicitly specify the generic arguments to fix this issue}} } trailingClosure6(5) { // expected-error{{missing argument label 'value:' in call}}{{18-18=value: }} + // expected-error@-1 {{generic parameter 'T' could not be inferred}} return Optional.Some(5) // expected-error@-1 {{enum type 'Optional' has no case 'Some'; did you mean 'some'?}} {{19-23=some}} // expected-error@-2 {{generic parameter 'Wrapped' could not be inferred}} diff --git a/validation-test/Sema/type_checker_perf/slow/rdar22079400.swift b/validation-test/Sema/type_checker_perf/fast/rdar22079400.swift similarity index 74% rename from validation-test/Sema/type_checker_perf/slow/rdar22079400.swift rename to validation-test/Sema/type_checker_perf/fast/rdar22079400.swift index ea8334f5b28fa..3f6a797c4b5f3 100644 --- a/validation-test/Sema/type_checker_perf/slow/rdar22079400.swift +++ b/validation-test/Sema/type_checker_perf/fast/rdar22079400.swift @@ -1,7 +1,7 @@ // RUN: %target-typecheck-verify-swift -solver-expression-time-threshold=1 // REQUIRES: tools-release,no_asan -let _ = (0...1).lazy.flatMap { // expected-error {{reasonable time}} +let _ = (0...1).lazy.flatMap { a in (1...2).lazy.map { b in (a, b) } }.filter { 1 < $0 && $0 < $1 && $0 + $1 < 3 From 95b47aead6e7a7a933e3c764e3c918ba7507465e Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 6 Dec 2024 21:20:10 -0800 Subject: [PATCH 55/60] [CSOptimizer] Reduce overload types before ranking This would make sure that any same-type requirements to a concrete type are substituted with such types which is especially important for SIMD operators like `&{+, -}` because they have `Scalar == (U)Int*` requirements. --- lib/Sema/CSOptimizer.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 03ac05fab72e3..3878a1d771bdd 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -780,6 +780,14 @@ static void determineBestChoicesInContext( favorExactMatchesOnly = true; } + // This is important for SIMD operators in particular because + // a lot of their overloads have same-type requires to a concrete + // type: `(_: SIMD*, ...) -> ...`. + if (genericSig) { + overloadType = overloadType->getReducedType(genericSig) + ->castTo(); + } + double score = 0.0; unsigned numDefaulted = 0; for (unsigned paramIdx = 0, n = overloadType->getNumParams(); From c767f7aff7ec02d0d772455df56bb8cc8cdd8ff3 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 16 Dec 2024 18:57:06 -0800 Subject: [PATCH 56/60] [ConstraintSystem] Fix `getEffectiveOverloadType` handling of `mutating` methods --- 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 a33ca48501a09..fac56c1b8f9df 100644 --- a/lib/Sema/TypeOfReference.cpp +++ b/lib/Sema/TypeOfReference.cpp @@ -1833,11 +1833,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 f7d81d5271c779da900f5984ff371a0ce7cd28f8 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Mon, 16 Dec 2024 22:09:34 -0800 Subject: [PATCH 57/60] [TypeCheckEffects] Downgrade missing `await` to a warning for initializer calls with a single unlabeled parameter Mitigation for a historic incorrect type-checker behavior caused by one of the performance hacks that used to favor sync constructor overload over async one in async context if initializers take a single unlabeled argument. ```swift struct S { init(_: Int) {} init(_: Int) async {} } func test(v: Int) async { S(v) } ``` The type-checker should use `init(_: Int) async` in `test` context but used to select the sync overload. The hack is now gone but we need to downgrade an error to a warning to give the developers time to fix their code. --- lib/Sema/TypeCheckEffects.cpp | 23 +++++++++++++++++++++++ test/Constraints/async.swift | 23 ++++++++++++++++++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/lib/Sema/TypeCheckEffects.cpp b/lib/Sema/TypeCheckEffects.cpp index 6a4fda5e14268..e9a15d8169887 100644 --- a/lib/Sema/TypeCheckEffects.cpp +++ b/lib/Sema/TypeCheckEffects.cpp @@ -1400,6 +1400,29 @@ class ApplyClassifier { // then look at all of the closure arguments. LLVM_FALLTHROUGH; } else { + // Mitigation for a historic incorrect type-checker behavior + // caused by one of the performance hacks that used to favor + // sync constructor overload over async one in async context + // if initializers take a single unlabeled argument. + // + // struct S { + // init(_: Int) {} + // init(_: Int) async {} + // } + // + // func test(v: Int) async { S(v) } + // + // The type-checker should use `init(_: Int) async` in `test` context + // but used to select the sync overload. The hack is now gone but we + // need to downgrade an error to a warning to give the developers time + // to fix their code. + if (kind == EffectKind::Async && + fnRef.getKind() == AbstractFunction::Function) { + if (auto *ctor = dyn_cast(E->getFn())) { + if (ctor->getFn()->isImplicit() && args->isUnlabeledUnary()) + result.setDowngradeToWarning(true); + } + } break; } diff --git a/test/Constraints/async.swift b/test/Constraints/async.swift index f914d49b56372..9985f169c0a22 100644 --- a/test/Constraints/async.swift +++ b/test/Constraints/async.swift @@ -1,4 +1,4 @@ -// RUN: %target-typecheck-verify-swift +// RUN: %target-typecheck-verify-swift // REQUIRES: concurrency @@ -197,3 +197,24 @@ func test_sync_in_closure_context() { test(42) // Ok (select sync overloads and discards the result) } } + +@available(SwiftStdlib 5.5, *) +func test_async_calls_in_async_context(v: Int) async { + final class Test : Sendable { + init(_: Int) {} + init(_: Int) async {} + + func test(_: Int) {} + func test(_: Int) async {} + + static func test(_: Int) {} + static func test(_: Int) async {} + } + + // Only implicit `.init` should be accepted with a warning due type-checker previously picking an incorrect overload. + _ = Test(v) // expected-warning {{expression is 'async' but is not marked with 'await'; this is an error in the Swift 6 language mode}} expected-note {{call is 'async'}} + _ = Test.init(v) // expected-error {{expression is 'async' but is not marked with 'await'}} expected-note {{call is 'async'}} + + Test.test(v) // expected-error {{expression is 'async' but is not marked with 'await'}} expected-note {{call is 'async'}} + Test(v).test(v) // expected-error {{expression is 'async' but is not marked with 'await'}} expected-note 2 {{call is 'async'}} +} From 43ca7dfff981a927f4b82a2f045ed5bd58099c00 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 17 Dec 2024 09:44:07 -0800 Subject: [PATCH 58/60] [CSOptimizer] Simplify handling of non-applied disjunctions If a disjunction doesn't have an application, let's prefer it if it's passed as an argument to an operator application or involved in a member chain, in such situations attempting a disjunction could provide context to parent call/member chain. If disjunction is passed as an unapplied reference to some parameter i.e. `.map(String.init(describing:))` we don't favor it for now because optimizer cannot do enough checking to determine whether preferring such disjunction would help make forward progress in solving by pruning some space or providing additional context. --- lib/Sema/CSOptimizer.cpp | 56 ++++++++++++--------------------- test/Constraints/operator.swift | 17 ++++++++++ 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 3878a1d771bdd..aae6d9dc3e8f3 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -253,37 +253,6 @@ static void findFavoredChoicesBasedOnArity( favoredChoice(choice); } -/// Determine whether the given disjunction serves as a base of -/// another member reference i.e. `x.y` where `x` could be overloaded. -static bool isPartOfMemberChain(ConstraintSystem &CS, Constraint *disjunction) { - if (isOperatorDisjunction(disjunction)) - return false; - - auto &CG = CS.getConstraintGraph(); - - TypeVariableType *typeVar = nullptr; - - // If disjunction is applied, the member is chained on the result. - if (auto appliedFn = CS.getAppliedDisjunctionArgumentFunction(disjunction)) { - typeVar = appliedFn->getResult()->getAs(); - } else { - typeVar = disjunction->getNestedConstraints()[0] - ->getFirstType() - ->getAs(); - } - - if (!typeVar) - return false; - - return llvm::any_of( - CG[typeVar].getConstraints(), [&typeVar](Constraint *constraint) { - if (constraint->getKind() != ConstraintKind::ValueMember) - return false; - - return constraint->getFirstType()->isEqual(typeVar); - }); -} - } // end anonymous namespace /// Given a set of disjunctions, attempt to determine @@ -318,11 +287,26 @@ static void determineBestChoicesInContext( getApplicableFnConstraint(cs.getConstraintGraph(), disjunction); if (applicableFn.isNull()) { - // If this is a chained member reference it could be prioritized since - // it helps to establish context for other calls i.e. `a.b + 2` if - // `a` is a disjunction it should be preferred over `+`. - if (isPartOfMemberChain(cs, disjunction)) - recordResult(disjunction, {/*score=*/1.0}); + auto *locator = disjunction->getLocator(); + if (auto expr = getAsExpr(locator->getAnchor())) { + if (auto *parentExpr = cs.getParentExpr(expr)) { + // If this is a chained member reference or a direct operator + // argument it could be prioritized since it helps to establish + // context for other calls i.e. `(a.)b + 2` if `a` and/or `b` + // are disjunctions they should be preferred over `+`. + switch (parentExpr->getKind()) { + case ExprKind::Binary: + case ExprKind::PrefixUnary: + case ExprKind::PostfixUnary: + case ExprKind::UnresolvedDot: + recordResult(disjunction, {/*score=*/1.0}); + continue; + + default: + break; + } + } + } continue; } diff --git a/test/Constraints/operator.swift b/test/Constraints/operator.swift index 77d07f5dd3078..d12bcbbc20a04 100644 --- a/test/Constraints/operator.swift +++ b/test/Constraints/operator.swift @@ -375,3 +375,20 @@ do { col <<<>>> val // Ok } } + +// Make sure that ?? selects an overload that doesn't produce an optional. +do { + class Obj { + var x: String! + } + + class Child : Obj { + func x() -> String? { nil } + static func x(_: Int) -> String { "" } + } + + func test(arr: [Child], v: String, defaultV: Child) -> Child { + let result = arr.first { $0.x == v } ?? defaultV + return result // Ok + } +} From 860ae08d1b9e647eb2911991f715b4a80fcb7a32 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 17 Dec 2024 13:24:54 -0800 Subject: [PATCH 59/60] [CSOptimizer] Mark bitwise operators as supported --- 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 aae6d9dc3e8f3..7bb51e3794e85 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -70,7 +70,7 @@ static bool isSupportedOperator(Constraint *disjunction) { auto name = decl->getBaseIdentifier(); if (name.isArithmeticOperator() || name.isStandardComparisonOperator() || - name.is("^")) { + name.isBitwiseOperator()) { return true; } From bc3a15fbe67e13e1ba8b6808b27508f8881a2608 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 18 Dec 2024 00:22:49 -0800 Subject: [PATCH 60/60] [CSOptimizer] 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. --- lib/Sema/CSOptimizer.cpp | 20 ++++++++++++++++++- .../fast/cgfloat_with_unary_operators.swift | 19 ++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) create mode 100644 validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index 7bb51e3794e85..afaa7aaa9acd2 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -490,6 +490,7 @@ static void determineBestChoicesInContext( OnParam = 0x01, Literal = 0x02, ExactOnly = 0x04, + DisableCGFloatDoubleConversion = 0x08, }; using MatchOptions = OptionSet; @@ -518,13 +519,20 @@ static void determineBestChoicesInContext( return a->getDesugaredType()->isEqual(b->getDesugaredType()); }; + auto isCGFloatDoubleConversionSupported = [&options]() { + // CGFloat <-> Double conversion is supposed only while + // match argument candidates to parameters. + return options.contains(MatchFlag::OnParam) && + !options.contains(MatchFlag::DisableCGFloatDoubleConversion); + }; + // Allow CGFloat -> Double widening conversions between // candidate argument types and parameter types. This would // make sure that Double is always preferred over CGFloat // when using literals and ranking supported disjunction // choices. Narrowing conversion (Double -> CGFloat) should // be delayed as much as possible. - if (options.contains(MatchFlag::OnParam)) { + if (isCGFloatDoubleConversionSupported()) { if (candidateType->isCGFloat() && paramType->isDouble()) { return options.contains(MatchFlag::Literal) ? 0.2 : 0.9; } @@ -854,6 +862,16 @@ static void determineBestChoicesInContext( if (favorExactMatchesOnly) options |= MatchFlag::ExactOnly; + // Disable CGFloat -> Double conversion for unary operators. + // + // Some of the unary operators, i.e. prefix `-`, don't have + // CGFloat variants and expect generic `FloatingPoint` overload + // to match CGFloat type. Let's not attempt `CGFloat` -> `Double` + // conversion for unary operators because it always leads + // to a worse solutions vs. generic overloads. + if (n == 1 && decl->isOperator()) + options |= MatchFlag::DisableCGFloatDoubleConversion; + auto candidateScore = scoreCandidateMatch( genericSig, candidateType, paramType, options); diff --git a/validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift b/validation-test/Sema/type_checker_perf/fast/cgfloat_with_unary_operators.swift 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)) +}