From 020db2350b6211a90afd0cae1597256434de1573 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 22 Dec 2022 17:15:34 -0800 Subject: [PATCH 1/5] [AST] TypeWrappers: Always allow attributes that come from swiftinterfaces Since type wrapper attributes could be applied to protocols and the protocol types could be used as existential values, it's not always possible to wrap all uses of feature into `#if $TypeWrappers` block so let's allow use of attributes if they come from a Swift interface file. --- include/swift/AST/TypeCheckRequests.h | 17 ------- include/swift/AST/TypeCheckerTypeIDZone.def | 3 -- lib/AST/ASTPrinter.cpp | 14 +----- lib/AST/TypeWrapper.cpp | 50 --------------------- lib/Sema/TypeCheckAttr.cpp | 11 ++++- 5 files changed, 11 insertions(+), 84 deletions(-) diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index ae79861f0a3bf..965b99dfadd79 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -3762,23 +3762,6 @@ class GetTypeWrapperInitializer bool isCached() const { return true; } }; -/// Check whether this is a protocol that has a type wrapper attribute -/// or one of its dependencies does. -class UsesTypeWrapperFeature - : public SimpleRequest { -public: - using SimpleRequest::SimpleRequest; - -private: - friend SimpleRequest; - - bool evaluate(Evaluator &evaluator, NominalTypeDecl *) const; - -public: - bool isCached() const { return true; } -}; - /// Find the definition of a given macro. class MacroDefinitionRequest : public SimpleRequest typeDecl; - - if (auto *extension = dyn_cast(decl)) { - typeDecl = extension->getExtendedNominal(); - } else { - typeDecl = dyn_cast(decl); - } - - if (!typeDecl) - return false; - - return evaluateOrDefault(decl->getASTContext().evaluator, - UsesTypeWrapperFeature{typeDecl.get()}, false); + return false; } static bool usesFeatureRuntimeDiscoverableAttrs(Decl *decl) { diff --git a/lib/AST/TypeWrapper.cpp b/lib/AST/TypeWrapper.cpp index d22c670825f49..a3dd1349f2051 100644 --- a/lib/AST/TypeWrapper.cpp +++ b/lib/AST/TypeWrapper.cpp @@ -44,53 +44,3 @@ bool VarDecl::isTypeWrapperLocalStorageForInitializer() const { } return false; } - -bool UsesTypeWrapperFeature::evaluate(Evaluator &evaluator, - NominalTypeDecl *decl) const { - // This is a type wrapper type. - if (decl->getAttrs().hasAttribute()) - return true; - - // This is a type wrapped type. - if (decl->hasTypeWrapper()) - return true; - - // This type could be depending on a type wrapper feature - // indirectly by conforming to a protocol with a type - // wrapper attribute. To determine that we need to walk - // protocol dependency chains and check each one. - - auto &ctx = decl->getASTContext(); - - auto usesTypeWrapperFeature = [&](ProtocolDecl *protocol) { - return evaluateOrDefault(ctx.evaluator, UsesTypeWrapperFeature{protocol}, - false); - }; - - for (unsigned i : indices(decl->getInherited())) { - auto inheritedType = evaluateOrDefault( - ctx.evaluator, - InheritedTypeRequest{decl, i, TypeResolutionStage::Interface}, Type()); - - if (!(inheritedType && inheritedType->isConstraintType())) - continue; - - if (auto *protocol = - dyn_cast_or_null(inheritedType->getAnyNominal())) { - if (usesTypeWrapperFeature(protocol)) - return true; - } - - if (auto composition = inheritedType->getAs()) { - for (auto member : composition->getMembers()) { - if (auto *protocol = - dyn_cast_or_null(member->getAnyNominal())) { - if (usesTypeWrapperFeature(protocol)) - return true; - } - } - } - } - - return false; -} diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 9bf260fdb5466..0c9798da0194a 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -3826,7 +3826,16 @@ void AttributeChecker::visitPropertyWrapperAttr(PropertyWrapperAttr *attr) { } void AttributeChecker::visitTypeWrapperAttr(TypeWrapperAttr *attr) { - if (!Ctx.LangOpts.hasFeature(Feature::TypeWrappers)) { + auto isEnabled = [&]() { + if (Ctx.LangOpts.hasFeature(Feature::TypeWrappers)) + return true; + + // Accept attributes that come from swiftinterface files. + auto *parentSF = D->getDeclContext()->getParentSourceFile(); + return parentSF && parentSF->Kind == SourceFileKind::Interface; + }; + + if (!isEnabled()) { diagnose(attr->getLocation(), diag::type_wrappers_are_experimental); attr->setInvalid(); return; From ed2e07e0a17d26eb6f614a803195cf0047d46536 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 22 Dec 2022 17:19:21 -0800 Subject: [PATCH 2/5] [Sema] TypeWrappers: If type wrapper comes from `.swiftinterface` use synthesized decls Swift interfaces have both attribute and all publicly accessible synthesized members, which means that the synthesis needs to be taught to lookup existing synthesized declarations for `$storage`, `$Storage`, and `init(storageWrapper:)` if request comes for declaration that belongs to a Swift interface file. --- lib/Sema/CodeSynthesis.cpp | 17 ++++++++++++++++- lib/Sema/TypeCheckTypeWrapper.cpp | 29 +++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index ad3f2b50aa777..f6361021fa978 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -1657,6 +1657,22 @@ ConstructorDecl *SynthesizeTypeWrappedTypeStorageWrapperInitializer::evaluate( if (!wrappedType->hasTypeWrapper()) return nullptr; + auto &ctx = wrappedType->getASTContext(); + + // .swiftinterfaces have both attribute and a synthesized member + // (if it's public), so in this case we need use existing declaration + // if available. + { + auto parentSF = wrappedType->getDeclContext()->getParentSourceFile(); + if (parentSF && parentSF->Kind == SourceFileKind::Interface) { + DeclName initName(ctx, DeclBaseName::createConstructor(), + /*labels=*/{ctx.Id_storageWrapper}); + auto results = wrappedType->lookupDirect(initName); + if (results.size() == 1) + return cast(results.front()); + } + } + // `@typeWrapperIgnored` properties suppress this initializer. if (llvm::any_of(wrappedType->getMembers(), [&](Decl *member) { return member->getAttrs().hasAttribute(); @@ -1664,7 +1680,6 @@ ConstructorDecl *SynthesizeTypeWrappedTypeStorageWrapperInitializer::evaluate( return nullptr; // Create the implicit type wrapper storage constructor. - auto &ctx = wrappedType->getASTContext(); auto ctor = createImplicitConstructor( wrappedType, ImplicitConstructorKind::TypeWrapperStorage, ctx); wrappedType->addMember(ctor); diff --git a/lib/Sema/TypeCheckTypeWrapper.cpp b/lib/Sema/TypeCheckTypeWrapper.cpp index febbafd42054b..b7b96387afb7f 100644 --- a/lib/Sema/TypeCheckTypeWrapper.cpp +++ b/lib/Sema/TypeCheckTypeWrapper.cpp @@ -26,6 +26,17 @@ using namespace swift; +/// Check whether given declaration comes from the .swiftinterface file. +static bool inSwiftInterfaceContext(NominalTypeDecl *typeDecl) { + auto *SF = typeDecl->getDeclContext()->getParentSourceFile(); + return SF && SF->Kind == SourceFileKind::Interface; +} + +static ValueDecl *findMember(NominalTypeDecl *typeDecl, Identifier memberName) { + auto members = typeDecl->lookupDirect(memberName); + return members.size() == 1 ? members.front() : nullptr; +} + static PatternBindingDecl *injectVariable(DeclContext *DC, Identifier name, Type type, VarDecl::Introducer introducer, @@ -284,6 +295,15 @@ TypeDecl *GetTypeWrapperStorage::evaluate(Evaluator &evaluator, auto &ctx = parent->getASTContext(); + // .swiftinterfaces have both attribute and a synthesized member + // (if it's public), so in this case we need use existing declaration + // if available. + if (inSwiftInterfaceContext(parent)) { + if (auto *storage = dyn_cast_or_null( + findMember(parent, ctx.Id_TypeWrapperStorage))) + return storage; + } + TypeDecl *storage = nullptr; if (isa(parent)) { // If type wrapper is associated with a protocol, we need to @@ -320,6 +340,15 @@ GetTypeWrapperProperty::evaluate(Evaluator &evaluator, if (!typeWrapper) return nullptr; + // .swiftinterfaces have both attribute and a synthesized member + // (if it's public), so in this case we need use existing declaration + // if available. + if (inSwiftInterfaceContext(parent)) { + if (auto *storage = dyn_cast_or_null( + findMember(parent, ctx.Id_TypeWrapperProperty))) + return storage; + } + auto *storage = parent->getTypeWrapperStorageDecl(); assert(storage); From 0987eff8282b285fd39b457215665267dd1d56f6 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Thu, 22 Dec 2022 17:23:51 -0800 Subject: [PATCH 3/5] [Sema] TypeWrappers: Availability of `init(storageWrapper:)` should match that of `$Storage` --- lib/Sema/CodeSynthesis.cpp | 2 +- test/ModuleInterface/type_wrappers.swift | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index f6361021fa978..55bfc480795d8 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -362,7 +362,7 @@ static ConstructorDecl *createImplicitConstructor(NominalTypeDecl *decl, params.push_back(arg); } } else if (ICK == ImplicitConstructorKind::TypeWrapperStorage) { - accessLevel = AccessLevel::Public; + accessLevel = decl->getTypeWrapperStorageDecl()->getFormalAccess(); auto typeWrapperInfo = decl->getTypeWrapper(); assert(typeWrapperInfo); diff --git a/test/ModuleInterface/type_wrappers.swift b/test/ModuleInterface/type_wrappers.swift index f374fdde9cc5b..1af1dfe260105 100644 --- a/test/ModuleInterface/type_wrappers.swift +++ b/test/ModuleInterface/type_wrappers.swift @@ -32,7 +32,6 @@ public struct Wrapper { // CHECK: @TypeWrappers.Wrapper public class Test where T : Swift.StringProtocol { // CHECK: public init(a: Swift.Int, b: [T]) -// CHECK: public init(storageWrapper: TypeWrappers.Wrapper, TypeWrappers.Test.$Storage>) // CHECK: } @Wrapper From 740ae8d4e7a3ec44d0db8b33606d551f32b44bbd Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 4 Jan 2023 09:29:11 -0800 Subject: [PATCH 4/5] [Tests] TypeWrappers/NFC: Add a tests to round-trip type wrappers in swift interfaces --- .../type_wrapper_in_swiftinterface.swift | 93 +++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 test/Serialization/type_wrapper_in_swiftinterface.swift diff --git a/test/Serialization/type_wrapper_in_swiftinterface.swift b/test/Serialization/type_wrapper_in_swiftinterface.swift new file mode 100644 index 0000000000000..b9e3ef577d7fa --- /dev/null +++ b/test/Serialization/type_wrapper_in_swiftinterface.swift @@ -0,0 +1,93 @@ +// RUN: %empty-directory(%t/src) +// RUN: %empty-directory(%t/sdk) +// RUN: split-file %s %t/src + +/// Build the library. +// RUN: %target-swift-frontend -emit-module %t/src/PublicModule.swift \ +// RUN: -module-name PublicModule -swift-version 5 -enable-library-evolution \ +// RUN: -emit-module-path %t/PublicModule.swiftmodule \ +// RUN: -emit-module-interface-path %t/PublicModule.swiftinterface \ +// RUN: -emit-private-module-interface-path %t/PublicModule.private.swiftinterface \ +// RUN: -enable-experimental-feature TypeWrappers + +/// Verify swiftinterfaces. +// RUN: %target-swift-typecheck-module-from-interface(%t/PublicModule.swiftinterface) -module-name PublicModule +// RUN: %target-swift-typecheck-module-from-interface(%t/PublicModule.private.swiftinterface) -module-name PublicModule + +/// Test the client against the binary swiftmodule. +// RUN: %target-swift-frontend -typecheck %t/src/Client.swift \ +// RUN: -enable-experimental-feature TypeWrappers \ +// RUN: -module-name Client -I %t + +/// Test the client against the private swiftinterface. +// RUN: rm %t/PublicModule.swiftmodule +// RUN: %target-swift-frontend -typecheck %t/src/Client.swift \ +// RUN: -module-name Client -I %t \ +// RUN: -enable-experimental-feature TypeWrappers + +/// Test the client against the public swiftinterface. +// RUN: rm %t/PublicModule.private.swiftinterface +// RUN: %target-swift-frontend -typecheck %t/src/Client.swift \ +// RUN: -module-name Client -I %t \ +// RUN: -enable-experimental-feature TypeWrappers + +//--- PublicModule.swift +@typeWrapper +public struct Wrapper { + public init(for: W.Type, storage: S) {} + + public subscript(propertyKeyPath _: KeyPath, storageKeyPath path: KeyPath) -> V { + get { fatalError() } + } + + public subscript(propertyKeyPath _: KeyPath, storageKeyPath path: WritableKeyPath) -> V { + get { fatalError() } + set { } + } +} + +@Wrapper +public protocol Wrapped {} + +public struct MyWrappedType : Wrapped { + var x: Int = 0 + + public init() {} +} + +@propertyWrapper +public struct PropWrapper { + typealias Wrapped = T + + public var wrappedValue: T { + get { fatalError() } + set { value = newValue } + } + + var value: T? + + public init() {} + + public init(wrappedValue: T) { + self.value = wrappedValue + } +} + +//--- Client.swift + +import PublicModule + +final class S : Wrapped { + @PropWrapper var x: Int + + required init() {} + + required init(x: Int) { + self.x = x + } +} + +let s = S() +_ = s.$storage + +_ = MyWrappedType() From 4f84d8ac48d0db3922604ffee13fabdf8915d425 Mon Sep 17 00:00:00 2001 From: Pavel Yaskevich Date: Wed, 4 Jan 2023 16:45:03 -0800 Subject: [PATCH 5/5] [Sema] TypeWrappers: Request type wrapper attribute type eagerly The type is needed for serialization so it has to be requested and chached in type wrapper info. --- include/swift/AST/TypeCheckRequests.h | 17 ---------- include/swift/AST/TypeCheckerTypeIDZone.def | 3 -- include/swift/AST/TypeWrappers.h | 9 ++--- lib/Sema/TypeCheckTypeWrapper.cpp | 37 ++++++++------------- 4 files changed, 18 insertions(+), 48 deletions(-) diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 965b99dfadd79..b1ab9149fd117 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -3546,23 +3546,6 @@ class GetTypeWrapper bool isCached() const { return true; } }; -/// Return a type of the type wrapper (if any) associated with the given -/// declaration. -class GetTypeWrapperType - : public SimpleRequest { -public: - using SimpleRequest::SimpleRequest; - -private: - friend SimpleRequest; - - Type evaluate(Evaluator &evaluator, NominalTypeDecl *) const; - -public: - bool isCached() const { return true; } -}; - /// Inject or get `$Storage` type which has all of the stored properties /// of the given type with a type wrapper. class GetTypeWrapperStorage diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 222b9a5e56c8d..2fab93590d5b7 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -404,9 +404,6 @@ SWIFT_REQUEST(TypeChecker, GetSourceFileAsyncNode, SWIFT_REQUEST(TypeChecker, GetTypeWrapper, Optional(NominalTypeDecl *), Cached, NoLocationInfo) -SWIFT_REQUEST(TypeChecker, GetTypeWrapperType, - Type(NominalTypeDecl *), - Cached, NoLocationInfo) SWIFT_REQUEST(TypeChecker, GetTypeWrapperStorage, TypeDecl *(NominalTypeDecl *), Cached, NoLocationInfo) diff --git a/include/swift/AST/TypeWrappers.h b/include/swift/AST/TypeWrappers.h index 6127c8cff21a0..03a11ef38c92d 100644 --- a/include/swift/AST/TypeWrappers.h +++ b/include/swift/AST/TypeWrappers.h @@ -21,17 +21,18 @@ namespace swift { struct TypeWrapperInfo { CustomAttr *Attr; + Type AttrType; NominalTypeDecl *Wrapper; NominalTypeDecl *AttachedTo; bool IsInferred; - TypeWrapperInfo(CustomAttr *attr, NominalTypeDecl *wrapperDecl, + TypeWrapperInfo(CustomAttr *attr, Type attrType, NominalTypeDecl *wrapperDecl, NominalTypeDecl *attachedTo, bool isInferred) - : Attr(attr), Wrapper(wrapperDecl), AttachedTo(attachedTo), - IsInferred(isInferred) {} + : Attr(attr), AttrType(attrType), Wrapper(wrapperDecl), + AttachedTo(attachedTo), IsInferred(isInferred) {} TypeWrapperInfo asInferred() const { - return {Attr, Wrapper, AttachedTo, true}; + return {Attr, AttrType, Wrapper, AttachedTo, true}; } }; diff --git a/lib/Sema/TypeCheckTypeWrapper.cpp b/lib/Sema/TypeCheckTypeWrapper.cpp index b7b96387afb7f..323ee68afee43 100644 --- a/lib/Sema/TypeCheckTypeWrapper.cpp +++ b/lib/Sema/TypeCheckTypeWrapper.cpp @@ -130,9 +130,19 @@ static void getTypeWrappers(NominalTypeDecl *decl, continue; auto *typeWrapper = nominal->getAttrs().getAttribute(); - if (typeWrapper && typeWrapper->isValid()) + if (typeWrapper && typeWrapper->isValid()) { + auto attrType = evaluateOrDefault( + ctx.evaluator, + CustomAttrTypeRequest{mutableAttr, decl, + CustomAttrTypeKind::TypeWrapper}, + Type()); + + if (!attrType || attrType->hasError()) + continue; + typeWrappers.push_back( - {mutableAttr, nominal, decl, /*isInferred=*/false}); + {mutableAttr, attrType, nominal, decl, /*isInferred=*/false}); + } } // Do not allow transitive protocol inference between protocols. @@ -195,24 +205,6 @@ GetTypeWrapper::evaluate(Evaluator &evaluator, NominalTypeDecl *decl) const { return typeWrappers.front(); } -Type GetTypeWrapperType::evaluate(Evaluator &evaluator, - NominalTypeDecl *decl) const { - auto typeWrapperInfo = decl->getTypeWrapper(); - if (!typeWrapperInfo) - return Type(); - - auto type = evaluateOrDefault( - evaluator, - CustomAttrTypeRequest{typeWrapperInfo->Attr, decl->getDeclContext(), - CustomAttrTypeKind::TypeWrapper}, - Type()); - - if (!type || type->hasError()) { - return ErrorType::get(decl->getASTContext()); - } - return type; -} - VarDecl *NominalTypeDecl::getTypeWrapperProperty() const { auto *mutableSelf = const_cast(this); return evaluateOrDefault(getASTContext().evaluator, @@ -352,10 +344,7 @@ GetTypeWrapperProperty::evaluate(Evaluator &evaluator, auto *storage = parent->getTypeWrapperStorageDecl(); assert(storage); - auto *typeWrapperType = - evaluateOrDefault(ctx.evaluator, GetTypeWrapperType{parent}, Type()) - ->castTo(); - assert(typeWrapperType); + auto *typeWrapperType = typeWrapper->AttrType->castTo(); // $storage: Wrapper<, .$Storage> auto propertyTy = BoundGenericType::get(