From dc21edd100cd4d63a8c26e89ec9c3abcf251b662 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 7 Jan 2025 13:39:36 -0500 Subject: [PATCH 1/3] Sema: Clean up local property wrapper bookkeeping --- include/swift/Sema/ConstraintSystem.h | 7 +++++++ lib/Sema/CSApply.cpp | 16 ++++++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 1436bb8df57cc..f3e391f7c1f61 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -1613,6 +1613,13 @@ class Solution { /// A map from argument expressions to their applied property wrapper expressions. llvm::DenseMap> appliedPropertyWrappers; + ArrayRef getAppliedPropertyWrappers(ASTNode anchor) { + auto found = appliedPropertyWrappers.find(anchor); + if (found != appliedPropertyWrappers.end()) + return found->second; + return ArrayRef(); + } + /// A mapping from the constraint locators for references to various /// names (e.g., member references, normal name references, possible /// constructions) to the argument lists for the call to that locator. diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index b1b3a599df9cb..6190486831bbf 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -1189,8 +1189,8 @@ namespace { calleeFnTy = calleeFnTy->getResult()->castTo(); } - const auto &appliedPropertyWrappers = - solution.appliedPropertyWrappers[locator.getAnchor()]; + auto appliedPropertyWrappers = + solution.getAppliedPropertyWrappers(locator.getAnchor()); const auto calleeDeclRef = resolveConcreteDeclRef( dyn_cast(declOrClosure), locator); @@ -2332,8 +2332,8 @@ namespace { ->castTo(); auto fullSubscriptTy = openedFullFnType->getResult() ->castTo(); - auto &appliedWrappers = - solution.appliedPropertyWrappers[memberLoc->getAnchor()]; + auto appliedWrappers = + solution.getAppliedPropertyWrappers(memberLoc->getAnchor()); args = coerceCallArguments( args, fullSubscriptTy, subscriptRef, nullptr, locator.withPathElement(ConstraintLocator::ApplyArgument), @@ -6328,6 +6328,7 @@ ArgumentList *ExprRewriter::coerceCallArguments( auto *paramDecl = getParameterAt(callee, paramIdx); assert(paramDecl); + ASSERT(appliedWrapperIndex < appliedPropertyWrappers.size()); auto appliedWrapper = appliedPropertyWrappers[appliedWrapperIndex++]; auto wrapperType = solution.simplifyType(appliedWrapper.wrapperType); auto initKind = appliedWrapper.initKind; @@ -8230,7 +8231,8 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType, // Resolve into a DynamicTypeExpr. auto args = apply->getArgs(); - auto &appliedWrappers = solution.appliedPropertyWrappers[calleeLocator.getAnchor()]; + auto appliedWrappers = solution.getAppliedPropertyWrappers( + calleeLocator.getAnchor()); auto fnType = cs.getType(fn)->getAs(); args = coerceCallArguments( args, fnType, declRef, apply, @@ -8426,7 +8428,9 @@ Expr *ExprRewriter::finishApply(ApplyExpr *apply, Type openedType, // For function application, convert the argument to the input type of // the function. if (auto fnType = cs.getType(fn)->getAs()) { - auto &appliedWrappers = solution.appliedPropertyWrappers[calleeLocator.getAnchor()]; + auto appliedWrappers = solution.getAppliedPropertyWrappers( + calleeLocator.getAnchor()); + args = coerceCallArguments( args, fnType, callee, apply, locator.withPathElement(ConstraintLocator::ApplyArgument), From 9cfbea3fd9ef68f36c82a397f4c80581ef79701d Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 7 Jan 2025 13:41:04 -0500 Subject: [PATCH 2/3] Sema: Fix local property wrappers on constructor Fixes rdar://problem/142443421. --- include/swift/Sema/ConstraintSystem.h | 3 ++- lib/Sema/CSGen.cpp | 8 +++---- lib/Sema/CSSimplify.cpp | 5 ++++- lib/Sema/TypeOfReference.cpp | 5 +++-- test/Sema/property_wrapper_parameter.swift | 25 ++++++++++++++++++++++ 5 files changed, 37 insertions(+), 9 deletions(-) diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index f3e391f7c1f61..8b632834f0639 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -5221,7 +5221,8 @@ class ConstraintSystem { /// property wrapper type by applying the property wrapper. TypeMatchResult applyPropertyWrapperToParameter( Type wrapperType, Type paramType, ParamDecl *param, Identifier argLabel, - ConstraintKind matchKind, ConstraintLocatorBuilder locator); + ConstraintKind matchKind, ConstraintLocator *locator, + ConstraintLocator *calleeLocator); /// Used by applyPropertyWrapperToParameter() to update appliedPropertyWrappers /// and record a change in the trail. diff --git a/lib/Sema/CSGen.cpp b/lib/Sema/CSGen.cpp index b1b113d03443f..065bcb19891bc 100644 --- a/lib/Sema/CSGen.cpp +++ b/lib/Sema/CSGen.cpp @@ -5057,11 +5057,9 @@ void ConstraintSystem::removePropertyWrapper(Expr *anchor) { ConstraintSystem::TypeMatchResult ConstraintSystem::applyPropertyWrapperToParameter( Type wrapperType, Type paramType, ParamDecl *param, Identifier argLabel, - ConstraintKind matchKind, ConstraintLocatorBuilder locator) { - Expr *anchor = getAsExpr(locator.getAnchor()); - if (auto *apply = dyn_cast(anchor)) { - anchor = apply->getFn(); - } + ConstraintKind matchKind, ConstraintLocator *locator, + ConstraintLocator *calleeLocator) { + Expr *anchor = getAsExpr(calleeLocator->getAnchor()); if (argLabel.hasDollarPrefix() && (!param || !param->hasExternalPropertyWrapper())) { if (!shouldAttemptFixes()) diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 24f6dca84b77d..ca1f3bce03db9 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -1820,7 +1820,9 @@ static ConstraintSystem::TypeMatchResult matchCallArguments( assert(param); if (cs.applyPropertyWrapperToParameter(paramTy, argTy, const_cast(param), - argLabel, subKind, loc) + argLabel, subKind, + cs.getConstraintLocator(loc), + calleeLocator) .isFailure()) { return cs.getTypeMatchFailure(loc); } @@ -11920,6 +11922,7 @@ bool ConstraintSystem::resolveClosure(TypeVariableType *typeVar, auto result = applyPropertyWrapperToParameter(backingType, param.getParameterType(), paramDecl, paramDecl->getName(), ConstraintKind::Equal, + getConstraintLocator(closure), getConstraintLocator(closure)); if (result.isFailure()) return false; diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp index d75bbb3ccd08e..58088b1b5216f 100644 --- a/lib/Sema/TypeOfReference.cpp +++ b/lib/Sema/TypeOfReference.cpp @@ -741,14 +741,15 @@ unwrapPropertyWrapperParameterTypes(ConstraintSystem &cs, AbstractFunctionDecl * continue; } - auto *wrappedType = cs.createTypeVariable(cs.getConstraintLocator(locator), 0); + auto *loc = cs.getConstraintLocator(locator); + auto *wrappedType = cs.createTypeVariable(loc, 0); auto paramType = paramTypes[i].getParameterType(); auto paramLabel = paramTypes[i].getLabel(); auto paramInternalLabel = paramTypes[i].getInternalLabel(); adjustedParamTypes.push_back(AnyFunctionType::Param( wrappedType, paramLabel, ParameterTypeFlags(), paramInternalLabel)); cs.applyPropertyWrapperToParameter(paramType, wrappedType, paramDecl, argLabel, - ConstraintKind::Equal, locator); + ConstraintKind::Equal, loc, loc); } return FunctionType::get(adjustedParamTypes, functionType->getResult(), diff --git a/test/Sema/property_wrapper_parameter.swift b/test/Sema/property_wrapper_parameter.swift index 250b354ec20fb..9f69fce58d1a6 100644 --- a/test/Sema/property_wrapper_parameter.swift +++ b/test/Sema/property_wrapper_parameter.swift @@ -189,3 +189,28 @@ func takesWrapperClosure(_: ProjectionWrapper<[S]>, closure: (ProjectionWr func testGenericPropertyWrapper(@ProjectionWrapper wrappers: [S]) { takesWrapperClosure($wrappers) { $wrapper in } } + +@propertyWrapper +struct Binding { + var wrappedValue: Value + + init(wrappedValue: Value) { + self.wrappedValue = wrappedValue + } + + public var projectedValue: Binding { + return self + } + + public init(projectedValue: Binding) { + self = projectedValue + } +} + +struct Widget { + init(@ProjectionWrapper w: Int) {} +} + +func buildWidget(_ w: ProjectionWrapper) -> Widget { + Widget($w: w) +} From 7e0e0ece9cb1d7211a77d30c1c87cf28160d01cb Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Fri, 22 Nov 2024 13:58:41 -0800 Subject: [PATCH 3/3] [CSApply] Avoid shortcutting argument conversion when parameter has an external property wrapper The check to see whether argument matches the parameter exactly causes two problems: prevents projected value initialized injection; and, if there are multiple parameters with property wrappers, would apply incorrect wrapper to other locations because the wrapper application index wasn't incremented. Resolves: rdar://140282980 --- CHANGELOG.md | 23 +++++++++++ lib/Sema/CSApply.cpp | 12 +++++- test/SILGen/property_wrapper_parameter.swift | 42 ++++++++++++++++++++ 3 files changed, 75 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66609da1bea4f..978cc0d647182 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,29 @@ ## Swift 6.1 +* Projected value initializers are now correctly injected into calls when + an argument exactly matches a parameter with an external property wrapper. + + For example: + + ```swift + struct Binding { + ... + init(projectedValue: Self) { ... } + } + + func checkValue(@Binding value: Int) {} + + func use(v: Binding) { + checkValue($value: v) + // Transformed into: `checkValue(value: Binding(projectedValue: v))` + } + ``` + + Previous versions of the Swift compiler incorrectly omitted projected value + initializer injection in the call to `checkValue` because the argument type + matched the parameter type exactly. + * [SE-0444][]: When the upcoming feature `MemberImportVisibility` is enabled, Swift will require that a module be directly imported in a source file when resolving diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 6190486831bbf..941a2d7867c76 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -6297,9 +6297,17 @@ ArgumentList *ExprRewriter::coerceCallArguments( // `sending` parameter etc. applyFlagsToArgument(paramIdx, argExpr); - // If the types exactly match, this is easy. + auto canShortcutConversion = [&](Type argType, Type paramType) { + if (shouldInjectWrappedValuePlaceholder || + paramInfo.hasExternalPropertyWrapper(paramIdx)) + return false; + + return argType->isEqual(paramType); + }; + auto paramType = param.getOldType(); - if (argType->isEqual(paramType) && !shouldInjectWrappedValuePlaceholder) { + + if (canShortcutConversion(argType, paramType)) { newArgs.push_back(arg); continue; } diff --git a/test/SILGen/property_wrapper_parameter.swift b/test/SILGen/property_wrapper_parameter.swift index 23ff87673835d..ccda9e29d65e8 100644 --- a/test/SILGen/property_wrapper_parameter.swift +++ b/test/SILGen/property_wrapper_parameter.swift @@ -537,3 +537,45 @@ func testCaptures(@ClassWrapper ref: Int, @Wrapper value: Int) { // closure #2 in closure #2 in implicit closure #2 in testCaptures(ref:value:) // CHECK-LABEL: sil private [ossa] @$s26property_wrapper_parameter12testCaptures3ref5valueySi_AA7WrapperVySiGtFyAA010ProjectionH0VySiGcfu0_yAJcfU1_AJycfU0_ : $@convention(thin) (ProjectionWrapper) -> ProjectionWrapper } + +do { + @propertyWrapper + struct Binding { + var wrappedValue: Value { + get { fatalError() } + nonmutating set { } + } + + var projectedValue: Self { self } + + init(projectedValue: Self) { self = projectedValue } + } + + final class Value { + enum Kind { + } + + var kind: Binding { + fatalError() + } + } + + struct Test { + var value: Value + + // CHECK-LABEL: sil private [ossa] @$s26property_wrapper_parameter4TestL_V4test5otheryAA7BindingL_VyAA5ValueL_C4KindOG_tF : $@convention(method) (Binding, @guaranteed Test) -> () + // CHECK: [[CHECK_PROJECTED_VALUE_INIT_1:%.*]] = function_ref @$s26property_wrapper_parameter4TestL_V9checkKind4kindyAA7BindingL_VyAA5ValueL_C0F0OG_tFAEL_AKvpfW + // CHECK-NEXT: {{.*}} = apply [[CHECK_PROJECTED_VALUE_INIT_1]]({{.*}}) : $@convention(thin) (Binding) -> Binding + // CHECK: [[CHECK_PROJECTED_VALUE_INIT_A:%.*]] = function_ref @$s26property_wrapper_parameter4TestL_V15doubleCheckKind1a1byAA7BindingL_VyAA5ValueL_C0G0OG_AMtFAEL_ALvpfW + // CHECK-NEXT: {{.*}} = apply [[CHECK_PROJECTED_VALUE_INIT_A]]({{.*}}) : $@convention(thin) (Binding) -> Binding + // CHECK: [[CHECK_PROJECTED_VALUE_INIT_B:%.*]] = function_ref @$s26property_wrapper_parameter4TestL_V15doubleCheckKind1a1byAA7BindingL_VyAA5ValueL_C0G0OG_AMtFAFL_ALvpfW + // CHECK-NEXT: {{.*}} = apply [[CHECK_PROJECTED_VALUE_INIT_B]]({{.*}}) : $@convention(thin) (Binding) -> Binding + func test(other: Binding) { + checkKind($kind: value.kind) // Ok + doubleCheckKind($a: value.kind, $b: other) // Ok + } + + func checkKind(@Binding kind: Value.Kind) {} + func doubleCheckKind(@Binding a: Value.Kind, @Binding b: Value.Kind) {} + } +}