diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h index c6b12e2e33278..eee1dea674fc4 100644 --- a/include/swift/AST/Types.h +++ b/include/swift/AST/Types.h @@ -1103,6 +1103,12 @@ class alignas(1 << TypeAlignInBits) TypeBase /// Check if this is a std.string type from C++. bool isCxxString(); + /// Check if this type is known to represent key paths. + bool isKnownKeyPathType(); + + /// Check if this type is known to represent immutable key paths. + bool isKnownImmutableKeyPathType(); + /// Check if this is either an Array, Set or Dictionary collection type defined /// at the top level of the Swift module bool isKnownStdlibCollectionType(); diff --git a/include/swift/Sema/ConstraintSystem.h b/include/swift/Sema/ConstraintSystem.h index 1bc1ad5f7f16b..358893257487e 100644 --- a/include/swift/Sema/ConstraintSystem.h +++ b/include/swift/Sema/ConstraintSystem.h @@ -6525,10 +6525,6 @@ class TypeVarRefCollector : public ASTWalker { } }; -/// Determine whether given type is a known one -/// for a key path `{Any, Partial, Writable, ReferenceWritable}KeyPath`. -bool isKnownKeyPathType(Type type); - /// Determine whether the given type is a PartialKeyPath and /// AnyKeyPath or existential type thererof, for example, /// `PartialKeyPath<...> & Sendable`. diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 65a7620d017ca..ba6a64f87c442 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -1309,6 +1309,15 @@ bool TypeBase::isCxxString() { ctx.Id_basic_string.is(clangDecl->getName()); } +bool TypeBase::isKnownKeyPathType() { + return isKeyPath() || isWritableKeyPath() || isReferenceWritableKeyPath() || + isPartialKeyPath() || isAnyKeyPath(); +} + +bool TypeBase::isKnownImmutableKeyPathType() { + return isKeyPath() || isPartialKeyPath() || isAnyKeyPath(); +} + bool TypeBase::isKnownStdlibCollectionType() { if (isArray() || isDictionary() || isSet()) { return true; diff --git a/lib/IDE/SelectedOverloadInfo.cpp b/lib/IDE/SelectedOverloadInfo.cpp index d9a339116fb6c..2fbb1c40c04c9 100644 --- a/lib/IDE/SelectedOverloadInfo.cpp +++ b/lib/IDE/SelectedOverloadInfo.cpp @@ -79,7 +79,7 @@ swift::ide::getSelectedOverloadInfo(const Solution &S, fnType->getParams()[0].getPlainType()->castTo(); auto *keyPathDecl = keyPathTy->getAnyNominal(); - assert(isKnownKeyPathType(keyPathTy) && + assert(keyPathTy->isKnownKeyPathType() && "parameter is supposed to be a keypath"); auto KeyPathDynamicLocator = CS.getConstraintLocator( diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 79376e002ee95..167f25c810899 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -2689,7 +2689,7 @@ namespace { Type keyPathTy = paramType; if (auto *existential = keyPathTy->getAs()) { keyPathTy = existential->getSuperclass(); - assert(isKnownKeyPathType(keyPathTy)); + assert(keyPathTy->isKnownKeyPathType()); } SmallVector components; diff --git a/lib/Sema/CSBindings.cpp b/lib/Sema/CSBindings.cpp index b42bad750a6b7..5c43218521578 100644 --- a/lib/Sema/CSBindings.cpp +++ b/lib/Sema/CSBindings.cpp @@ -513,7 +513,7 @@ void BindingSet::inferTransitiveBindings() { auto bindingTy = binding.BindingType->lookThroughAllOptionalTypes(); Type inferredRootTy; - if (isKnownKeyPathType(bindingTy)) { + if (bindingTy->isKnownKeyPathType()) { // AnyKeyPath doesn't have a root type. if (bindingTy->isAnyKeyPath()) continue; @@ -727,7 +727,8 @@ bool BindingSet::finalize(bool transitive) { for (const auto &binding : Bindings) { auto bindingTy = binding.BindingType->lookThroughAllOptionalTypes(); - assert(isKnownKeyPathType(bindingTy) || bindingTy->is()); + assert(bindingTy->isKnownKeyPathType() || + bindingTy->is()); // Functions don't have capability so we can simply add them. if (auto *fnType = bindingTy->getAs()) { @@ -1694,7 +1695,7 @@ PotentialBindings::inferFromRelational(ConstraintSystem &CS, if (type->isExistentialType()) { auto layout = type->getExistentialLayout(); if (auto superclass = layout.explicitSuperclass) { - if (isKnownKeyPathType(superclass)) { + if (superclass->isKnownKeyPathType()) { type = superclass; objectTy = superclass; } @@ -1702,7 +1703,7 @@ PotentialBindings::inferFromRelational(ConstraintSystem &CS, } } - if (!(isKnownKeyPathType(objectTy) || objectTy->is())) + if (!(objectTy->isKnownKeyPathType() || objectTy->is())) return std::nullopt; } diff --git a/lib/Sema/CSDiagnostics.cpp b/lib/Sema/CSDiagnostics.cpp index c4e76f309168b..ec6ea6d6d5c68 100644 --- a/lib/Sema/CSDiagnostics.cpp +++ b/lib/Sema/CSDiagnostics.cpp @@ -2262,7 +2262,7 @@ bool AssignmentFailure::diagnoseAsError() { auto type = getType(immutableExpr); - if (isKnownKeyPathType(type)) + if (type->isKnownKeyPathType()) message += " is read-only"; else if (VD->isCaptureList()) message += " is an immutable capture"; @@ -7487,7 +7487,7 @@ bool ArgumentMismatchFailure::diagnoseAsError() { // Unresolved key path argument requires a tailored diagnostic // that doesn't mention a fallback type - `KeyPath<_, _>`. - if (argType->isKeyPath() && !isKnownKeyPathType(paramType)) { + if (argType->isKeyPath() && !paramType->isKnownKeyPathType()) { auto keyPathTy = argType->castTo(); auto rootTy = keyPathTy->getGenericArgs()[0]; if (rootTy->is()) { @@ -7852,7 +7852,7 @@ bool ArgumentMismatchFailure::diagnoseKeyPathAsFunctionResultMismatch() const { auto argType = getFromType(); auto paramType = getToType(); - if (!isKnownKeyPathType(argType)) + if (!argType->isKnownKeyPathType()) return false; auto kpType = argType->castTo(); diff --git a/lib/Sema/CSSimplify.cpp b/lib/Sema/CSSimplify.cpp index 0ff46495538d7..fc1bd2a079d8d 100644 --- a/lib/Sema/CSSimplify.cpp +++ b/lib/Sema/CSSimplify.cpp @@ -5598,7 +5598,7 @@ bool ConstraintSystem::repairFailures( // fix-up here unless last component has already a invalid type or // instance fix recorded. if (isExpr(anchor)) { - if (isKnownKeyPathType(lhs) && isKnownKeyPathType(rhs)) { + if (lhs->isKnownKeyPathType() && rhs->isKnownKeyPathType()) { // If we have a conversion happening here, we should let fix happen in // simplifyRestrictedConstraint. if (hasAnyRestriction()) diff --git a/lib/Sema/ConstraintSystem.cpp b/lib/Sema/ConstraintSystem.cpp index e6d462f004f1a..897de2bf547c3 100644 --- a/lib/Sema/ConstraintSystem.cpp +++ b/lib/Sema/ConstraintSystem.cpp @@ -4363,12 +4363,6 @@ Solution::getFunctionArgApplyInfo(ConstraintLocator *locator) const { fnInterfaceType, fnType, callee); } -bool constraints::isKnownKeyPathType(Type type) { - return type->isKeyPath() || type->isWritableKeyPath() || - type->isReferenceWritableKeyPath() || type->isPartialKeyPath() || - type->isAnyKeyPath(); -} - bool constraints::isTypeErasedKeyPathType(Type type) { assert(type); diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index f46a2c25eb65c..c457b765b022f 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -29,6 +29,7 @@ #include "swift/AST/ClangModuleLoader.h" #include "swift/AST/DeclExportabilityVisitor.h" #include "swift/AST/DiagnosticsParse.h" +#include "swift/AST/ExistentialLayout.h" #include "swift/AST/GenericEnvironment.h" #include "swift/AST/Initializer.h" #include "swift/AST/NameLookup.h" @@ -2232,9 +2233,6 @@ class ExprAvailabilityWalker : public BaseDiagnosticWalker { /// The context does not involve a key path access. None, - /// The context is an expression that is coerced to a read-only key path. - ReadOnlyCoercion, - /// The context is a key path application (`x[keyPath: \.member]`). Application, }; @@ -2361,15 +2359,6 @@ class ExprAvailabilityWalker : public BaseDiagnosticWalker { FCE->getLoc(), Where.getDeclContext()); } - if (auto DTBE = dyn_cast(E)) { - if (auto ty = DTBE->getType()) { - if (ty->isKeyPath()) { - walkInKeyPathAccessContext(DTBE->getSubExpr(), - KeyPathAccessContext::ReadOnlyCoercion); - return Action::SkipChildren(E); - } - } - } if (auto KP = dyn_cast(E)) { maybeDiagKeyPath(KP); } @@ -2586,19 +2575,49 @@ class ExprAvailabilityWalker : public BaseDiagnosticWalker { StorageAccessKind getStorageAccessKindForKeyPath(KeyPathExpr *KP) const { switch (KeyPathAccess) { - case KeyPathAccessContext::None: - // Use the key path's type to determine the access kind. - if (!KP->isObjC()) - if (auto keyPathType = KP->getKeyPathType()) - if (keyPathType->isKeyPath()) + case KeyPathAccessContext::None: { + // Obj-C key paths are always considered mutable. + if (KP->isObjC()) + return StorageAccessKind::GetSet; + + // Check whether key path type indicates immutability, in which case only + // the getter will be used. + if (auto keyPathType = KP->getKeyPathType()) + if (keyPathType->isKnownImmutableKeyPathType()) + return StorageAccessKind::Get; + + // Check if there's a conversion expression containing this one directly + // above it in the stack. If so, and that conversion is to an immutable + // key path type, then we should infer that use of the key path only + // implies use of the getter. + ArrayRef exprs = ExprStack; + // Must be called while visiting the given key path expression. + ASSERT(exprs.back() == KP); + + for (auto *expr : llvm::reverse(exprs.drop_back())) { + // Only traverse through conversions on the stack. + if (!isa(expr) && + !isa(expr)) + break; + + if (auto ty = expr->getType()) { + if (ty->isKnownImmutableKeyPathType()) return StorageAccessKind::Get; - return StorageAccessKind::GetSet; + if (auto existential = dyn_cast(ty)) { + if (auto superclass = + existential->getExistentialLayout().getSuperclass()) { + if (superclass->isKnownImmutableKeyPathType()) + return StorageAccessKind::Get; + } + } + } + } - case KeyPathAccessContext::ReadOnlyCoercion: - // The type of this key path is being coerced to the type KeyPath<_, _> so - // ignore the actual key path type. - return StorageAccessKind::Get; + // We failed to prove that the key path is only used immutably, + // so diagnose both get and set access. + return StorageAccessKind::GetSet; + } case KeyPathAccessContext::Application: // The key path is being applied directly to a base so treat it as if it diff --git a/lib/Sema/TypeOfReference.cpp b/lib/Sema/TypeOfReference.cpp index 52679cb498eee..b92a6e8f0cf97 100644 --- a/lib/Sema/TypeOfReference.cpp +++ b/lib/Sema/TypeOfReference.cpp @@ -2392,13 +2392,13 @@ void ConstraintSystem::bindOverloadType(const SelectedOverload &overload, if (auto *existential = paramTy->getAs()) { paramTy = existential->getSuperclass(); - assert(isKnownKeyPathType(paramTy)); + assert(paramTy->isKnownKeyPathType()); } auto keyPathTy = paramTy->castTo(); auto *keyPathDecl = keyPathTy->getAnyNominal(); - assert(isKnownKeyPathType(keyPathTy) && + assert(keyPathTy->isKnownKeyPathType() && "parameter is supposed to be a keypath"); auto *keyPathLoc = getConstraintLocator( diff --git a/test/Availability/availability_accessors.swift b/test/Availability/availability_accessors.swift index 578b92de6dacf..37d29f619b544 100644 --- a/test/Availability/availability_accessors.swift +++ b/test/Availability/availability_accessors.swift @@ -56,7 +56,7 @@ struct BaseStruct { var unavailableGetter: T { @available(*, unavailable) - get { fatalError() } // expected-note 69 {{getter for 'unavailableGetter' has been explicitly marked unavailable here}} + get { fatalError() } // expected-note 73 {{getter for 'unavailableGetter' has been explicitly marked unavailable here}} set { } } @@ -68,7 +68,7 @@ struct BaseStruct { var unavailableGetterAndSetter: T { @available(*, unavailable) - get { fatalError() } // expected-note 68 {{getter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}} + get { fatalError() } // expected-note 71 {{getter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}} @available(*, unavailable) set { fatalError() } // expected-note 33 {{setter for 'unavailableGetterAndSetter' has been explicitly marked unavailable here}} } @@ -116,11 +116,6 @@ struct SubscriptHelper { return t } -func takesKeyPath(_ t: T, _ keyPath: KeyPath) -> () { } -func takesWritableKeyPath(_ t: inout T, _ keyPath: WritableKeyPath) -> () { - // expected-note@-1 2 {{in call to function 'takesWritableKeyPath'}} -} - func testDiscardedRValueLoads_Struct() { var x = BaseStruct() // expected-warning {{variable 'x' was never mutated; consider changing to 'let' constant}} @@ -548,6 +543,8 @@ func testDiscardedApplyOfFuncWithInOutParam_Class() { func testKeyPathArguments_Struct() { var x = BaseStruct() + func takesKeyPath(_ t: T, _ keyPath: KeyPath) -> () { } + takesKeyPath(x, \.available) takesKeyPath(x, \.unavailableGetter) // expected-error {{getter for 'unavailableGetter' is unavailable}} takesKeyPath(x, \.unavailableSetter) @@ -555,6 +552,35 @@ func testKeyPathArguments_Struct() { takesKeyPath(x, \.deprecatedGetter) // expected-warning {{getter for 'deprecatedGetter' is deprecated: reading not recommended}} takesKeyPath(x, \.deprecatedSetter) + func takesAnyKeyPath(_ keyPath: AnyKeyPath) -> () { } + + takesAnyKeyPath(\BaseStruct.available) + takesAnyKeyPath(\BaseStruct.unavailableGetter) // expected-error {{getter for 'unavailableGetter' is unavailable}} + takesAnyKeyPath(\BaseStruct.unavailableSetter) + takesAnyKeyPath(\BaseStruct.unavailableGetterAndSetter) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} + takesAnyKeyPath(\BaseStruct.deprecatedGetter) // expected-warning {{getter for 'deprecatedGetter' is deprecated: reading not recommended}} + takesAnyKeyPath(\BaseStruct.deprecatedSetter) + + func takesPartialKeyPath(_ t: T, _ keyPath: PartialKeyPath) -> () { } + takesPartialKeyPath(x, \.available) + takesPartialKeyPath(x, \.unavailableGetter) // expected-error {{getter for 'unavailableGetter' is unavailable}} + takesPartialKeyPath(x, \.unavailableSetter) + takesPartialKeyPath(x, \.unavailableGetterAndSetter) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} + takesPartialKeyPath(x, \.deprecatedGetter) // expected-warning {{getter for 'deprecatedGetter' is deprecated: reading not recommended}} + takesPartialKeyPath(x, \.deprecatedSetter) + + func takesSendableKeyPath(_ t: T, _ keyPath: KeyPath & Sendable) -> () { } + + takesSendableKeyPath(x, \.available) + takesSendableKeyPath(x, \.unavailableGetter) // expected-error {{getter for 'unavailableGetter' is unavailable}} + takesSendableKeyPath(x, \.unavailableSetter) + takesSendableKeyPath(x, \.unavailableGetterAndSetter) // expected-error {{getter for 'unavailableGetterAndSetter' is unavailable}} + takesSendableKeyPath(x, \.deprecatedGetter) // expected-warning {{getter for 'deprecatedGetter' is deprecated: reading not recommended}} + takesSendableKeyPath(x, \.deprecatedSetter) + + func takesWritableKeyPath(_ t: inout T, _ keyPath: WritableKeyPath) -> () { } + // expected-note@-1 2 {{in call to function 'takesWritableKeyPath'}} + takesWritableKeyPath(&x, \.available) takesWritableKeyPath(&x, \.unavailableGetter) // expected-error {{getter for 'unavailableGetter' is unavailable}} // FIXME: Ideally we would diagnose the unavailability of the setter instead @@ -566,6 +592,14 @@ func testKeyPathArguments_Struct() { // expected-error@-1 {{generic parameter 'U' could not be inferred}} takesWritableKeyPath(&x, \.deprecatedGetter) // expected-warning {{getter for 'deprecatedGetter' is deprecated: reading not recommended}} takesWritableKeyPath(&x, \.deprecatedSetter) // expected-warning {{setter for 'deprecatedSetter' is deprecated: writing not recommended}} + + func takesUnifiedTypes(_: T, _: T) { } + + takesUnifiedTypes(\BaseStruct.available, \BaseStruct.available) + takesUnifiedTypes(\BaseStruct.available, \BaseStruct.unavailableGetter) // expected-error {{getter for 'unavailableGetter' is unavailable}} + takesUnifiedTypes(\BaseStruct.available, \BaseStruct.unavailableSetter) + takesUnifiedTypes(\BaseStruct.available, \BaseStruct.deprecatedSetter) // expected-warning {{setter for 'deprecatedSetter' is deprecated: writing not recommended}} + takesUnifiedTypes(\BaseStruct.unavailableSetter, \BaseStruct.deprecatedSetter) } var global = BaseStruct()