From 5f3eaa8feba60921e6272fc83e9d8fa84222c8bd Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 6 Aug 2018 19:29:41 -0400 Subject: [PATCH 1/2] Fix a couple of oversights relating to modify coroutines. --- include/swift/AST/Decl.h | 2 + lib/Sema/TypeCheckProtocol.cpp | 16 ++++++-- lib/Serialization/Deserialization.cpp | 45 +++++++---------------- lib/Serialization/DeserializationErrors.h | 6 +++ 4 files changed, 35 insertions(+), 34 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 9b94794ba69eb..51ade9151ebc7 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -6506,6 +6506,8 @@ AbstractStorageDecl::overwriteSetterAccess(AccessLevel accessLevel) { setter->overwriteAccess(accessLevel); if (auto materializeForSet = getMaterializeForSetFunc()) materializeForSet->overwriteAccess(accessLevel); + if (auto modify = getModifyCoroutine()) + modify->overwriteAccess(accessLevel); if (auto mutableAddressor = getMutableAddressor()) mutableAddressor->overwriteAccess(accessLevel); } diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 81d82adafdc20..4557a8372a3b3 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -295,7 +295,10 @@ static bool checkMutating(FuncDecl *requirement, FuncDecl *witness, witnessMutating = isReadWriteMutating(); break; - default: +#define OPAQUE_ACCESSOR(ID, KEYWORD) +#define ACCESSOR(ID) \ + case AccessorKind::ID: +#include "swift/AST/AccessorKinds.def" llvm_unreachable("unexpected accessor requirement"); } } @@ -380,6 +383,8 @@ static ValueDecl *getStandinForAccessor(AbstractStorageDecl *witnessStorage, case AccessorKind::Get: if (auto addressor = getExplicitAccessor(AccessorKind::Address)) return addressor; + if (auto read = getExplicitAccessor(AccessorKind::Read)) + return read; break; case AccessorKind::MaterializeForSet: @@ -390,10 +395,15 @@ static ValueDecl *getStandinForAccessor(AbstractStorageDecl *witnessStorage, case AccessorKind::Set: if (auto addressor = getExplicitAccessor(AccessorKind::MutableAddress)) return addressor; + if (auto modify = getExplicitAccessor(AccessorKind::Modify)) + return modify; break; - default: - break; +#define OPAQUE_ACCESSOR(ID, KEYWORD) +#define ACCESSOR(ID) \ + case AccessorKind::ID: +#include "swift/AST/AccessorKinds.def" + llvm_unreachable("unexpected accessor requirement"); } // Otherwise, just diagnose starting at the storage declaration itself. diff --git a/lib/Serialization/Deserialization.cpp b/lib/Serialization/Deserialization.cpp index 397703848b263..3c5e2d0e4c206 100644 --- a/lib/Serialization/Deserialization.cpp +++ b/lib/Serialization/Deserialization.cpp @@ -196,6 +196,17 @@ void ModuleFile::finishPendingActions() { "wrong module used for delayed actions"); } +static Optional +getActualAccessorKind(uint8_t raw) { + switch (serialization::AccessorKind(raw)) { +#define ACCESSOR(ID) \ + case serialization::AccessorKind::ID: return swift::AccessorKind::ID; +#include "swift/AST/AccessorKinds.def" + } + + return None; +} + /// Translate from the serialization DefaultArgumentKind enumerators, which are /// guaranteed to be stable, to the AST ones. static Optional @@ -1587,30 +1598,13 @@ ModuleFile::resolveCrossReference(ModuleDecl *baseModule, uint32_t pathLen) { if (!values.front()->getBaseName().isOperator()) { pathTrace.addAccessor(rawKind); if (auto storage = dyn_cast(values.front())) { - switch (rawKind) { - case Get: - values.front() = storage->getGetter(); - break; - case Set: - values.front() = storage->getSetter(); - break; - case MaterializeForSet: - values.front() = storage->getMaterializeForSetFunc(); - break; - case Address: - values.front() = storage->getAddressor(); - break; - case MutableAddress: - values.front() = storage->getMutableAddressor(); - break; - case WillSet: - case DidSet: - llvm_unreachable("invalid XREF accessor kind"); - default: + auto actualKind = getActualAccessorKind(rawKind); + if (!actualKind) { // Unknown accessor kind. error(); return nullptr; } + values.front() = storage->getAccessor(*actualKind); } break; } @@ -2032,17 +2026,6 @@ getActualOptionalTypeKind(uint8_t raw) { return None; } -static Optional -getActualAccessorKind(uint8_t raw) { - switch (serialization::AccessorKind(raw)) { -#define ACCESSOR(ID) \ - case serialization::AccessorKind::ID: return swift::AccessorKind::ID; -#include "swift/AST/AccessorKinds.def" - } - - return None; -} - static Optional getActualAddressorKind(uint8_t raw) { switch (serialization::AddressorKind(raw)) { diff --git a/lib/Serialization/DeserializationErrors.h b/lib/Serialization/DeserializationErrors.h index 2d3b8f9cc3147..d9ec1aea7b6c1 100644 --- a/lib/Serialization/DeserializationErrors.h +++ b/lib/Serialization/DeserializationErrors.h @@ -131,6 +131,12 @@ class XRefTracePath { case DidSet: os << "(didSet)"; break; + case Read: + os << "(read)"; + break; + case Modify: + os << "(modify)"; + break; default: os << "(unknown accessor kind)"; break; From ebed6afd59121acdfda69fcc0998fbb0d92736af Mon Sep 17 00:00:00 2001 From: John McCall Date: Mon, 6 Aug 2018 19:30:02 -0400 Subject: [PATCH 2/2] Make it easy to iterate over all the opaque accessors of a declaration. --- include/swift/AST/Decl.h | 10 ++ include/swift/SIL/SILWitnessVisitor.h | 12 +- lib/AST/Decl.cpp | 50 +++++++ lib/SILGen/SILGen.cpp | 13 +- lib/Sema/CodeSynthesis.cpp | 199 +++++++++++++------------- lib/Sema/CodeSynthesis.h | 2 - lib/TBDGen/TBDGen.cpp | 13 +- 7 files changed, 174 insertions(+), 125 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index 51ade9151ebc7..1b9c326f6b7db 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -4263,6 +4263,13 @@ class AbstractStorageDecl : public ValueDecl { return {}; } + /// Visit all the opaque accessors that this storage is expected to have. + void visitExpectedOpaqueAccessors( + llvm::function_ref) const; + + /// Visit all the opaque accessors of this storage declaration. + void visitOpaqueAccessors(llvm::function_ref) const; + void setAccessors(StorageImplInfo storageImpl, SourceLoc lbraceLoc, ArrayRef accessors, SourceLoc rbraceLoc); @@ -4284,6 +4291,9 @@ class AbstractStorageDecl : public ValueDecl { /// \brief Add a synthesized materializeForSet accessor. void setSynthesizedMaterializeForSet(AccessorDecl *materializeForSet); + /// Does this storage require a materializeForSet accessor? + bool requiresMaterializeForSet() const; + SourceRange getBracesRange() const { if (auto info = Accessors.getPointer()) return info->getBracesRange(); diff --git a/include/swift/SIL/SILWitnessVisitor.h b/include/swift/SIL/SILWitnessVisitor.h index 69c0dd3582c0f..a350e3cb7e5f3 100644 --- a/include/swift/SIL/SILWitnessVisitor.h +++ b/include/swift/SIL/SILWitnessVisitor.h @@ -133,15 +133,9 @@ template class SILWitnessVisitor : public ASTVisitor { } void visitAbstractStorageDecl(AbstractStorageDecl *sd) { - asDerived().addMethod(SILDeclRef(sd->getGetter(), - SILDeclRef::Kind::Func)); - if (sd->isSettable(sd->getDeclContext())) { - asDerived().addMethod(SILDeclRef(sd->getSetter(), - SILDeclRef::Kind::Func)); - if (sd->getMaterializeForSetFunc()) - asDerived().addMethod(SILDeclRef(sd->getMaterializeForSetFunc(), - SILDeclRef::Kind::Func)); - } + sd->visitOpaqueAccessors([&](AccessorDecl *accessor) { + asDerived().addMethod(SILDeclRef(accessor, SILDeclRef::Kind::Func)); + }); } void visitConstructorDecl(ConstructorDecl *cd) { diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 90bbb46efcdfb..ffd1d727d4c5e 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1624,6 +1624,56 @@ AbstractStorageDecl::getAccessStrategy(AccessSemantics semantics, llvm_unreachable("bad access semantics"); } +bool AbstractStorageDecl::requiresMaterializeForSet() const { + // Only for mutable storage. + if (!supportsMutation()) + return false; + + // We only need materializeForSet in type contexts. + // TODO: resilient global variables? + auto *dc = getDeclContext(); + if (!dc->isTypeContext()) + return false; + + // Requirements of ObjC protocols don't need materializeForSet. + if (auto protoDecl = dyn_cast(dc)) + if (protoDecl->isObjC()) + return false; + + // Members of structs imported by Clang don't need an eagerly-synthesized + // materializeForSet. + if (auto structDecl = dyn_cast(dc)) + if (structDecl->hasClangNode()) + return false; + + return true; +} + +void AbstractStorageDecl::visitExpectedOpaqueAccessors( + llvm::function_ref visit) const { + // For now, always assume storage declarations should have getters + // instead of read accessors. + visit(AccessorKind::Get); + + if (supportsMutation()) { + // All mutable storage should have a setter. + visit(AccessorKind::Set); + + // Include materializeForSet if necessary. + if (requiresMaterializeForSet()) + visit(AccessorKind::MaterializeForSet); + } +} + +void AbstractStorageDecl::visitOpaqueAccessors( + llvm::function_ref visit) const { + visitExpectedOpaqueAccessors([&](AccessorKind kind) { + auto accessor = getAccessor(kind); + assert(accessor && "didn't have expected opaque accessor"); + visit(accessor); + }); +} + static bool hasPrivateOrFilePrivateFormalAccess(const ValueDecl *D) { return D->hasAccess() && D->getFormalAccess() <= AccessLevel::FilePrivate; } diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 2a282af7de66a..15587458c6215 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -1184,14 +1184,13 @@ void SILGenModule::visitVarDecl(VarDecl *vd) { if (vd->getImplInfo().isSimpleStored()) { // If the global variable has storage, it might also have synthesized // accessors. Emit them here, since they won't appear anywhere else. - if (auto getter = vd->getGetter()) - emitFunction(getter); - if (auto setter = vd->getSetter()) - emitFunction(setter); - if (auto materializeForSet = vd->getMaterializeForSetFunc()) - emitFunction(materializeForSet); + vd->visitExpectedOpaqueAccessors([&](AccessorKind kind) { + auto accessor = vd->getAccessor(kind); + if (accessor) + emitFunction(accessor); + }); } - + tryEmitPropertyDescriptor(vd); } diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index 59d71462b7d32..2b39e5bc38c7a 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -888,30 +888,77 @@ static void addSetterToStorage(TypeChecker &TC, AbstractStorageDecl *storage) { storage->setSynthesizedSetter(setter); } +/// Add a materializeForSet accessor to the given declaration. +static void addMaterializeForSetToStorage(TypeChecker &TC, + AbstractStorageDecl *storage) { + if (TC.Context.getOptionalDecl() == nullptr) { + TC.diagnose(storage->getStartLoc(), diag::optional_intrinsics_not_found); + return; + } + + auto materializeForSet = createMaterializeForSetPrototype( + storage, storage->getGetter(), storage->getSetter(), TC); + + // Install the prototype. + storage->setSynthesizedMaterializeForSet(materializeForSet); +} + + +static void addOpaqueAccessorToStorage(TypeChecker &TC, + AbstractStorageDecl *storage, + AccessorKind kind) { + switch (kind) { + case AccessorKind::Get: + return addGetterToStorage(TC, storage); + + case AccessorKind::Set: + return addSetterToStorage(TC, storage); + + case AccessorKind::MaterializeForSet: + return addMaterializeForSetToStorage(TC, storage); + +#define OPAQUE_ACCESSOR(ID, KEYWORD) +#define ACCESSOR(ID) \ + case AccessorKind::ID: +#include "swift/AST/AccessorKinds.def" + llvm_unreachable("not an opaque accessor"); + } +} + +static void addExpectedOpaqueAccessorsToStorage(TypeChecker &TC, + AbstractStorageDecl *storage) { + storage->visitExpectedOpaqueAccessors([&](AccessorKind kind) { + // If the accessor is already present, there's nothing to do. + if (storage->getAccessor(kind)) + return; + + addOpaqueAccessorToStorage(TC, storage, kind); + }); +} + /// Add trivial accessors to a Stored or Addressed property. static void addTrivialAccessorsToStorage(AbstractStorageDecl *storage, TypeChecker &TC) { assert(!isSynthesizedComputedProperty(storage)); + addExpectedOpaqueAccessorsToStorage(TC, storage); +} - if (!storage->getGetter()) - addGetterToStorage(TC, storage); - - if (storage->supportsMutation()) { - if (!storage->getSetter()) - addSetterToStorage(TC, storage); - - if (!storage->getMaterializeForSetFunc()) - maybeAddMaterializeForSet(storage, TC); +static StorageImplInfo getProtocolStorageImpl(AbstractStorageDecl *storage) { + auto protocol = cast(storage->getDeclContext()); + if (protocol->isObjC()) { + return StorageImplInfo::getComputed(storage->supportsMutation()); + } else { + return StorageImplInfo::getOpaque(storage->supportsMutation()); } } -static void convertStoredVarInProtocolToComputed(VarDecl *VD, TypeChecker &TC) { - if (!VD->getGetter()) { - addGetterToStorage(TC, VD); - } +/// Given a storage declaration in a protocol, set it up with the right +/// StorageImpl and add the right set of opaque accessors. +static void setProtocolStorageImpl(TypeChecker &TC, + AbstractStorageDecl *storage) { + addExpectedOpaqueAccessorsToStorage(TC, storage); - // Okay, we have the getter; make the VD computed. - VD->overwriteImplInfo(StorageImplInfo::getImmutableComputed()); + storage->overwriteImplInfo(getProtocolStorageImpl(storage)); } /// Synthesize the body of a setter which just delegates to a mutable @@ -932,23 +979,6 @@ static void synthesizeModifyCoroutineSetterBody(TypeChecker &TC, setter->getStorage()); } -/// Add a materializeForSet accessor to the given declaration. -static FuncDecl *addMaterializeForSet(AbstractStorageDecl *storage, - TypeChecker &TC) { - if (TC.Context.getOptionalDecl() == nullptr) { - TC.diagnose(storage->getStartLoc(), diag::optional_intrinsics_not_found); - return nullptr; - } - - auto materializeForSet = createMaterializeForSetPrototype( - storage, storage->getGetter(), storage->getSetter(), TC); - - // Install the prototype. - storage->setSynthesizedMaterializeForSet(materializeForSet); - - return materializeForSet; -} - static void convertNSManagedStoredVarToComputed(VarDecl *VD, TypeChecker &TC) { // If it's not still stored, just bail out. if (!VD->getImplInfo().isSimpleStored()) @@ -974,7 +1004,7 @@ static void convertNSManagedStoredVarToComputed(VarDecl *VD, TypeChecker &TC) { // Okay, we have both a getter and setter; overwrite the impl info. VD->overwriteImplInfo(StorageImplInfo::getMutableComputed()); - maybeAddMaterializeForSet(VD, TC); + addExpectedOpaqueAccessorsToStorage(TC, VD); } /// The specified AbstractStorageDecl was just found to satisfy a @@ -984,42 +1014,45 @@ void TypeChecker::synthesizeWitnessAccessorsForStorage( AbstractStorageDecl *requirement, AbstractStorageDecl *storage) { bool addedAccessor = false; - auto flagAddedAccessor = [&](AccessorDecl *accessor, - Optional kind) { + + requirement->visitExpectedOpaqueAccessors([&](AccessorKind kind) { + // If the accessor already exists, we have nothing to do. + if (storage->getAccessor(kind)) + return; + + // Otherwise, synthesize it. + addOpaqueAccessorToStorage(*this, storage, kind); + + // Flag that we've added an accessor. addedAccessor = true; - // Synthesize a trivial body when we create an on-demand accessor. - if (kind && isOnDemandAccessor(accessor->getStorage(), - accessor->getAccessorKind())) { - triggerSynthesis(*this, accessor, *kind); - } - }; + // Trigger synthesize of the accessor body if it's created on-demand. + SynthesizedFunction::Kind synthKind; + switch (kind) { + case AccessorKind::MaterializeForSet: + // No need to do this for materializeForSet, which we always synthesize + // in SILGen. + return; - // Synthesize a getter. - if (!storage->getGetter()) { - addGetterToStorage(*this, storage); - flagAddedAccessor(storage->getGetter(), SynthesizedFunction::Getter); - } + case AccessorKind::Get: + synthKind = SynthesizedFunction::Getter; + break; - // Synthesize a setter if the storage is mutable and the requirement - // needs it. - if (!storage->getSetter() && - storage->supportsMutation() && - requirement->supportsMutation()) { - addSetterToStorage(*this, storage); - flagAddedAccessor(storage->getSetter(), SynthesizedFunction::Setter); - } + case AccessorKind::Set: + synthKind = SynthesizedFunction::Setter; + break; - // @objc protocols don't need a materializeForSet since ObjC doesn't - // have that concept. - bool wantMaterializeForSet = - !requirement->isObjC() && requirement->getSetter(); +#define OPAQUE_ACCESSOR(ID, KEYWORD) +#define ACCESSOR(ID) \ + case AccessorKind::ID: +#include "swift/AST/AccessorKinds.def" + llvm_unreachable("unexpected synthesized accessor"); + } - // If we want wantMaterializeForSet, create it now. - if (wantMaterializeForSet && !storage->getMaterializeForSetFunc()) { - addMaterializeForSet(storage, *this); - flagAddedAccessor(storage->getMaterializeForSetFunc(), None); - } + if (isOnDemandAccessor(storage, kind)) { + triggerSynthesis(*this, storage->getAccessor(kind), synthKind); + } + }); // Cue (delayed) validation of any accessors we just added, just // in case this is coming after the normal delayed validation finished. @@ -1728,38 +1761,6 @@ void TypeChecker::completeLazyVarImplementation(VarDecl *VD) { Storage->overwriteSetterAccess(AccessLevel::Private); } -/// Consider add a materializeForSet accessor to the given storage -/// decl (which has accessors). -void swift::maybeAddMaterializeForSet(AbstractStorageDecl *storage, - TypeChecker &TC) { - assert(storage->getGetter()); - - // Be idempotent. There are a bunch of places where we want to - // ensure that there's a materializeForSet accessor. - if (storage->getMaterializeForSetFunc()) return; - - // Never add materializeForSet to readonly declarations. - if (!storage->getSetter()) return; - - // We only need materializeForSet in type contexts. - auto *dc = storage->getDeclContext(); - if (!dc->isTypeContext()) - return; - - // Requirements of ObjC protocols don't need this. - if (auto protoDecl = dyn_cast(dc)) - if (protoDecl->isObjC()) - return; - - // Members of structs imported by Clang don't need this, because we can - // synthesize it later. - if (auto structDecl = dyn_cast(dc)) - if (structDecl->hasClangNode()) - return; - - addMaterializeForSet(storage, TC); -} - void swift::triggerAccessorSynthesis(TypeChecker &TC, AbstractStorageDecl *storage) { auto VD = dyn_cast(storage); @@ -1959,7 +1960,7 @@ static void maybeAddAccessorsToLazyVariable(TypeChecker &TC, VarDecl *var) { var->overwriteImplInfo(StorageImplInfo::getMutableComputed()); - maybeAddMaterializeForSet(var, TC); + addExpectedOpaqueAccessorsToStorage(TC, var); } /// Try to add the appropriate accessors required a storage declaration. @@ -2008,11 +2009,9 @@ void swift::maybeAddAccessorsToStorage(TypeChecker &TC, diag::protocol_property_must_be_computed_var); else TC.diagnose(var->getLoc(), diag::protocol_property_must_be_computed); - - convertStoredVarInProtocolToComputed(var, TC); } - maybeAddMaterializeForSet(storage, TC); + setProtocolStorageImpl(TC, storage); return; // NSManaged properties on classes require special handling. @@ -2034,7 +2033,7 @@ void swift::maybeAddAccessorsToStorage(TypeChecker &TC, if (auto sourceFile = dc->getParentSourceFile()) if (sourceFile->Kind == SourceFileKind::SIL) { if (storage->getGetter() && storage->getSetter()) { - maybeAddMaterializeForSet(storage, TC); + addExpectedOpaqueAccessorsToStorage(TC, storage); } return; } diff --git a/lib/Sema/CodeSynthesis.h b/lib/Sema/CodeSynthesis.h index 5d38391a60901..73dacb8a53939 100644 --- a/lib/Sema/CodeSynthesis.h +++ b/lib/Sema/CodeSynthesis.h @@ -100,8 +100,6 @@ void makeFinal(ASTContext &ctx, ValueDecl *D); bool checkOverrides(ValueDecl *decl); // These are implemented in CodeSynthesis.cpp. -void maybeAddMaterializeForSet(AbstractStorageDecl *storage, - TypeChecker &TC); void maybeAddAccessorsToStorage(TypeChecker &TC, AbstractStorageDecl *storage); void triggerAccessorSynthesis(TypeChecker &TC, AbstractStorageDecl *storage); diff --git a/lib/TBDGen/TBDGen.cpp b/lib/TBDGen/TBDGen.cpp index 4c0d87804c710..1098bcaad2506 100644 --- a/lib/TBDGen/TBDGen.cpp +++ b/lib/TBDGen/TBDGen.cpp @@ -125,13 +125,12 @@ void TBDGenVisitor::addConformances(DeclContext *DC) { addSymbolIfNecessary(valueReq, witnessDecl); } else if (auto *storage = dyn_cast(valueReq)) { auto witnessStorage = cast(witnessDecl); - if (auto *getter = storage->getGetter()) - addSymbolIfNecessary(getter, witnessStorage->getGetter()); - if (auto *setter = storage->getSetter()) - addSymbolIfNecessary(setter, witnessStorage->getSetter()); - if (auto *materializeForSet = storage->getMaterializeForSetFunc()) - addSymbolIfNecessary(materializeForSet, - witnessStorage->getMaterializeForSetFunc()); + storage->visitOpaqueAccessors([&](AccessorDecl *reqtAccessor) { + auto witnessAccessor = + witnessStorage->getAccessor(reqtAccessor->getAccessorKind()); + assert(witnessAccessor && "no corresponding witness accessor?"); + addSymbolIfNecessary(reqtAccessor, witnessAccessor); + }); } }); }