From a01692ace1d88379e23a695de6336e08f0745976 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Tue, 11 Nov 2025 15:00:06 -0800 Subject: [PATCH] [CSOptimizer] Skip `??` if it's involved in optional chain with unresolved result type `??` operator is overloaded on optionality of its result. When the first argument matches exactly, the ranking is going to be skewed towards selecting an overload choice that returns a non-optional type. This is not always correct i.e. when operator is involved in optional chaining. To avoid producing an incorrect favoring, let's skip the this disjunction when constraints associated with result type indicate that it should be optional. Simply adding it as a binding won't work because if the second argument is non-optional the overload that returns `T?` would still have a lower score. Resolves: rdar://164201746 --- lib/Sema/CSOptimizer.cpp | 32 +++++++++++++++++++ .../Constraints/nil-coalescing-favoring.swift | 9 ++++++ 2 files changed, 41 insertions(+) diff --git a/lib/Sema/CSOptimizer.cpp b/lib/Sema/CSOptimizer.cpp index ddf2c81cb8348..4a5113fa453f1 100644 --- a/lib/Sema/CSOptimizer.cpp +++ b/lib/Sema/CSOptimizer.cpp @@ -261,6 +261,14 @@ static bool isStandardInfixLogicalOperator(Constraint *disjunction) { return false; } +static bool isNilCoalescingOperator(Constraint *disjunction) { + auto *choice = disjunction->getNestedConstraints()[0]; + if (auto *decl = getOverloadChoiceDecl(choice)) + return decl->isOperator() && + decl->getBaseIdentifier().isNilCoalescingOperator(); + return false; +} + static bool isArithmeticOperator(ValueDecl *decl) { return decl->isOperator() && decl->getBaseIdentifier().isArithmeticOperator(); } @@ -1086,6 +1094,30 @@ static void determineBestChoicesInContext( if (auto *typeVar = resultType->getAs()) { auto bindingSet = cs.getBindingsFor(typeVar); + // `??` operator is overloaded on optionality of its result. When the + // first argument matches exactly, the ranking is going to be skewed + // towards selecting an overload choice that returns a non-optional type. + // This is not always correct i.e. when operator is involved in optional + // chaining. To avoid producing an incorrect favoring, let's skip the this + // disjunction when constraints associated with result type indicate + // that it should be optional. + // + // Simply adding it as a binding won't work because if the second argument + // is non-optional the overload that returns `T?` would still have a lower + // score. + if (!bindingSet && isNilCoalescingOperator(disjunction)) { + auto &cg = cs.getConstraintGraph(); + if (llvm::any_of(cg[typeVar].getConstraints(), + [&typeVar](Constraint *constraint) { + if (constraint->getKind() != + ConstraintKind::OptionalObject) + return false; + return constraint->getFirstType()->isEqual(typeVar); + })) { + continue; + } + } + for (const auto &binding : bindingSet.Bindings) { resultTypes.push_back(binding.BindingType); } diff --git a/test/Constraints/nil-coalescing-favoring.swift b/test/Constraints/nil-coalescing-favoring.swift index 5caa9ca548156..25d60629980da 100644 --- a/test/Constraints/nil-coalescing-favoring.swift +++ b/test/Constraints/nil-coalescing-favoring.swift @@ -62,3 +62,12 @@ func test_no_incorrect_favoring(v: Int?, o: Int) { sameType(s1, as: Int?.self) sameType(s2, as: Int?.self) } + +extension Int { + func test() -> Int { 42 } +} + +func test_optional_chaining(v: Int?, defaultV: Int) { + // CHECK: declref_expr type="(consuming Int?, @autoclosure () throws -> Int?) throws -> Int?" {{.*}} decl="Swift.(file).?? + _ = (v ?? defaultV)?.test() // Ok (no warnings) +}