From 38a9ef71cb89d05c0fe0dc15e982383932b8fc11 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Fri, 15 Nov 2024 15:34:38 -0800 Subject: [PATCH 1/4] [NFC] Fixed function name spelling. --- lib/SILGen/SILGenLValue.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp index 6e5388c972664..9a260020b83b3 100644 --- a/lib/SILGen/SILGenLValue.cpp +++ b/lib/SILGen/SILGenLValue.cpp @@ -3889,7 +3889,7 @@ static bool isCurrentFunctionAccessor(SILGenFunction &SGF, contextAccessorDecl->getAccessorKind() == accessorKind; } -static bool isSynthesizedDefaultImplementionatThunk(SILGenFunction &SGF) { +static bool isSynthesizedDefaultImplementionThunk(SILGenFunction &SGF) { if (!SGF.FunctionDC) return false; @@ -3932,7 +3932,7 @@ LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e, AccessStrategy strategy = var->getAccessStrategy( accessSemantics, getFormalAccessKind(accessKind), SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(), - /*useOldABI=*/isSynthesizedDefaultImplementionatThunk(SGF)); + /*useOldABI=*/isSynthesizedDefaultImplementionThunk(SGF)); bool isOnSelfParameter = isCallToSelfOfCurrentFunction(SGF, e); @@ -4141,7 +4141,7 @@ LValue SILGenLValue::visitSubscriptExpr(SubscriptExpr *e, auto strategy = decl->getAccessStrategy( accessSemantics, getFormalAccessKind(accessKind), SGF.SGM.M.getSwiftModule(), SGF.F.getResilienceExpansion(), - /*useOldABI=*/isSynthesizedDefaultImplementionatThunk(SGF)); + /*useOldABI=*/isSynthesizedDefaultImplementionThunk(SGF)); bool isOnSelfParameter = isCallToSelfOfCurrentFunction(SGF, e); bool isContextRead = isCurrentFunctionAccessor(SGF, AccessorKind::Read); From 8eaae59c3b4514acc8bbf4c4cb5e8d1cc90573bb Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Tue, 19 Nov 2024 07:43:03 -0800 Subject: [PATCH 2/4] [NFC] AST: Moved impl from Sema. Now that the function is declared in AST, it should be implemented there. --- lib/AST/Decl.cpp | 28 ++++++++++++++++++++++++++++ lib/Sema/TypeCheckStorage.cpp | 28 ---------------------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 7a48328d0434c..0a8f02af0ae75 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -10540,6 +10540,34 @@ ArrayRef AccessorDecl::getAccessedProperties() const { return {}; } +bool AccessorDecl::doesAccessorHaveBody() const { + auto *accessor = this; + auto *storage = accessor->getStorage(); + + if (isa(accessor->getDeclContext())) { + if (!accessor->getASTContext().LangOpts.hasFeature( + Feature::CoroutineAccessors)) { + return false; + } + if (!requiresFeatureCoroutineAccessors(accessor->getAccessorKind())) { + return false; + } + if (storage->getOverrideLoc()) { + return false; + } + return accessor->getStorage() + ->requiresCorrespondingUnderscoredCoroutineAccessor( + accessor->getAccessorKind(), accessor); + } + + // NSManaged getters and setters don't have bodies. + if (storage->getAttrs().hasAttribute(/*AllowInvalid=*/true)) + if (accessor->isGetterOrSetter()) + return false; + + return true; +} + StaticSpellingKind FuncDecl::getCorrectStaticSpelling() const { assert(getDeclContext()->isTypeContext()); if (!isStatic()) diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 3d27bc4b2d2b2..1859bd652f5c4 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -938,34 +938,6 @@ buildIndexForwardingParamList(AbstractStorageDecl *storage, return ParameterList::create(context, elements); } -bool AccessorDecl::doesAccessorHaveBody() const { - auto *accessor = this; - auto *storage = accessor->getStorage(); - - if (isa(accessor->getDeclContext())) { - if (!accessor->getASTContext().LangOpts.hasFeature( - Feature::CoroutineAccessors)) { - return false; - } - if (!requiresFeatureCoroutineAccessors(accessor->getAccessorKind())) { - return false; - } - if (storage->getOverrideLoc()) { - return false; - } - return accessor->getStorage() - ->requiresCorrespondingUnderscoredCoroutineAccessor( - accessor->getAccessorKind(), accessor); - } - - // NSManaged getters and setters don't have bodies. - if (storage->getAttrs().hasAttribute(/*AllowInvalid=*/true)) - if (accessor->isGetterOrSetter()) - return false; - - return true; -} - /// Build an argument list referencing the subscript parameters for this /// subscript accessor. static ArgumentList *buildSubscriptArgumentList(ASTContext &ctx, From 563f932a00bb211bf4074e39c2b5c1ffe46cec02 Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Fri, 15 Nov 2024 17:03:23 -0800 Subject: [PATCH 3/4] [NFC] Extracted predicate. --- include/swift/AST/Decl.h | 5 +++++ lib/AST/Decl.cpp | 31 ++++++++++++++++++------------- 2 files changed, 23 insertions(+), 13 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 0e662968c8db0..f2268b8418425 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -8454,6 +8454,11 @@ class AccessorDecl final : public FuncDecl { /// even when it does not have one _yet_. bool doesAccessorHaveBody() const; + /// Whether this accessor is a protocol requirement for which a default + /// implementation must be provided for back-deployment. For example, read2 + /// and modify2 requirements with early enough availability. + bool isRequirementWithSynthesizedDefaultImplementation() const; + static bool classof(const Decl *D) { return D->getKind() == DeclKind::Accessor; } diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 0a8f02af0ae75..94e5bceefa688 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -10540,24 +10540,29 @@ ArrayRef AccessorDecl::getAccessedProperties() const { return {}; } +bool AccessorDecl::isRequirementWithSynthesizedDefaultImplementation() const { + if (!isa(getDeclContext())) + return false; + + if (!getASTContext().LangOpts.hasFeature(Feature::CoroutineAccessors)) { + return false; + } + if (!requiresFeatureCoroutineAccessors(getAccessorKind())) { + return false; + } + if (getStorage()->getOverrideLoc()) { + return false; + } + return getStorage()->requiresCorrespondingUnderscoredCoroutineAccessor( + getAccessorKind(), this); +} + bool AccessorDecl::doesAccessorHaveBody() const { auto *accessor = this; auto *storage = accessor->getStorage(); if (isa(accessor->getDeclContext())) { - if (!accessor->getASTContext().LangOpts.hasFeature( - Feature::CoroutineAccessors)) { - return false; - } - if (!requiresFeatureCoroutineAccessors(accessor->getAccessorKind())) { - return false; - } - if (storage->getOverrideLoc()) { - return false; - } - return accessor->getStorage() - ->requiresCorrespondingUnderscoredCoroutineAccessor( - accessor->getAccessorKind(), accessor); + return isRequirementWithSynthesizedDefaultImplementation(); } // NSManaged getters and setters don't have bodies. From bdf662a871127119539ba05f86d273e41a778b2b Mon Sep 17 00:00:00 2001 From: Nate Chandler Date: Fri, 15 Nov 2024 16:51:45 -0800 Subject: [PATCH 4/4] [CoroutineAccessors] Default impls are transparent Like every other method entry in the default witness table, the default implementations of the `read2` and `modify2` accessors that just call `read` and `modify` respectively should be transparent. --- lib/Sema/TypeCheckStorage.cpp | 5 ++++ test/SILGen/coroutine_accessors_skip.swift | 33 ++++++++++++++++++++++ test/SILGen/read_requirements.swift | 2 +- 3 files changed, 39 insertions(+), 1 deletion(-) create mode 100644 test/SILGen/coroutine_accessors_skip.swift diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 1859bd652f5c4..69ccc4f4772e4 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -2828,6 +2828,11 @@ IsAccessorTransparentRequest::evaluate(Evaluator &evaluator, if (accessor->getAttrs().hasAttribute()) return true; + // Default implementations of read2 and modify2 provided for back-deployment + // are transparent. + if (accessor->isRequirementWithSynthesizedDefaultImplementation()) + return true; + if (!accessor->isImplicit()) return false; diff --git a/test/SILGen/coroutine_accessors_skip.swift b/test/SILGen/coroutine_accessors_skip.swift new file mode 100644 index 0000000000000..d5717302be901 --- /dev/null +++ b/test/SILGen/coroutine_accessors_skip.swift @@ -0,0 +1,33 @@ +// RUN: %target-swift-emit-silgen \ +// RUN: %s \ +// RUN: -experimental-skip-non-inlinable-function-bodies \ +// RUN: -enable-library-evolution \ +// RUN: -enable-experimental-feature CoroutineAccessors \ +// RUN: | %FileCheck %s --check-prefixes=CHECK,CHECK-NOUNWIND + +// RUN: %target-swift-emit-silgen \ +// RUN: %s \ +// RUN: -experimental-skip-non-inlinable-function-bodies \ +// RUN: -enable-library-evolution \ +// RUN: -enable-experimental-feature CoroutineAccessors \ +// RUN: -enable-experimental-feature CoroutineAccessorsUnwindOnCallerError \ +// RUN: | %FileCheck %s --check-prefixes=CHECK,CHECK-UNWIND + +// REQUIRES: swift_feature_CoroutineAccessors +// REQUIRES: swift_feature_CoroutineAccessorsUnwindOnCallerError + +// CHECK-LABEL: sil_default_witness_table MutatableAssociatedField { +// CHECK-NEXT: no_default +// CHECK-NEXT: no_default +// CHECK-NEXT: method #MutatableAssociatedField.field!read2 +// CHECK-SAME: : @$s24coroutine_accessors_skip24MutatableAssociatedFieldP5field5AssocQzvy +// CHECK-NEXT: no_default +// CHECK-NEXT: no_default +// CHECK-NEXT: method #MutatableAssociatedField.field!modify2 +// CHECK-SAME: : @$s24coroutine_accessors_skip24MutatableAssociatedFieldP5field5AssocQzvx +// CHECK-NEXT: } +public protocol MutatableAssociatedField { + associatedtype Assoc + + var field: Assoc { read set } +} diff --git a/test/SILGen/read_requirements.swift b/test/SILGen/read_requirements.swift index 958c102e4b16e..d20b494b71924 100644 --- a/test/SILGen/read_requirements.swift +++ b/test/SILGen/read_requirements.swift @@ -55,7 +55,7 @@ public protocol P1 : ~Copyable { // CHECK: unwind // CHECK-LABEL: } // end sil function '$s17read_requirements2P1P4ubgsAA1UVvy' -// CHECK-LABEL: sil [ossa] @$s17read_requirements2P1P4ubgsAA1UVvx : {{.*}} { +// CHECK-LABEL: sil{{.*}} [ossa] @$s17read_requirements2P1P4ubgsAA1UVvx : {{.*}} { // CHECK: bb0( // CHECK-SAME: [[SELF_UNCHECKED:%[^:]+]] // CHECK-SAME: ):