diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index a271d4b940970..445c019d6f60c 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -5295,10 +5295,10 @@ class AbstractStorageDecl : public ValueDecl { /// Overwrite the registered implementation-info. This should be /// used carefully. - void setImplInfo(StorageImplInfo implInfo) { - LazySemanticInfo.ImplInfoComputed = 1; - ImplInfo = implInfo; - } + void setImplInfo(StorageImplInfo implInfo); + + /// Cache the implementation-info, for use by the request-evaluator. + void cacheImplInfo(StorageImplInfo implInfo); ReadImplKind getReadImpl() const { return getImplInfo().getReadImpl(); @@ -5313,9 +5313,7 @@ class AbstractStorageDecl : public ValueDecl { /// Return true if this is a VarDecl that has storage associated with /// it. - bool hasStorage() const { - return getImplInfo().hasStorage(); - } + bool hasStorage() const; /// Return true if this storage has the basic accessors/capability /// to be mutated. This is generally constant after the accessors are diff --git a/include/swift/AST/DiagnosticsSema.def b/include/swift/AST/DiagnosticsSema.def index b3dcedcb8ce1b..7e1747ebf7eb3 100644 --- a/include/swift/AST/DiagnosticsSema.def +++ b/include/swift/AST/DiagnosticsSema.def @@ -7095,6 +7095,13 @@ ERROR(invalid_macro_role_for_macro_syntax,none, (unsigned)) ERROR(macro_cannot_introduce_names,none, "'%0' macros are not allowed to introduce names", (StringRef)) +ERROR(macro_accessor_missing_from_expansion,none, + "expansion of macro %0 did not produce a %select{non-|}1observing " + "accessor", + (DeclName, bool)) +ERROR(macro_init_accessor_not_documented,none, + "expansion of macro %0 produced an unexpected 'init' accessor", + (DeclName)) ERROR(macro_resolve_circular_reference, none, "circular reference resolving %select{freestanding|attached}0 macro %1", diff --git a/include/swift/AST/Evaluator.h b/include/swift/AST/Evaluator.h index 70b89958befe5..7e42e727eb61f 100644 --- a/include/swift/AST/Evaluator.h +++ b/include/swift/AST/Evaluator.h @@ -316,6 +316,12 @@ class Evaluator { cache.insert(request, std::move(output)); } + template::type* = nullptr> + bool hasCachedResult(const Request &request) { + return cache.find_as(request) != cache.end(); + } + /// Do not introduce new callers of this function. template::type* = nullptr> diff --git a/include/swift/AST/NameLookup.h b/include/swift/AST/NameLookup.h index 7f3634589140c..82085c59f5b09 100644 --- a/include/swift/AST/NameLookup.h +++ b/include/swift/AST/NameLookup.h @@ -555,6 +555,13 @@ void forEachPotentialResolvedMacro( llvm::function_ref body ); +/// For each macro with the given role that might be attached to the given +/// declaration, call the body. +void forEachPotentialAttachedMacro( + Decl *decl, MacroRole role, + llvm::function_ref body +); + } // end namespace namelookup /// Describes an inherited nominal entry. diff --git a/include/swift/AST/TypeCheckRequests.h b/include/swift/AST/TypeCheckRequests.h index 1e1e9a515c36f..8574c38fa42f1 100644 --- a/include/swift/AST/TypeCheckRequests.h +++ b/include/swift/AST/TypeCheckRequests.h @@ -1686,6 +1686,26 @@ class InitAccessorPropertiesRequest : ArrayRef evaluate(Evaluator &evaluator, NominalTypeDecl *decl) const; + // Evaluation. + bool evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const; + +public: + bool isCached() const { return true; } +}; + +class HasStorageRequest : + public SimpleRequest { +public: + using SimpleRequest::SimpleRequest; + +private: + friend SimpleRequest; + + // Evaluation. + bool evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const; + public: bool isCached() const { return true; } }; diff --git a/include/swift/AST/TypeCheckerTypeIDZone.def b/include/swift/AST/TypeCheckerTypeIDZone.def index 581a7bd27d8b2..e13aced855628 100644 --- a/include/swift/AST/TypeCheckerTypeIDZone.def +++ b/include/swift/AST/TypeCheckerTypeIDZone.def @@ -302,6 +302,9 @@ SWIFT_REQUEST(TypeChecker, SelfAccessKindRequest, SelfAccessKind(FuncDecl *), SWIFT_REQUEST(TypeChecker, StorageImplInfoRequest, StorageImplInfo(AbstractStorageDecl *), SeparatelyCached, NoLocationInfo) +SWIFT_REQUEST(TypeChecker, HasStorageRequest, + bool(AbstractStorageDecl *), Cached, + NoLocationInfo) SWIFT_REQUEST(TypeChecker, StoredPropertiesAndMissingMembersRequest, ArrayRef(NominalTypeDecl *), Cached, NoLocationInfo) SWIFT_REQUEST(TypeChecker, StoredPropertiesRequest, diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 2a37eb3e7a8b8..b6d9b5b05b76d 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -2525,7 +2525,7 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics, ResilienceExpansion expansion) const { switch (semantics) { case AccessSemantics::DirectToStorage: - assert(hasStorage()); + assert(hasStorage() || getASTContext().Diags.hadAnyError()); return AccessStrategy::getStorage(); case AccessSemantics::DistributedThunk: @@ -6384,6 +6384,13 @@ bool ProtocolDecl::hasCircularInheritedProtocols() const { ctx.evaluator, HasCircularInheritedProtocolsRequest{mutableThis}, true); } +bool AbstractStorageDecl::hasStorage() const { + ASTContext &ctx = getASTContext(); + return evaluateOrDefault(ctx.evaluator, + HasStorageRequest{const_cast(this)}, + false); +} + StorageImplInfo AbstractStorageDecl::getImplInfo() const { ASTContext &ctx = getASTContext(); return evaluateOrDefault(ctx.evaluator, @@ -6391,6 +6398,26 @@ StorageImplInfo AbstractStorageDecl::getImplInfo() const { StorageImplInfo::getSimpleStored(StorageIsMutable)); } +void AbstractStorageDecl::cacheImplInfo(StorageImplInfo implInfo) { + LazySemanticInfo.ImplInfoComputed = 1; + ImplInfo = implInfo; +} + +void AbstractStorageDecl::setImplInfo(StorageImplInfo implInfo) { + cacheImplInfo(implInfo); + + if (isImplicit()) { + auto &evaluator = getASTContext().evaluator; + HasStorageRequest request{this}; + if (!evaluator.hasCachedResult(request)) + evaluator.cacheOutput(request, implInfo.hasStorage()); + else { + assert( + evaluateOrDefault(evaluator, request, false) == implInfo.hasStorage()); + } + } +} + bool AbstractStorageDecl::hasPrivateAccessor() const { for (auto accessor : getAllAccessors()) { if (hasPrivateOrFilePrivateFormalAccess(accessor)) diff --git a/lib/AST/NameLookup.cpp b/lib/AST/NameLookup.cpp index c656b5f8b87e1..98b13f3550473 100644 --- a/lib/AST/NameLookup.cpp +++ b/lib/AST/NameLookup.cpp @@ -1627,7 +1627,7 @@ void namelookup::forEachPotentialResolvedMacro( /// For each macro with the given role that might be attached to the given /// declaration, call the body. -static void forEachPotentialAttachedMacro( +void namelookup::forEachPotentialAttachedMacro( Decl *decl, MacroRole role, llvm::function_ref body ) { diff --git a/lib/AST/TypeCheckRequests.cpp b/lib/AST/TypeCheckRequests.cpp index 0b7666c79bddf..e499f32abb3c0 100644 --- a/lib/AST/TypeCheckRequests.cpp +++ b/lib/AST/TypeCheckRequests.cpp @@ -687,7 +687,7 @@ StorageImplInfoRequest::getCachedResult() const { void StorageImplInfoRequest::cacheResult(StorageImplInfo value) const { auto *storage = std::get<0>(getStorage()); - storage->setImplInfo(value); + storage->cacheImplInfo(value); } //----------------------------------------------------------------------------// diff --git a/lib/ClangImporter/ClangImporter.cpp b/lib/ClangImporter/ClangImporter.cpp index 8edfa5bd038bb..6088ffe0a15bd 100644 --- a/lib/ClangImporter/ClangImporter.cpp +++ b/lib/ClangImporter/ClangImporter.cpp @@ -34,6 +34,7 @@ #include "swift/AST/NameLookupRequests.h" #include "swift/AST/PrettyStackTrace.h" #include "swift/AST/SourceFile.h" +#include "swift/AST/TypeCheckRequests.h" #include "swift/AST/Types.h" #include "swift/Basic/Defer.h" #include "swift/Basic/Platform.h" @@ -5042,6 +5043,7 @@ cloneBaseMemberDecl(ValueDecl *decl, DeclContext *newContext) { out->setIsObjC(var->isObjC()); out->setIsDynamic(var->isDynamic()); out->copyFormalAccessFrom(var); + out->getASTContext().evaluator.cacheOutput(HasStorageRequest{out}, false); out->setAccessors(SourceLoc(), makeBaseClassMemberAccessors(newContext, out, var), SourceLoc()); diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index 04515f620d861..f2e981dc1d161 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -126,6 +126,7 @@ void ClangImporter::Implementation::makeComputed(AbstractStorageDecl *storage, AccessorDecl *getter, AccessorDecl *setter) { assert(getter); + storage->getASTContext().evaluator.cacheOutput(HasStorageRequest{storage}, false); if (setter) { storage->setImplInfo(StorageImplInfo::getMutableComputed()); storage->setAccessors(SourceLoc(), {getter, setter}, SourceLoc()); diff --git a/lib/Sema/DerivedConformanceEquatableHashable.cpp b/lib/Sema/DerivedConformanceEquatableHashable.cpp index 4c88cffb24fa2..98e5bedec016a 100644 --- a/lib/Sema/DerivedConformanceEquatableHashable.cpp +++ b/lib/Sema/DerivedConformanceEquatableHashable.cpp @@ -890,6 +890,10 @@ static ValueDecl *deriveHashable_hashValue(DerivedConformance &derived) { SourceLoc(), C.Id_hashValue, parentDC); hashValueDecl->setInterfaceType(intType); hashValueDecl->setSynthesized(); + hashValueDecl->setImplicit(); + hashValueDecl->setImplInfo(StorageImplInfo::getImmutableComputed()); + hashValueDecl->copyFormalAccessFrom(derived.Nominal, + /*sourceIsParentContext*/ true); ParameterList *params = ParameterList::createEmpty(C); @@ -908,12 +912,7 @@ static ValueDecl *deriveHashable_hashValue(DerivedConformance &derived) { /*sourceIsParentContext*/ true); // Finish creating the property. - hashValueDecl->setImplicit(); - hashValueDecl->setInterfaceType(intType); - hashValueDecl->setImplInfo(StorageImplInfo::getImmutableComputed()); hashValueDecl->setAccessors(SourceLoc(), {getterDecl}, SourceLoc()); - hashValueDecl->copyFormalAccessFrom(derived.Nominal, - /*sourceIsParentContext*/ true); // The derived hashValue of an actor must be nonisolated. if (!addNonIsolatedToSynthesized(derived.Nominal, hashValueDecl) && diff --git a/lib/Sema/TypeCheckMacros.cpp b/lib/Sema/TypeCheckMacros.cpp index 4e9343e359f78..5a64184d06448 100644 --- a/lib/Sema/TypeCheckMacros.cpp +++ b/lib/Sema/TypeCheckMacros.cpp @@ -1241,6 +1241,50 @@ static SourceFile *evaluateAttachedMacro(MacroDecl *macro, Decl *attachedTo, return macroSourceFile; } +bool swift::accessorMacroOnlyIntroducesObservers( + MacroDecl *macro, + const MacroRoleAttr *attr +) { + // Will this macro introduce observers? + bool foundObserver = false; + for (auto name : attr->getNames()) { + if (name.getKind() == MacroIntroducedDeclNameKind::Named && + (name.getName().getBaseName().userFacingName() == "willSet" || + name.getName().getBaseName().userFacingName() == "didSet")) { + foundObserver = true; + } else { + // Introduces something other than an observer. + return false; + } + } + + if (foundObserver) + return true; + + // WORKAROUND: Older versions of the Observation library make + // `ObservationIgnored` an accessor macro that implies that it makes a + // stored property computed. Override that, because we know it produces + // nothing. + if (macro->getName().getBaseName().userFacingName() == "ObservationIgnored") { + return true; + } + + return false; +} + +bool swift::accessorMacroIntroducesInitAccessor( + MacroDecl *macro, const MacroRoleAttr *attr +) { + for (auto name : attr->getNames()) { + if (name.getKind() == MacroIntroducedDeclNameKind::Named && + (name.getName().getBaseName().getKind() == + DeclBaseName::Kind::Constructor)) + return true; + } + + return false; +} + Optional swift::expandAccessors( AbstractStorageDecl *storage, CustomAttr *attr, MacroDecl *macro ) { @@ -1252,30 +1296,54 @@ Optional swift::expandAccessors( return None; PrettyStackTraceDecl debugStack( - "type checking expanded declaration macro", storage); + "type checking expanded accessor macro", storage); // Trigger parsing of the sequence of accessor declarations. This has the // side effect of registering those accessor declarations with the storage // declaration, so there is nothing further to do. + bool foundNonObservingAccessor = false; + bool foundInitAccessor = false; for (auto decl : macroSourceFile->getTopLevelItems()) { auto accessor = dyn_cast_or_null(decl.dyn_cast()); if (!accessor) continue; - if (accessor->isObservingAccessor()) - continue; + if (accessor->isInitAccessor()) + foundInitAccessor = true; + if (!accessor->isObservingAccessor()) + foundNonObservingAccessor = true; + } + + auto roleAttr = macro->getMacroRoleAttr(MacroRole::Accessor); + bool expectedNonObservingAccessor = + !accessorMacroOnlyIntroducesObservers(macro, roleAttr); + if (foundNonObservingAccessor) { // If any non-observing accessor was added, mark the initializer as // subsumed. if (auto var = dyn_cast(storage)) { if (auto binding = var->getParentPatternBinding()) { unsigned index = binding->getPatternEntryIndexForVarDecl(var); binding->setInitializerSubsumed(index); - break; } } } + // Make sure we got non-observing accessors exactly where we expected to. + if (foundNonObservingAccessor != expectedNonObservingAccessor) { + storage->diagnose( + diag::macro_accessor_missing_from_expansion, macro->getName(), + !expectedNonObservingAccessor); + } + + // 'init' accessors must be documented in the macro role attribute. + if (foundInitAccessor && + !accessorMacroIntroducesInitAccessor(macro, roleAttr)) { + storage->diagnose( + diag::macro_init_accessor_not_documented, macro->getName()); + // FIXME: Add the appropriate "names: named(init)". + } + return macroSourceFile->getBufferID(); } diff --git a/lib/Sema/TypeCheckMacros.h b/lib/Sema/TypeCheckMacros.h index 4fe1bce6fa780..b960baac4010c 100644 --- a/lib/Sema/TypeCheckMacros.h +++ b/lib/Sema/TypeCheckMacros.h @@ -77,6 +77,16 @@ Optional expandPeers(CustomAttr *attr, MacroDecl *macro, Decl *decl); Optional expandConformances(CustomAttr *attr, MacroDecl *macro, NominalTypeDecl *nominal); +/// Determine whether an accessor macro with the given attribute only +/// introduces observers like willSet and didSet. +bool accessorMacroOnlyIntroducesObservers( + MacroDecl *macro, const MacroRoleAttr *attr); + +/// Determine whether an accessor macro (defined with the given role attribute) +/// introduces an init accessor. +bool accessorMacroIntroducesInitAccessor( + MacroDecl *macro, const MacroRoleAttr *attr); + } // end namespace swift #endif /* SWIFT_SEMA_TYPECHECKMACROS_H */ diff --git a/lib/Sema/TypeCheckStorage.cpp b/lib/Sema/TypeCheckStorage.cpp index 4ab480542ec15..c9a754885ee2f 100644 --- a/lib/Sema/TypeCheckStorage.cpp +++ b/lib/Sema/TypeCheckStorage.cpp @@ -106,8 +106,16 @@ static bool hasStoredProperties(NominalTypeDecl *decl, || (decl != implDecl)))); } -static void computeLoweredStoredProperties(NominalTypeDecl *decl, - IterableDeclContext *implDecl) { +namespace { + enum class LoweredPropertiesReason { + Stored, + Memberwise + }; +} + +static void computeLoweredProperties(NominalTypeDecl *decl, + IterableDeclContext *implDecl, + LoweredPropertiesReason reason) { // Expand synthesized member macros. auto &ctx = decl->getASTContext(); (void)evaluateOrDefault(ctx.evaluator, @@ -127,15 +135,20 @@ static void computeLoweredStoredProperties(NominalTypeDecl *decl, if (!var || var->isStatic()) continue; - if (var->getAttrs().hasAttribute()) - (void) var->getLazyStorageProperty(); + if (reason == LoweredPropertiesReason::Stored) { + if (var->getAttrs().hasAttribute()) + (void) var->getLazyStorageProperty(); - if (var->hasAttachedPropertyWrapper()) { - (void) var->getPropertyWrapperAuxiliaryVariables(); - (void) var->getPropertyWrapperInitializerInfo(); + if (var->hasAttachedPropertyWrapper()) { + (void) var->getPropertyWrapperAuxiliaryVariables(); + (void) var->getPropertyWrapperInitializerInfo(); + } } } + if (reason != LoweredPropertiesReason::Stored) + return; + // If this is an actor, check conformance to the Actor protocol to // ensure that the actor storage will get created (if needed). if (auto classDecl = dyn_cast(decl)) { @@ -164,6 +177,11 @@ static void computeLoweredStoredProperties(NominalTypeDecl *decl, } } +static void computeLoweredStoredProperties(NominalTypeDecl *decl, + IterableDeclContext *implDecl) { + computeLoweredProperties(decl, implDecl, LoweredPropertiesReason::Stored); +} + /// Enumerate both the stored properties and missing members, /// in a deterministic order. static void enumerateStoredPropertiesAndMissing( @@ -283,13 +301,46 @@ StoredPropertiesAndMissingMembersRequest::evaluate(Evaluator &evaluator, return decl->getASTContext().AllocateCopy(results); } +/// Determine whether the given variable has an init accessor. +static bool hasInitAccessor(VarDecl *var) { + if (var->getAccessor(AccessorKind::Init)) + return true; + + // Look to see whether it is possible that there is an init accessor. + bool hasInitAccessor = false; + namelookup::forEachPotentialAttachedMacro( + var, MacroRole::Accessor, + [&](MacroDecl *macro, const MacroRoleAttr *attr) { + if (accessorMacroIntroducesInitAccessor(macro, attr)) + hasInitAccessor = true; + }); + + // There is no chance for an init accessor, so we're done. + if (!hasInitAccessor) + return false; + + // We might get an init accessor by expanding accessor macros; do so now. + (void)evaluateOrDefault( + var->getASTContext().evaluator, ExpandAccessorMacros{var}, { }); + + return var->getAccessor(AccessorKind::Init); +} + ArrayRef InitAccessorPropertiesRequest::evaluate(Evaluator &evaluator, NominalTypeDecl *decl) const { + IterableDeclContext *implDecl = decl->getImplementationContext(); + + if (!hasStoredProperties(decl, implDecl)) + return ArrayRef(); + + // Make sure we expand what we need to to get all of the properties. + computeLoweredProperties(decl, implDecl, LoweredPropertiesReason::Memberwise); + SmallVector results; for (auto *member : decl->getMembers()) { auto *var = dyn_cast(member); - if (!var || !var->getAccessor(AccessorKind::Init)) { + if (!var || var->isStatic() || !hasInitAccessor(var)) { continue; } @@ -299,41 +350,6 @@ InitAccessorPropertiesRequest::evaluate(Evaluator &evaluator, return decl->getASTContext().AllocateCopy(results); } -/// Check whether the pattern may have storage. -/// -/// This query is careful not to trigger accessor macro expansion, which -/// creates a cycle. It conservatively assumes that all accessor macros -/// produce computed properties, which is... incorrect. -/// -/// The query also avoids triggering a `StorageImplInfoRequest` for patterns -/// involved in a ProtocolDecl, because we know they can never contain storage. -/// For background, vars of noncopyable type have their OpaqueReadOwnership -/// determined by the type of the var decl, but that type hasn't always been -/// determined when this query is made. -static bool mayHaveStorage(Pattern *pattern) { - // Check whether there are any accessor macros, or it's a protocol member. - bool hasAccessorMacros = false; - bool inProtocolDecl = false; - pattern->forEachVariable([&](VarDecl *VD) { - if (isa(VD->getDeclContext())) - inProtocolDecl = true; - - VD->forEachAttachedMacro(MacroRole::Accessor, - [&](CustomAttr *customAttr, MacroDecl *macro) { - hasAccessorMacros = true; - }); - }); - - if (hasAccessorMacros) - return false; - - // protocol members can never contain storage; avoid triggering request. - if (inProtocolDecl) - return false; - - return pattern->hasStorage(); -} - /// Validate the \c entryNumber'th entry in \c binding. const PatternBindingEntry *PatternBindingEntryRequest::evaluate( Evaluator &eval, PatternBindingDecl *binding, unsigned entryNumber, @@ -434,7 +450,7 @@ const PatternBindingEntry *PatternBindingEntryRequest::evaluate( // default-initializable. If so, do it. if (!pbe.isInitialized() && binding->isDefaultInitializable(entryNumber) && - mayHaveStorage(pattern)) { + pattern->hasStorage()) { if (auto defaultInit = TypeChecker::buildDefaultInitializer(patternType)) { // If we got a default initializer, install it and re-type-check it // to make sure it is properly coerced to the pattern type. @@ -1403,7 +1419,7 @@ synthesizeTrivialGetterBody(AccessorDecl *getter, TargetImpl target, /// underlying storage. static std::pair synthesizeTrivialGetterBody(AccessorDecl *getter, ASTContext &ctx) { - assert(getter->getStorage()->hasStorage()); + assert(getter->getStorage()->getImplInfo().hasStorage()); return synthesizeTrivialGetterBody(getter, TargetImpl::Storage, ctx); } @@ -3444,6 +3460,73 @@ static StorageImplInfo classifyWithHasStorageAttr(VarDecl *var) { return StorageImplInfo(ReadImplKind::Stored, writeImpl, readWriteImpl); } +bool HasStorageRequest::evaluate(Evaluator &evaluator, + AbstractStorageDecl *storage) const { + // Parameters are always stored. + if (isa(storage)) + return true; + + // Only variables can be stored. + auto *var = dyn_cast(storage); + if (!var) + return false; + + // @_hasStorage implies that it... has storage. + if (var->getAttrs().hasAttribute()) + return true; + + // Protocol requirements never have storage. + if (isa(storage->getDeclContext())) + return false; + + // lazy declarations do not have storage. + if (storage->getAttrs().hasAttribute()) + return false; + + // @NSManaged attributes don't have storage + if (storage->getAttrs().hasAttribute()) + return false; + + // Any accessors that read or write imply that there is no storage. + if (storage->getParsedAccessor(AccessorKind::Get) || + storage->getParsedAccessor(AccessorKind::Read) || + storage->getParsedAccessor(AccessorKind::Address) || + storage->getParsedAccessor(AccessorKind::Set) || + storage->getParsedAccessor(AccessorKind::Modify) || + storage->getParsedAccessor(AccessorKind::MutableAddress)) + return false; + + // willSet or didSet in an overriding property imply that there is no storage. + if ((storage->getParsedAccessor(AccessorKind::WillSet) || + storage->getParsedAccessor(AccessorKind::DidSet)) && + storage->getAttrs().hasAttribute()) + return false; + + // The presence of a property wrapper implies that there is no storage. + if (var->hasAttachedPropertyWrapper()) + return false; + + // Look for any accessor macros that might make this property computed. + bool hasStorage = true; + namelookup::forEachPotentialAttachedMacro( + var, MacroRole::Accessor, + [&](MacroDecl *macro, const MacroRoleAttr *attr) { + // Will this macro introduce observers? + bool foundObserver = accessorMacroOnlyIntroducesObservers(macro, attr); + + // If it's not (just) introducing observers, it's making the property + // computed. + if (!foundObserver) + hasStorage = false; + + // If it will introduce observers, and there is an "override", + // the property doesn't have storage. + if (foundObserver && storage->getAttrs().hasAttribute()) + hasStorage = false; + }); + return hasStorage; +} + StorageImplInfo StorageImplInfoRequest::evaluate(Evaluator &evaluator, AbstractStorageDecl *storage) const { @@ -3603,6 +3686,8 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator, StorageImplInfo info(readImpl, writeImpl, readWriteImpl); finishStorageImplInfo(storage, info); + assert(info.hasStorage() == storage->hasStorage() || + storage->getASTContext().Diags.hadAnyError()); return info; } diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index cb22eb64de93b..d5e1af5156d69 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -2795,6 +2795,11 @@ void ModuleFile::configureStorage(AbstractStorageDecl *decl, auto readWriteImpl = getActualReadWriteImplKind(rawReadWriteImplKind); if (!readWriteImpl) return; + auto implInfo = StorageImplInfo(*readImpl, *writeImpl, *readWriteImpl); + decl->setImplInfo(implInfo); + + decl->getASTContext().evaluator.cacheOutput(HasStorageRequest{decl}, implInfo.hasStorage()); + SmallVector accessors; for (DeclID id : rawIDs.IDs) { auto accessor = dyn_cast_or_null(getDecl(id)); @@ -2802,9 +2807,6 @@ void ModuleFile::configureStorage(AbstractStorageDecl *decl, accessors.push_back(accessor); } - auto implInfo = StorageImplInfo(*readImpl, *writeImpl, *readWriteImpl); - decl->setImplInfo(implInfo); - if (implInfo.isSimpleStored() && accessors.empty()) return; @@ -3421,6 +3423,9 @@ class DeclDeserializer { var->setIsSetterMutating(isSetterMutating); declOrOffset = var; + MF.configureStorage(var, opaqueReadOwnership, + readImpl, writeImpl, readWriteImpl, accessors); + auto interfaceTypeOrError = MF.getTypeChecked(interfaceTypeID); if (!interfaceTypeOrError) return interfaceTypeOrError.takeError(); @@ -3432,8 +3437,6 @@ class DeclDeserializer { AddAttribute( new (ctx) ReferenceOwnershipAttr(referenceStorage->getOwnership())); - MF.configureStorage(var, opaqueReadOwnership, - readImpl, writeImpl, readWriteImpl, accessors); auto accessLevel = getActualAccessLevel(rawAccessLevel); if (!accessLevel) return MF.diagnoseFatal(); diff --git a/stdlib/public/Observation/Sources/Observation/Observable.swift b/stdlib/public/Observation/Sources/Observation/Observable.swift index a2f837af85db3..cc7d1f5732c3b 100644 --- a/stdlib/public/Observation/Sources/Observation/Observable.swift +++ b/stdlib/public/Observation/Sources/Observation/Observable.swift @@ -23,11 +23,11 @@ #endif @attached(memberAttribute) @attached(conformance) -public macro Observable() = +public macro Observable() = #externalMacro(module: "ObservationMacros", type: "ObservableMacro") @available(SwiftStdlib 5.9, *) -@attached(accessor) +@attached(accessor, names: named(init), named(get), named(set)) #if OBSERVATION_SUPPORTS_PEER_MACROS @attached(peer, names: prefixed(_)) #endif @@ -35,7 +35,7 @@ public macro ObservationTracked() = #externalMacro(module: "ObservationMacros", type: "ObservationTrackedMacro") @available(SwiftStdlib 5.9, *) -@attached(accessor) +@attached(accessor, names: named(willSet)) public macro ObservationIgnored() = #externalMacro(module: "ObservationMacros", type: "ObservationIgnoredMacro") diff --git a/test/Macros/accessor_has_storage_cycle.swift b/test/Macros/accessor_has_storage_cycle.swift new file mode 100644 index 0000000000000..49fb2076adfa4 --- /dev/null +++ b/test/Macros/accessor_has_storage_cycle.swift @@ -0,0 +1,21 @@ +// REQUIRES: swift_swift_parser + +// RUN: %target-typecheck-verify-swift -swift-version 5 -swift-version 5 + +struct Predicate { } + +@freestanding(expression) +macro Predicate(_ body: (T) -> Void) -> Predicate = #externalMacro(module: "A", type: "B") +// expected-warning@-1{{could not be found}} + +@attached(accessor) +@attached(peer) +macro Foo(filter: Predicate) = #externalMacro(module: "A", type: "B") +// expected-warning@-1{{could not be found}} +// expected-note@-2 2{{declared here}} + +struct Content { + @Foo(filter: #Predicate { $0 == true }) var foo: Bool = true + //expected-error@-1 2{{could not be found for macro}} + // expected-warning@-2{{result of operator '==' is unused}} +} diff --git a/test/ModuleInterface/Observable.swift b/test/ModuleInterface/Observable.swift index 7bcad03394c64..181a4325ef231 100644 --- a/test/ModuleInterface/Observable.swift +++ b/test/ModuleInterface/Observable.swift @@ -1,5 +1,5 @@ // RUN: %empty-directory(%t) -// RUN: %target-swift-emit-module-interface(%t/Library.swiftinterface) %s -module-name Library -plugin-path %swift-host-lib-dir/plugins -disable-availability-checking +// RUN: %target-swift-emit-module-interface(%t/Library.swiftinterface) %s -module-name Library -plugin-path %swift-host-lib-dir/plugins -disable-availability-checking -enable-experimental-feature InitAccessors // RUN: %target-swift-typecheck-module-from-interface(%t/Library.swiftinterface) -module-name Library -disable-availability-checking // RUN: %FileCheck %s < %t/Library.swiftinterface diff --git a/test/Serialization/macros.swift b/test/Serialization/macros.swift index fa7da055d3509..4933762698d3f 100644 --- a/test/Serialization/macros.swift +++ b/test/Serialization/macros.swift @@ -21,6 +21,7 @@ func test(a: Int, b: Int) { struct TestStruct { @myWrapper var x: Int + // expected-error@-1{{expansion of macro 'myWrapper' did not produce a non-observing accessor}} } @ArbitraryMembers diff --git a/test/stdlib/Observation/Observable.swift b/test/stdlib/Observation/Observable.swift index 8e2f4755cfc8f..d7ead2e4f65ea 100644 --- a/test/stdlib/Observation/Observable.swift +++ b/test/stdlib/Observation/Observable.swift @@ -33,7 +33,7 @@ func validateMemberwiseInitializers() { @Observable struct DefiniteInitialization { var field: Int - + init(field: Int) { self.field = field } @@ -60,21 +60,21 @@ class ContainsIUO { } class NonObservable { - + } @Observable class InheritsFromNonObservable: NonObservable { - + } protocol NonObservableProtocol { - + } @Observable class ConformsToNonObservableProtocol: NonObservableProtocol { - + } struct NonObservableContainer { @@ -89,19 +89,19 @@ class ImplementsAccessAndMutation { var field = 3 let accessCalled: (PartialKeyPath) -> Void let withMutationCalled: (PartialKeyPath) -> Void - + init(accessCalled: @escaping (PartialKeyPath) -> Void, withMutationCalled: @escaping (PartialKeyPath) -> Void) { self.accessCalled = accessCalled self.withMutationCalled = withMutationCalled } - + internal func access( keyPath: KeyPath ) { accessCalled(keyPath) _$observationRegistrar.access(self, keyPath: keyPath) } - + internal func withMutation( keyPath: KeyPath, _ mutation: () throws -> T @@ -126,7 +126,7 @@ class Entity { class Person : Entity { var firstName = "" var lastName = "" - + var friends = [Person]() var fullName: String { firstName + " " + lastName } @@ -163,7 +163,7 @@ class HasIntermediaryConformance: Intermediary { } class CapturedState: @unchecked Sendable { var state: State - + init(state: State) { self.state = state }