From 0c5d52d8609aaa3016621887d03571c7582984a5 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 30 Jul 2019 22:24:15 -0400 Subject: [PATCH 1/7] AST: Introduce AbstractStorageDecl::get{Parsed,Opaque}Accessor() Also, change visitOpaqueAccessors() to call getOpaqueAccessor() instead of asserting if the expected accessor does not exist. --- include/swift/AST/Decl.h | 17 ++++++++ lib/AST/ASTPrinter.cpp | 2 +- lib/AST/ASTVerifier.cpp | 52 +++++++++++------------ lib/AST/AccessRequests.cpp | 3 +- lib/AST/Decl.cpp | 37 +++++++++++++---- lib/AST/SwiftNameTranslation.cpp | 2 +- lib/AST/Type.cpp | 2 +- lib/ClangImporter/ImportDecl.cpp | 18 ++++---- lib/IDE/Formatting.cpp | 5 +-- lib/IRGen/GenClass.cpp | 8 ++-- lib/IRGen/GenObjC.cpp | 13 +++--- lib/IRGen/Linking.cpp | 2 +- lib/Index/Index.cpp | 8 +--- lib/ParseSIL/ParseSIL.cpp | 2 +- lib/PrintAsObjC/PrintAsObjC.cpp | 15 ++++--- lib/SIL/SIL.cpp | 10 ++--- lib/SIL/TypeLowering.cpp | 6 ++- lib/SILGen/SILGen.cpp | 13 +++--- lib/SILGen/SILGenApply.cpp | 4 +- lib/SILGen/SILGenExpr.cpp | 6 +-- lib/SILGen/SILGenLValue.cpp | 21 +++++----- lib/SILGen/SILGenType.cpp | 11 ++--- lib/Sema/CSApply.cpp | 6 +-- lib/Sema/CodeSynthesis.cpp | 64 +++++++++++++---------------- lib/Sema/TypeCheckAttr.cpp | 8 ++-- lib/Sema/TypeCheckDecl.cpp | 20 ++++----- lib/Sema/TypeCheckDeclOverride.cpp | 21 +++++----- lib/Sema/TypeCheckProtocol.cpp | 50 ++++++++++------------ lib/Serialization/Serialization.cpp | 9 ---- lib/TBDGen/TBDGen.cpp | 4 +- 30 files changed, 224 insertions(+), 215 deletions(-) diff --git a/include/swift/AST/Decl.h b/include/swift/AST/Decl.h index c2afe755da23e..5ae75346d4cbc 100644 --- a/include/swift/AST/Decl.h +++ b/include/swift/AST/Decl.h @@ -4592,6 +4592,23 @@ class AbstractStorageDecl : public ValueDecl { /// with caution. AccessorDecl *getSynthesizedAccessor(AccessorKind kind) const; + /// Return an accessor part of the set of opaque accessors dictated by the + /// requirements of the ABI. + /// + /// This will synthesize the accessor if one is required but not specified + /// in source; for example, most of the time a mutable property is required + /// to have a 'modify' accessor, but if the property was only written with + /// 'get' and 'set' accessors, 'modify' will be synthesized to call 'get' + /// followed by 'set'. + /// + /// If the accessor is not needed for ABI reasons, this returns nullptr. + /// To ensure an accessor is always returned, use getSynthesizedAccessor(). + AccessorDecl *getOpaqueAccessor(AccessorKind kind) const; + + /// Return an accessor that was written in source. Returns null if the + /// accessor was not explicitly defined by the user. + AccessorDecl *getParsedAccessor(AccessorKind kind) const; + /// Visit all the opaque accessors that this storage is expected to have. void visitExpectedOpaqueAccessors( llvm::function_ref) const; diff --git a/lib/AST/ASTPrinter.cpp b/lib/AST/ASTPrinter.cpp index fa96df6a70bcc..0c087d79645c4 100644 --- a/lib/AST/ASTPrinter.cpp +++ b/lib/AST/ASTPrinter.cpp @@ -1804,7 +1804,7 @@ void PrintAST::printAccessors(const AbstractStorageDecl *ASD) { if ((PrintAbstract || isGetSetImpl()) && !Options.PrintGetSetOnRWProperties && !Options.FunctionDefinitions && - !ASD->getAccessor(AccessorKind::Get)->isMutating() && + !ASD->isGetterMutating() && !ASD->getAccessor(AccessorKind::Set)->isExplicitNonMutating()) { return; } diff --git a/lib/AST/ASTVerifier.cpp b/lib/AST/ASTVerifier.cpp index 9a2828d401e1d..6abc0e3acde5e 100644 --- a/lib/AST/ASTVerifier.cpp +++ b/lib/AST/ASTVerifier.cpp @@ -1876,7 +1876,7 @@ class Verifier : public ASTWalker { if (auto *baseIOT = E->getBase()->getType()->getAs()) { if (!baseIOT->getObjectType()->is()) { auto *VD = dyn_cast(E->getMember().getDecl()); - if (!VD || VD->getAllAccessors().empty()) { + if (!VD || !VD->requiresOpaqueAccessors()) { Out << "member_ref_expr on value of inout type\n"; E->dump(Out); Out << "\n"; @@ -2446,6 +2446,9 @@ class Verifier : public ASTWalker { } void verifyChecked(VarDecl *var) { + if (!var->hasInterfaceType()) + return; + PrettyStackTraceDecl debugStack("verifying VarDecl", var); // Variables must have materializable type. @@ -2472,27 +2475,21 @@ class Verifier : public ASTWalker { } Type typeForAccessors = var->getValueInterfaceType(); - if (!var->getDeclContext()->contextHasLazyGenericEnvironment()) { - typeForAccessors = - var->getDeclContext()->mapTypeIntoContext(typeForAccessors); - if (const FuncDecl *getter = var->getAccessor(AccessorKind::Get)) { - if (getter->getParameters()->size() != 0) { - Out << "property getter has parameters\n"; + if (const FuncDecl *getter = var->getAccessor(AccessorKind::Get)) { + if (getter->getParameters()->size() != 0) { + Out << "property getter has parameters\n"; + abort(); + } + if (getter->hasInterfaceType()) { + Type getterResultType = getter->getResultInterfaceType(); + if (!getterResultType->isEqual(typeForAccessors)) { + Out << "property and getter have mismatched types: '"; + typeForAccessors.print(Out); + Out << "' vs. '"; + getterResultType.print(Out); + Out << "'\n"; abort(); } - if (getter->hasInterfaceType()) { - Type getterResultType = getter->getResultInterfaceType(); - getterResultType = - var->getDeclContext()->mapTypeIntoContext(getterResultType); - if (!getterResultType->isEqual(typeForAccessors)) { - Out << "property and getter have mismatched types: '"; - typeForAccessors.print(Out); - Out << "' vs. '"; - getterResultType.print(Out); - Out << "'\n"; - abort(); - } - } } } @@ -2512,15 +2509,12 @@ class Verifier : public ASTWalker { } const ParamDecl *param = setter->getParameters()->get(0); Type paramType = param->getInterfaceType(); - if (!var->getDeclContext()->contextHasLazyGenericEnvironment()) { - paramType = var->getDeclContext()->mapTypeIntoContext(paramType); - if (!paramType->isEqual(typeForAccessors)) { - Out << "property and setter param have mismatched types:\n"; - typeForAccessors.dump(Out, 2); - Out << "vs.\n"; - paramType.dump(Out, 2); - abort(); - } + if (!paramType->isEqual(typeForAccessors)) { + Out << "property and setter param have mismatched types:\n"; + typeForAccessors.dump(Out, 2); + Out << "vs.\n"; + paramType.dump(Out, 2); + abort(); } } } diff --git a/lib/AST/AccessRequests.cpp b/lib/AST/AccessRequests.cpp index 6789a6c7e9035..befc0fb2d888b 100644 --- a/lib/AST/AccessRequests.cpp +++ b/lib/AST/AccessRequests.cpp @@ -161,8 +161,7 @@ static bool isStoredWithPrivateSetter(VarDecl *VD) { return false; if (VD->isLet() || - (VD->getAccessor(AccessorKind::Set) && - !VD->getAccessor(AccessorKind::Set)->isImplicit())) + VD->getParsedAccessor(AccessorKind::Set)) return false; return true; diff --git a/lib/AST/Decl.cpp b/lib/AST/Decl.cpp index 6112119d38587..c53261df81500 100644 --- a/lib/AST/Decl.cpp +++ b/lib/AST/Decl.cpp @@ -1970,6 +1970,28 @@ AccessorDecl *AbstractStorageDecl::getSynthesizedAccessor(AccessorKind kind) con nullptr); } +AccessorDecl *AbstractStorageDecl::getOpaqueAccessor(AccessorKind kind) const { + auto *accessor = getAccessor(kind); + if (accessor && !accessor->isImplicit()) + return accessor; + + if (!requiresOpaqueAccessors()) + return nullptr; + + if (!requiresOpaqueAccessor(kind)) + return nullptr; + + return getSynthesizedAccessor(kind); +} + +AccessorDecl *AbstractStorageDecl::getParsedAccessor(AccessorKind kind) const { + auto *accessor = getAccessor(kind); + if (accessor && !accessor->isImplicit()) + return accessor; + + return nullptr; +} + void AbstractStorageDecl::visitExpectedOpaqueAccessors( llvm::function_ref visit) const { if (!requiresOpaqueAccessors()) @@ -1993,8 +2015,9 @@ void AbstractStorageDecl::visitExpectedOpaqueAccessors( void AbstractStorageDecl::visitOpaqueAccessors( llvm::function_ref visit) const { visitExpectedOpaqueAccessors([&](AccessorKind kind) { - auto accessor = getAccessor(kind); - assert(accessor && "didn't have expected opaque accessor"); + auto accessor = getSynthesizedAccessor(kind); + assert(!accessor->hasForcedStaticDispatch() && + "opaque accessor with forced static dispatch?"); visit(accessor); }); } @@ -4029,7 +4052,7 @@ ClassDecl::findOverridingDecl(const AbstractFunctionDecl *Method) const { auto *Storage = Accessor->getStorage(); if (auto *Derived = ::findOverridingDecl(this, Storage)) { auto *DerivedStorage = cast(Derived); - return DerivedStorage->getAccessor(Accessor->getAccessorKind()); + return DerivedStorage->getOpaqueAccessor(Accessor->getAccessorKind()); } return nullptr; @@ -4651,9 +4674,9 @@ bool AbstractStorageDecl::hasPrivateAccessor() const { } bool AbstractStorageDecl::hasDidSetOrWillSetDynamicReplacement() const { - if (auto *func = getAccessor(AccessorKind::DidSet)) + if (auto *func = getParsedAccessor(AccessorKind::DidSet)) return func->getAttrs().hasAttribute(); - if (auto *func = getAccessor(AccessorKind::WillSet)) + if (auto *func = getParsedAccessor(AccessorKind::WillSet)) return func->getAttrs().hasAttribute(); return false; } @@ -4855,7 +4878,7 @@ getNameFromObjcAttribute(const ObjCAttr *attr, DeclName preferredName) { ObjCSelector AbstractStorageDecl::getObjCGetterSelector(Identifier preferredName) const { // If the getter has an @objc attribute with a name, use that. - if (auto getter = getAccessor(AccessorKind::Get)) { + if (auto getter = getParsedAccessor(AccessorKind::Get)) { if (auto name = getNameFromObjcAttribute(getter->getAttrs(). getAttribute(), preferredName)) return *name; @@ -4885,7 +4908,7 @@ AbstractStorageDecl::getObjCGetterSelector(Identifier preferredName) const { ObjCSelector AbstractStorageDecl::getObjCSetterSelector(Identifier preferredName) const { // If the setter has an @objc attribute with a name, use that. - auto setter = getAccessor(AccessorKind::Set); + auto setter = getParsedAccessor(AccessorKind::Set); auto objcAttr = setter ? setter->getAttrs().getAttribute() : nullptr; if (auto name = getNameFromObjcAttribute(objcAttr, DeclName(preferredName))) { diff --git a/lib/AST/SwiftNameTranslation.cpp b/lib/AST/SwiftNameTranslation.cpp index 28b1cd0562bbd..2a1d648e73507 100644 --- a/lib/AST/SwiftNameTranslation.cpp +++ b/lib/AST/SwiftNameTranslation.cpp @@ -114,7 +114,7 @@ getObjCNameForSwiftDecl(const ValueDecl *VD, DeclName PreferredName){ return {BaseName, ObjCSelector()}; return {VAD->getObjCPropertyName(), ObjCSelector()}; } else if (auto *SD = dyn_cast(VD)) { - return getObjCNameForSwiftDecl(SD->getAccessor(AccessorKind::Get), + return getObjCNameForSwiftDecl(SD->getParsedAccessor(AccessorKind::Get), PreferredName); } else if (auto *EL = dyn_cast(VD)) { SmallString<64> Buffer; diff --git a/lib/AST/Type.cpp b/lib/AST/Type.cpp index 14d48897d4f81..cfb76673bbde3 100644 --- a/lib/AST/Type.cpp +++ b/lib/AST/Type.cpp @@ -2556,7 +2556,7 @@ bool ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution( return true; } } else if (auto *asd = dyn_cast(namingDecl)) { - auto *getter = asd->getAccessor(AccessorKind::Get); + auto *getter = asd->getOpaqueAccessor(AccessorKind::Get); if (getter && getter->getResilienceExpansion() == ResilienceExpansion::Minimal) { return true; diff --git a/lib/ClangImporter/ImportDecl.cpp b/lib/ClangImporter/ImportDecl.cpp index fbbe5cb92d107..e9b64689a60d1 100644 --- a/lib/ClangImporter/ImportDecl.cpp +++ b/lib/ClangImporter/ImportDecl.cpp @@ -3628,13 +3628,13 @@ namespace { case ImportedAccessorKind::PropertyGetter: { auto property = getImplicitProperty(importedName, decl); if (!property) return nullptr; - return property->getAccessor(AccessorKind::Get); + return property->getParsedAccessor(AccessorKind::Get); } case ImportedAccessorKind::PropertySetter: auto property = getImplicitProperty(importedName, decl); if (!property) return nullptr; - return property->getAccessor(AccessorKind::Set); + return property->getParsedAccessor(AccessorKind::Set); } return importFunctionDecl(decl, importedName, correctSwiftName, None); @@ -5010,8 +5010,8 @@ namespace { // Only record overrides of class members. if (overridden) { result->setOverriddenDecl(overridden); - getter->setOverriddenDecl(overridden->getAccessor(AccessorKind::Get)); - if (auto parentSetter = overridden->getAccessor(AccessorKind::Set)) + getter->setOverriddenDecl(overridden->getParsedAccessor(AccessorKind::Get)); + if (auto parentSetter = overridden->getParsedAccessor(AccessorKind::Set)) if (setter) setter->setOverriddenDecl(parentSetter); } @@ -6516,10 +6516,10 @@ void SwiftDeclConverter::recordObjCOverride(SubscriptDecl *subscript) { // The index types match. This is an override, so mark it as such. subscript->setOverriddenDecl(parentSub); - auto getterThunk = subscript->getAccessor(AccessorKind::Get); - getterThunk->setOverriddenDecl(parentSub->getAccessor(AccessorKind::Get)); - if (auto parentSetter = parentSub->getAccessor(AccessorKind::Set)) { - if (auto setterThunk = subscript->getAccessor(AccessorKind::Set)) + auto getterThunk = subscript->getParsedAccessor(AccessorKind::Get); + getterThunk->setOverriddenDecl(parentSub->getParsedAccessor(AccessorKind::Get)); + if (auto parentSetter = parentSub->getParsedAccessor(AccessorKind::Set)) { + if (auto setterThunk = subscript->getParsedAccessor(AccessorKind::Set)) setterThunk->setOverriddenDecl(parentSetter); } @@ -8397,10 +8397,8 @@ ClangImporter::Implementation::createConstant(Identifier name, DeclContext *dc, func->computeType(); func->setAccess(getOverridableAccessLevel(dc)); func->setValidationToChecked(); - func->setImplicit(); func->setIsObjC(false); func->setIsDynamic(false); - func->setIsTransparent(false); func->setBodySynthesizer(synthesizeConstantGetterBody, ConstantGetterBodyContextData(valueExpr, convertKind) diff --git a/lib/IDE/Formatting.cpp b/lib/IDE/Formatting.cpp index aafdf2e623f0d..89a27d52979ec 100644 --- a/lib/IDE/Formatting.cpp +++ b/lib/IDE/Formatting.cpp @@ -268,9 +268,8 @@ class FormatContext { // return 0; <- No indentation added because of the getter. // } if (auto VD = dyn_cast_or_null(Cursor->getAsDecl())) { - if (auto Getter = VD->getAccessor(AccessorKind::Get)) { - if (!Getter->isImplicit() && - Getter->getAccessorKeywordLoc().isInvalid()) { + if (auto Getter = VD->getParsedAccessor(AccessorKind::Get)) { + if (Getter->getAccessorKeywordLoc().isInvalid()) { LineAndColumn = ParentLineAndColumn; continue; } diff --git a/lib/IRGen/GenClass.cpp b/lib/IRGen/GenClass.cpp index e181b522a6c65..bd5bc50bdad7a 100644 --- a/lib/IRGen/GenClass.cpp +++ b/lib/IRGen/GenClass.cpp @@ -1793,7 +1793,7 @@ namespace { } // Don't emit descriptors for properties without accessors. - auto getter = var->getAccessor(AccessorKind::Get); + auto getter = var->getOpaqueAccessor(AccessorKind::Get); if (!getter) return; @@ -1804,7 +1804,7 @@ namespace { auto &methods = getMethodList(var); methods.push_back(getter); - if (auto setter = var->getAccessor(AccessorKind::Set)) + if (auto setter = var->getOpaqueAccessor(AccessorKind::Set)) methods.push_back(setter); } } @@ -2030,13 +2030,13 @@ namespace { void visitSubscriptDecl(SubscriptDecl *subscript) { if (!requiresObjCSubscriptDescriptor(IGM, subscript)) return; - auto getter = subscript->getAccessor(AccessorKind::Get); + auto getter = subscript->getOpaqueAccessor(AccessorKind::Get); if (!getter) return; auto &methods = getMethodList(subscript); methods.push_back(getter); - if (auto setter = subscript->getAccessor(AccessorKind::Set)) + if (auto setter = subscript->getOpaqueAccessor(AccessorKind::Set)) methods.push_back(setter); } }; diff --git a/lib/IRGen/GenObjC.cpp b/lib/IRGen/GenObjC.cpp index 73fcca83d8d02..98ae4cfec3b95 100644 --- a/lib/IRGen/GenObjC.cpp +++ b/lib/IRGen/GenObjC.cpp @@ -923,7 +923,7 @@ static llvm::Constant *getObjCGetterPointer(IRGenModule &IGM, if (isa(property->getDeclContext())) return llvm::ConstantPointerNull::get(IGM.Int8PtrTy); - SILDeclRef getter = SILDeclRef(property->getAccessor(AccessorKind::Get), + SILDeclRef getter = SILDeclRef(property->getOpaqueAccessor(AccessorKind::Get), SILDeclRef::Kind::Func) .asForeign(); @@ -944,7 +944,7 @@ static llvm::Constant *getObjCSetterPointer(IRGenModule &IGM, assert(property->isSettable(property->getDeclContext()) && "property is not settable?!"); - SILDeclRef setter = SILDeclRef(property->getAccessor(AccessorKind::Set), + SILDeclRef setter = SILDeclRef(property->getOpaqueAccessor(AccessorKind::Set), SILDeclRef::Kind::Func) .asForeign(); return findSwiftAsObjCThunk(IGM, setter, silFn); @@ -1013,8 +1013,7 @@ static CanSILFunctionType getObjCMethodType(IRGenModule &IGM, static clang::CanQualType getObjCPropertyType(IRGenModule &IGM, VarDecl *property) { // Use the lowered return type of the foreign getter. - auto getter = property->getAccessor(AccessorKind::Get); - assert(getter); + auto getter = property->getOpaqueAccessor(AccessorKind::Get); CanSILFunctionType methodTy = getObjCMethodType(IGM, getter); return IGM.getClangType( methodTy->getFormalCSemanticResult().getASTType()); @@ -1173,7 +1172,7 @@ SILFunction *irgen::emitObjCGetterDescriptorParts(IRGenModule &IGM, Selector getterSel(subscript, Selector::ForGetter); selectorRef = IGM.getAddrOfObjCMethodName(getterSel.str()); auto methodTy = getObjCMethodType(IGM, - subscript->getAccessor(AccessorKind::Get)); + subscript->getOpaqueAccessor(AccessorKind::Get)); atEncoding = getObjCEncodingForMethodType(IGM, methodTy, /*extended*/false); SILFunction *silFn = nullptr; impl = getObjCGetterPointer(IGM, subscript, silFn); @@ -1250,7 +1249,7 @@ SILFunction *irgen::emitObjCSetterDescriptorParts(IRGenModule &IGM, Selector setterSel(subscript, Selector::ForSetter); selectorRef = IGM.getAddrOfObjCMethodName(setterSel.str()); auto methodTy = getObjCMethodType(IGM, - subscript->getAccessor(AccessorKind::Set)); + subscript->getOpaqueAccessor(AccessorKind::Set)); atEncoding = getObjCEncodingForMethodType(IGM, methodTy, /*extended*/false); SILFunction *silFn = nullptr; impl = getObjCSetterPointer(IGM, subscript, silFn); @@ -1401,7 +1400,7 @@ bool irgen::requiresObjCPropertyDescriptor(IRGenModule &IGM, // Don't generate a descriptor for a property without any accessors. // This is only possible in SIL files because Sema will normally // implicitly synthesize accessors for @objc properties. - return property->isObjC() && property->getAccessor(AccessorKind::Get); + return property->isObjC() && property->requiresOpaqueAccessors(); } bool irgen::requiresObjCSubscriptDescriptor(IRGenModule &IGM, diff --git a/lib/IRGen/Linking.cpp b/lib/IRGen/Linking.cpp index 0a6e7864f9258..be2b9485b6729 100644 --- a/lib/IRGen/Linking.cpp +++ b/lib/IRGen/Linking.cpp @@ -567,7 +567,7 @@ SILLinkage LinkEntity::getLinkage(ForDefinition_t forDefinition) const { // property itself (for instance, with a private/internal property whose // accessor is @inlinable or @usableFromInline) auto getterDecl = cast(getDecl()) - ->getAccessor(AccessorKind::Get); + ->getOpaqueAccessor(AccessorKind::Get); return getSILLinkage(getDeclLinkage(getterDecl), forDefinition); } diff --git a/lib/Index/Index.cpp b/lib/Index/Index.cpp index 49305a7177b71..cfc5e93e6f978 100644 --- a/lib/Index/Index.cpp +++ b/lib/Index/Index.cpp @@ -1062,14 +1062,10 @@ bool IndexSwiftASTWalker::report(ValueDecl *D) { if (startEntityDecl(D)) { // Pass accessors. if (auto StoreD = dyn_cast(D)) { - auto isNullOrImplicit = [](const Decl *D) -> bool { - return !D || D->isImplicit(); - }; - bool usedPseudoAccessors = false; if (isa(D) && - isNullOrImplicit(StoreD->getAccessor(AccessorKind::Get)) && - isNullOrImplicit(StoreD->getAccessor(AccessorKind::Set))) { + !StoreD->getParsedAccessor(AccessorKind::Get) && + !StoreD->getParsedAccessor(AccessorKind::Set)) { usedPseudoAccessors = true; auto VarD = cast(D); // No actual getter or setter, pass 'pseudo' accessors. diff --git a/lib/ParseSIL/ParseSIL.cpp b/lib/ParseSIL/ParseSIL.cpp index 6d4e651d28318..c9a8d791ad895 100644 --- a/lib/ParseSIL/ParseSIL.cpp +++ b/lib/ParseSIL/ParseSIL.cpp @@ -1447,7 +1447,7 @@ bool SILParser::parseSILDeclRef(SILDeclRef &Result, size_t destI = 0; for (size_t srcI = 0, e = values.size(); srcI != e; ++srcI) { if (auto storage = dyn_cast(values[srcI])) - if (auto accessor = storage->getAccessor(*accessorKind)) + if (auto accessor = storage->getOpaqueAccessor(*accessorKind)) values[destI++] = accessor; } values.resize(destI); diff --git a/lib/PrintAsObjC/PrintAsObjC.cpp b/lib/PrintAsObjC/PrintAsObjC.cpp index e46ecb0f19208..241737360afbe 100644 --- a/lib/PrintAsObjC/PrintAsObjC.cpp +++ b/lib/PrintAsObjC/PrintAsObjC.cpp @@ -1256,23 +1256,25 @@ class ObjCPrinter : private DeclVisitor, printAvailability(VD); + auto *getter = VD->getOpaqueAccessor(AccessorKind::Get); + auto *setter = VD->getOpaqueAccessor(AccessorKind::Set); + os << ";"; if (VD->isStatic()) { os << ")\n"; // Older Clangs don't support class properties, so print the accessors as // well. This is harmless. - printAbstractFunctionAsMethod(VD->getAccessor(AccessorKind::Get), true); + printAbstractFunctionAsMethod(getter, true); if (isSettable) { - auto *setter = VD->getAccessor(AccessorKind::Set); assert(setter && "settable ObjC property missing setter decl"); printAbstractFunctionAsMethod(setter, true); } } else { os << "\n"; if (looksLikeInitMethod(VD->getObjCGetterSelector())) - printAbstractFunctionAsMethod(VD->getAccessor(AccessorKind::Get), false); + printAbstractFunctionAsMethod(getter, false); if (isSettable && looksLikeInitMethod(VD->getObjCSetterSelector())) - printAbstractFunctionAsMethod(VD->getAccessor(AccessorKind::Set), false); + printAbstractFunctionAsMethod(setter, false); } } @@ -1287,9 +1289,10 @@ class ObjCPrinter : private DeclVisitor, isNSUIntegerSubscript = isNSUInteger(indexParam->getType()); } - printAbstractFunctionAsMethod(SD->getAccessor(AccessorKind::Get), false, + auto *getter = SD->getOpaqueAccessor(AccessorKind::Get); + printAbstractFunctionAsMethod(getter, false, isNSUIntegerSubscript); - if (auto setter = SD->getAccessor(AccessorKind::Set)) + if (auto *setter = SD->getOpaqueAccessor(AccessorKind::Set)) printAbstractFunctionAsMethod(setter, false, isNSUIntegerSubscript); } diff --git a/lib/SIL/SIL.cpp b/lib/SIL/SIL.cpp index 7accc9f56fbb5..3d12371f912bf 100644 --- a/lib/SIL/SIL.cpp +++ b/lib/SIL/SIL.cpp @@ -213,9 +213,10 @@ bool AbstractStorageDecl::exportsPropertyDescriptor() const { if (isa(getDeclContext())) return false; - // Any property that's potentially resilient should have accessors - // synthesized. - if (!getAccessor(AccessorKind::Get)) + // FIXME: We should support properties and subscripts with '_read' accessors; + // 'get' is not part of the opaque accessor set there. + auto *getter = getOpaqueAccessor(AccessorKind::Get); + if (!getter) return false; // If the getter is mutating, we cannot form a keypath to it at all. @@ -231,8 +232,7 @@ bool AbstractStorageDecl::exportsPropertyDescriptor() const { // then we still do. // Check the linkage of the declaration. - auto getter = SILDeclRef(getAccessor(AccessorKind::Get)); - auto getterLinkage = getter.getLinkage(ForDefinition); + auto getterLinkage = SILDeclRef(getter).getLinkage(ForDefinition); switch (getterLinkage) { case SILLinkage::Public: diff --git a/lib/SIL/TypeLowering.cpp b/lib/SIL/TypeLowering.cpp index a43a16fe97626..45c26a6ef7b50 100644 --- a/lib/SIL/TypeLowering.cpp +++ b/lib/SIL/TypeLowering.cpp @@ -2184,7 +2184,8 @@ TypeConverter::getLoweredLocalCaptures(AnyFunctionRef fn) { // of its accessors. if (auto capturedVar = dyn_cast(capture.getDecl())) { auto collectAccessorCaptures = [&](AccessorKind kind) { - collectFunctionCaptures(capturedVar->getAccessor(kind)); + if (auto *accessor = capturedVar->getParsedAccessor(kind)) + collectFunctionCaptures(accessor); }; if (!capture.isDirect()) { @@ -2212,6 +2213,9 @@ TypeConverter::getLoweredLocalCaptures(AnyFunctionRef fn) { case WriteImplKind::Stored: break; case WriteImplKind::StoredWithObservers: + collectAccessorCaptures(AccessorKind::WillSet); + collectAccessorCaptures(AccessorKind::DidSet); + break; case WriteImplKind::Set: collectAccessorCaptures(AccessorKind::Set); break; diff --git a/lib/SILGen/SILGen.cpp b/lib/SILGen/SILGen.cpp index 24fbc52a37a47..93f5b0be803b4 100644 --- a/lib/SILGen/SILGen.cpp +++ b/lib/SILGen/SILGen.cpp @@ -1240,7 +1240,7 @@ void SILGenModule::emitObjCMethodThunk(FuncDecl *method) { } void SILGenModule::emitObjCPropertyMethodThunks(AbstractStorageDecl *prop) { - auto *getter = prop->getAccessor(AccessorKind::Get); + auto *getter = prop->getOpaqueAccessor(AccessorKind::Get); // If we don't actually need an entry point for the getter, do nothing. if (!getter || !requiresObjCMethodEntryPoint(getter)) @@ -1270,7 +1270,7 @@ void SILGenModule::emitObjCPropertyMethodThunks(AbstractStorageDecl *prop) { return; // FIXME: Add proper location. - auto *setter = prop->getAccessor(AccessorKind::Set); + auto *setter = prop->getOpaqueAccessor(AccessorKind::Set); auto setterRef = SILDeclRef(setter, SILDeclRef::Kind::Func).asForeign(); SILFunction *f = getFunction(setterRef, ForDefinition); @@ -1396,11 +1396,12 @@ static bool canStorageUseTrivialDescriptor(SILGenModule &SGM, // property in a fixed-layout type. return !decl->isFormallyResilient(); } + // If the type is computed and doesn't have a setter that's hidden from // the public, then external components can form the canonical key path // without our help. - auto setter = decl->getAccessor(AccessorKind::Set); - if (setter == nullptr) + auto *setter = decl->getOpaqueAccessor(AccessorKind::Set); + if (!setter) return true; if (setter->getFormalAccessScope(nullptr, true).isPublic()) @@ -1415,8 +1416,8 @@ static bool canStorageUseTrivialDescriptor(SILGenModule &SGM, // or a fixed-layout type may not have been. // Without availability information, only get-only computed properties // can resiliently use trivial descriptors. - return !SGM.canStorageUseStoredKeyPathComponent(decl, expansion) - && decl->getAccessor(AccessorKind::Set) == nullptr; + return (!SGM.canStorageUseStoredKeyPathComponent(decl, expansion) && + !decl->supportsMutation()); } void SILGenModule::tryEmitPropertyDescriptor(AbstractStorageDecl *decl) { diff --git a/lib/SILGen/SILGenApply.cpp b/lib/SILGen/SILGenApply.cpp index 495dae4aa9001..9adf84323a721 100644 --- a/lib/SILGen/SILGenApply.cpp +++ b/lib/SILGen/SILGenApply.cpp @@ -5811,7 +5811,7 @@ RValue SILGenFunction::emitDynamicMemberRefExpr(DynamicMemberRefExpr *e, // Create the branch. FuncDecl *memberFunc; if (auto *VD = dyn_cast(e->getMember().getDecl())) { - memberFunc = VD->getAccessor(AccessorKind::Get); + memberFunc = VD->getOpaqueAccessor(AccessorKind::Get); memberMethodTy = FunctionType::get({}, memberMethodTy); } else memberFunc = cast(e->getMember().getDecl()); @@ -5920,7 +5920,7 @@ RValue SILGenFunction::emitDynamicSubscriptExpr(DynamicSubscriptExpr *e, // Create the branch. auto subscriptDecl = cast(e->getMember().getDecl()); - auto member = SILDeclRef(subscriptDecl->getAccessor(AccessorKind::Get), + auto member = SILDeclRef(subscriptDecl->getOpaqueAccessor(AccessorKind::Get), SILDeclRef::Kind::Func) .asForeign(); B.createDynamicMethodBranch(e, base, member, hasMemberBB, noMemberBB); diff --git a/lib/SILGen/SILGenExpr.cpp b/lib/SILGen/SILGenExpr.cpp index 5c7f47c83b7ea..c143cd28eb1b7 100644 --- a/lib/SILGen/SILGenExpr.cpp +++ b/lib/SILGen/SILGenExpr.cpp @@ -2604,9 +2604,9 @@ loadIndexValuesForKeyPathComponent(SILGenFunction &SGF, SILLocation loc, static AccessorDecl * getRepresentativeAccessorForKeyPath(AbstractStorageDecl *storage) { if (storage->requiresOpaqueGetter()) - return storage->getAccessor(AccessorKind::Get); + return storage->getOpaqueAccessor(AccessorKind::Get); assert(storage->requiresOpaqueReadCoroutine()); - return storage->getAccessor(AccessorKind::Read); + return storage->getOpaqueAccessor(AccessorKind::Read); } static SILFunction *getOrCreateKeyPathGetter(SILGenModule &SGM, @@ -2755,7 +2755,7 @@ static SILFunction *getOrCreateKeyPathSetter(SILGenModule &SGM, // back to the declaration whose setter introduced the witness table // entry. if (isa(property->getDeclContext())) { - auto setter = property->getAccessor(AccessorKind::Set); + auto setter = property->getOpaqueAccessor(AccessorKind::Set); if (!SILDeclRef::requiresNewWitnessTableEntry(setter)) { // Find the setter that does have a witness table entry. auto wtableSetter = diff --git a/lib/SILGen/SILGenLValue.cpp b/lib/SILGen/SILGenLValue.cpp index afac9b7250c65..22ed222d99c57 100644 --- a/lib/SILGen/SILGenLValue.cpp +++ b/lib/SILGen/SILGenLValue.cpp @@ -1307,11 +1307,10 @@ namespace { // If we have a nonmutating setter on a value type, the call // captures all of 'self' and we cannot rewrite an assignment // into an initialization. - auto setter = VD->getAccessor(AccessorKind::Set); - if (setter->isNonMutating() && - setter->getDeclContext()->getSelfNominalTypeDecl() && - setter->isInstanceMember() && - !setter->getDeclContext()->getDeclaredInterfaceType() + if (!VD->isSetterMutating() && + VD->getDeclContext()->getSelfNominalTypeDecl() && + VD->isInstanceMember() && + !VD->getDeclContext()->getDeclaredInterfaceType() ->hasReferenceSemantics()) { return false; } @@ -2485,7 +2484,7 @@ namespace { void emitUsingAccessor(AccessorKind accessorKind, bool isDirect) { auto accessor = - SGF.SGM.getAccessorDeclRef(Storage->getAccessor(accessorKind)); + SGF.SGM.getAccessorDeclRef(Storage->getOpaqueAccessor(accessorKind)); switch (accessorKind) { case AccessorKind::Get: @@ -2915,7 +2914,7 @@ static SGFAccessKind getBaseAccessKind(SILGenModule &SGM, case AccessStrategy::DirectToAccessor: case AccessStrategy::DispatchToAccessor: { - auto accessor = member->getAccessor(strategy.getAccessor()); + auto accessor = member->getOpaqueAccessor(strategy.getAccessor()); return getBaseAccessKindForAccessor(SGM, accessor, baseFormalType); } @@ -2968,10 +2967,10 @@ LValue SILGenLValue::visitMemberRefExpr(MemberRefExpr *e, // dynamic replacement directly call the implementation. if (isContextRead && isOnSelfParameter && strategy.hasAccessor() && strategy.getAccessor() == AccessorKind::Get && - var->getAccessor(AccessorKind::Read)) { + var->getOpaqueAccessor(AccessorKind::Read)) { bool isObjC = false; auto readAccessor = - SGF.SGM.getAccessorDeclRef(var->getAccessor(AccessorKind::Read)); + SGF.SGM.getAccessorDeclRef(var->getOpaqueAccessor(AccessorKind::Read)); if (isCallToReplacedInDynamicReplacement( SGF, readAccessor.getAbstractFunctionDecl(), isObjC)) { accessSemantics = AccessSemantics::DirectToImplementation; @@ -3139,10 +3138,10 @@ LValue SILGenLValue::visitSubscriptExpr(SubscriptExpr *e, // dynamic replacement directly call the implementation. if (isContextRead && isOnSelfParameter && strategy.hasAccessor() && strategy.getAccessor() == AccessorKind::Get && - decl->getAccessor(AccessorKind::Read)) { + decl->getOpaqueAccessor(AccessorKind::Read)) { bool isObjC = false; auto readAccessor = - SGF.SGM.getAccessorDeclRef(decl->getAccessor(AccessorKind::Read)); + SGF.SGM.getAccessorDeclRef(decl->getOpaqueAccessor(AccessorKind::Read)); if (isCallToReplacedInDynamicReplacement( SGF, readAccessor.getAbstractFunctionDecl(), isObjC)) { accessSemantics = AccessSemantics::DirectToImplementation; diff --git a/lib/SILGen/SILGenType.cpp b/lib/SILGen/SILGenType.cpp index c5d4fda7e7903..045d65b6f2c37 100644 --- a/lib/SILGen/SILGenType.cpp +++ b/lib/SILGen/SILGenType.cpp @@ -358,11 +358,12 @@ template class SILGenWitnessTable : public SILWitnessVisitor { return asDerived().addMissingMethod(requirementRef); auto witnessStorage = cast(witness.getDecl()); - auto witnessAccessor = - witnessStorage->getAccessor(reqAccessor->getAccessorKind()); - if (!witnessAccessor) + if (reqAccessor->isSetter() && !witnessStorage->supportsMutation()) return asDerived().addMissingMethod(requirementRef); + auto witnessAccessor = + witnessStorage->getSynthesizedAccessor(reqAccessor->getAccessorKind()); + return addMethodImplementation(requirementRef, SILDeclRef(witnessAccessor, SILDeclRef::Kind::Func), @@ -1120,8 +1121,8 @@ class SILGenExtension : public TypeMemberVisitor { if (hasDidSetOrWillSetDynamicReplacement && isa(storage->getDeclContext()) && - fd != storage->getAccessor(AccessorKind::WillSet) && - fd != storage->getAccessor(AccessorKind::DidSet)) + fd != storage->getParsedAccessor(AccessorKind::WillSet) && + fd != storage->getParsedAccessor(AccessorKind::DidSet)) return; } SGM.emitFunction(fd); diff --git a/lib/Sema/CSApply.cpp b/lib/Sema/CSApply.cpp index 8db5218d4bad4..a16ccdf9d861e 100644 --- a/lib/Sema/CSApply.cpp +++ b/lib/Sema/CSApply.cpp @@ -4085,12 +4085,12 @@ namespace { // The property is non-settable, so add "getter:". primaryDiag.fixItInsert(modifierLoc, "getter: "); E->overrideObjCSelectorKind(ObjCSelectorExpr::Getter, modifierLoc); - method = var->getAccessor(AccessorKind::Get); + method = var->getOpaqueAccessor(AccessorKind::Get); break; } case ObjCSelectorExpr::Getter: - method = var->getAccessor(AccessorKind::Get); + method = var->getOpaqueAccessor(AccessorKind::Get); break; case ObjCSelectorExpr::Setter: @@ -4111,7 +4111,7 @@ namespace { return E; } - method = var->getAccessor(AccessorKind::Set); + method = var->getOpaqueAccessor(AccessorKind::Set); break; } } else { diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index 24235a9349fbd..513db05b7e1ed 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -165,8 +165,6 @@ static void finishImplicitAccessor(AccessorDecl *accessor, static AccessorDecl *createGetterPrototype(AbstractStorageDecl *storage, ASTContext &ctx) { - assert(!storage->getAccessor(AccessorKind::Get)); - SourceLoc loc = storage->getLoc(); GenericEnvironment *genericEnvironmentOfLazyAccessor = nullptr; @@ -244,7 +242,6 @@ static AccessorDecl *createGetterPrototype(AbstractStorageDecl *storage, static AccessorDecl *createSetterPrototype(AbstractStorageDecl *storage, ASTContext &ctx, AccessorDecl *getter = nullptr) { - assert(!storage->getAccessor(AccessorKind::Set)); assert(storage->supportsMutation()); SourceLoc loc = storage->getLoc(); @@ -370,8 +367,8 @@ IsAccessorTransparentRequest::evaluate(Evaluator &evaluator, // FIXME: This should be folded into the WriteImplKind below. if (auto var = dyn_cast(storage)) { if (var->hasAttachedPropertyWrapper()) { - if (var->getAccessor(AccessorKind::DidSet) || - var->getAccessor(AccessorKind::WillSet)) + if (var->getParsedAccessor(AccessorKind::DidSet) || + var->getParsedAccessor(AccessorKind::WillSet)) return false; break; @@ -463,11 +460,11 @@ createCoroutineAccessorPrototype(AbstractStorageDecl *storage, // the storage (and its getters/setters if it has them). SmallVector asAvailableAs; asAvailableAs.push_back(storage); - if (FuncDecl *getter = storage->getAccessor(AccessorKind::Get)) { + if (FuncDecl *getter = storage->getParsedAccessor(AccessorKind::Get)) { asAvailableAs.push_back(getter); } if (kind == AccessorKind::Modify) { - if (FuncDecl *setter = storage->getAccessor(AccessorKind::Set)) { + if (FuncDecl *setter = storage->getParsedAccessor(AccessorKind::Set)) { asAvailableAs.push_back(setter); } } @@ -1148,7 +1145,7 @@ synthesizeInheritedGetterBody(AccessorDecl *getter, ASTContext &ctx) { /// Synthesize the body of a getter which just delegates to an addressor. static std::pair synthesizeAddressedGetterBody(AccessorDecl *getter, ASTContext &ctx) { - assert(getter->getStorage()->getAccessor(AccessorKind::Address)); + assert(getter->getStorage()->getParsedAccessor(AccessorKind::Address)); // This should call the addressor. return synthesizeTrivialGetterBody(getter, TargetImpl::Implementation, ctx); @@ -1158,7 +1155,7 @@ synthesizeAddressedGetterBody(AccessorDecl *getter, ASTContext &ctx) { /// coroutine accessor. static std::pair synthesizeReadCoroutineGetterBody(AccessorDecl *getter, ASTContext &ctx) { - assert(getter->getStorage()->getAccessor(AccessorKind::Read)); + assert(getter->getStorage()->getParsedAccessor(AccessorKind::Read)); // This should call the read coroutine. return synthesizeTrivialGetterBody(getter, TargetImpl::Implementation, ctx); @@ -1399,7 +1396,7 @@ synthesizeObservedSetterBody(AccessorDecl *Set, TargetImpl target, // TODO: check the body of didSet to only do this load (which may call the // superclass getter) if didSet takes an argument. VarDecl *OldValue = nullptr; - if (VD->getAccessor(AccessorKind::DidSet)) { + if (VD->getParsedAccessor(AccessorKind::DidSet)) { Expr *OldValueExpr = buildStorageReference(Set, VD, target, /*isLValue=*/true, Ctx); OldValueExpr = new (Ctx) LoadExpr(OldValueExpr, VD->getType()); @@ -1416,7 +1413,7 @@ synthesizeObservedSetterBody(AccessorDecl *Set, TargetImpl target, SetterBody.push_back(OldValue); } - if (auto willSet = VD->getAccessor(AccessorKind::WillSet)) + if (auto willSet = VD->getParsedAccessor(AccessorKind::WillSet)) callObserver(willSet, ValueDecl); // Create an assignment into the storage or call to superclass setter. @@ -1425,7 +1422,7 @@ synthesizeObservedSetterBody(AccessorDecl *Set, TargetImpl target, createPropertyStoreOrCallSuperclassSetter(Set, ValueDRE, VD, target, SetterBody, Ctx); - if (auto didSet = VD->getAccessor(AccessorKind::DidSet)) + if (auto didSet = VD->getParsedAccessor(AccessorKind::DidSet)) callObserver(didSet, OldValue); return { BraceStmt::create(Ctx, Loc, SetterBody, Loc, true), @@ -1792,11 +1789,9 @@ PropertyWrapperMutabilityRequest::evaluate(Evaluator &, unsigned numWrappers = var->getAttachedPropertyWrappers().size(); if (numWrappers < 1) return None; - if (var->getAccessor(AccessorKind::Get) && - !var->getAccessor(AccessorKind::Get)->isImplicit()) + if (var->getParsedAccessor(AccessorKind::Get)) return None; - if (var->getAccessor(AccessorKind::Set) && - !var->getAccessor(AccessorKind::Set)->isImplicit()) + if (var->getParsedAccessor(AccessorKind::Set)) return None; // Start with the traits from the outermost wrapper. @@ -2076,7 +2071,7 @@ static void finishPropertyWrapperImplInfo(VarDecl *var, } bool wrapperSetterIsUsable = false; - if (var->getAccessor(AccessorKind::Set)) { + if (var->getParsedAccessor(AccessorKind::Set)) { wrapperSetterIsUsable = true; } else if (parentSF && parentSF->Kind != SourceFileKind::Interface && !var->isLet()) { @@ -2153,11 +2148,6 @@ static void finishStorageImplInfo(AbstractStorageDecl *storage, finishProtocolStorageImplInfo(storage, info); } -static bool hasParsedAccessor(AbstractStorageDecl *storage, AccessorKind kind) { - auto *accessor = storage->getAccessor(kind); - return (accessor && !accessor->isImplicit()); -} - /// Gets the storage info of the provided storage decl if it has the /// @_hasStorage attribute and it's not in SIL mode. /// @@ -2179,8 +2169,8 @@ static StorageImplInfo classifyWithHasStorageAttr(VarDecl *var) { WriteImplKind writeImpl; ReadWriteImplKind readWriteImpl; - if (hasParsedAccessor(var, AccessorKind::Get) && - hasParsedAccessor(var, AccessorKind::Set)) { + if (var->getParsedAccessor(AccessorKind::Get) && + var->getParsedAccessor(AccessorKind::Set)) { // If we see `@_hasStorage var x: T { get set }`, then our property has // willSet/didSet observers. writeImpl = var->getAttrs().hasAttribute() ? @@ -2222,25 +2212,27 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator, auto *SF = storage->getDeclContext()->getParentSourceFile(); if (SF && SF->Kind == SourceFileKind::SIL) return StorageImplInfo::getSimpleStored( - StorageIsMutable_t(hasParsedAccessor(var, AccessorKind::Set))); + var->getParsedAccessor(AccessorKind::Set) + ? StorageIsMutable + : StorageIsNotMutable); return classifyWithHasStorageAttr(var); } } - bool hasWillSet = hasParsedAccessor(storage, AccessorKind::WillSet); - bool hasDidSet = hasParsedAccessor(storage, AccessorKind::DidSet); - bool hasSetter = hasParsedAccessor(storage, AccessorKind::Set); - bool hasModify = hasParsedAccessor(storage, AccessorKind::Modify); - bool hasMutableAddress = hasParsedAccessor(storage, AccessorKind::MutableAddress); + bool hasWillSet = storage->getParsedAccessor(AccessorKind::WillSet); + bool hasDidSet = storage->getParsedAccessor(AccessorKind::DidSet); + bool hasSetter = storage->getParsedAccessor(AccessorKind::Set); + bool hasModify = storage->getParsedAccessor(AccessorKind::Modify); + bool hasMutableAddress = storage->getParsedAccessor(AccessorKind::MutableAddress); // 'get', 'read', and a non-mutable addressor are all exclusive. ReadImplKind readImpl; - if (hasParsedAccessor(storage, AccessorKind::Get)) { + if (storage->getParsedAccessor(AccessorKind::Get)) { readImpl = ReadImplKind::Get; - } else if (hasParsedAccessor(storage, AccessorKind::Read)) { + } else if (storage->getParsedAccessor(AccessorKind::Read)) { readImpl = ReadImplKind::Read; - } else if (hasParsedAccessor(storage, AccessorKind::Address)) { + } else if (storage->getParsedAccessor(AccessorKind::Address)) { readImpl = ReadImplKind::Address; // If there's a writing accessor of any sort, there must also be a @@ -2349,7 +2341,7 @@ RequiresOpaqueAccessorsRequest::evaluate(Evaluator &evaluator, // But we might need to create opaque accessors for them. if (auto sourceFile = dc->getParentSourceFile()) { if (sourceFile->Kind == SourceFileKind::SIL) { - if (!var->getAccessor(AccessorKind::Get)) + if (!var->getParsedAccessor(AccessorKind::Get)) return false; } } @@ -2451,8 +2443,8 @@ synthesizeSetterBody(AccessorDecl *setter, ASTContext &ctx) { } if (var->hasAttachedPropertyWrapper()) { - if (var->getAccessor(AccessorKind::WillSet) || - var->getAccessor(AccessorKind::DidSet)) { + if (var->getParsedAccessor(AccessorKind::WillSet) || + var->getParsedAccessor(AccessorKind::DidSet)) { return synthesizeObservedSetterBody(setter, TargetImpl::Wrapper, ctx); } diff --git a/lib/Sema/TypeCheckAttr.cpp b/lib/Sema/TypeCheckAttr.cpp index 2d4e7f6d2dbf5..aae1d1665b513 100644 --- a/lib/Sema/TypeCheckAttr.cpp +++ b/lib/Sema/TypeCheckAttr.cpp @@ -2499,8 +2499,8 @@ void AttributeChecker::visitCustomAttr(CustomAttr *attr) { decl = func; } else if (auto storage = dyn_cast(D)) { decl = storage; - auto getter = storage->getAccessor(AccessorKind::Get); - if (!getter || getter->isImplicit() || !getter->hasBody()) { + auto getter = storage->getParsedAccessor(AccessorKind::Get); + if (!getter || !getter->hasBody()) { TC.diagnose(attr->getLocation(), diag::function_builder_attribute_on_storage_without_getter, nominal->getFullName(), @@ -2836,8 +2836,8 @@ void TypeChecker::addImplicitDynamicAttribute(Decl *D) { // exclusivity checking. // If there is a didSet or willSet function we allow dynamic replacement. if (VD->hasStorage() && - !VD->getAccessor(AccessorKind::DidSet) && - !VD->getAccessor(AccessorKind::WillSet)) + !VD->getParsedAccessor(AccessorKind::DidSet) && + !VD->getParsedAccessor(AccessorKind::WillSet)) return; // Don't add dynamic to local variables. if (VD->getDeclContext()->isLocalContext()) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index bad450e06ae35..e32dd6630107d 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2277,10 +2277,10 @@ IsGetterMutatingRequest::evaluate(Evaluator &evaluator, } auto checkMutability = [&](AccessorKind kind) -> bool { - auto *accessor = storage->getAccessor(kind); + auto *accessor = storage->getParsedAccessor(kind); if (!accessor) return false; - + return accessor->isMutating(); }; @@ -2337,7 +2337,7 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator, case WriteImplKind::StoredWithObservers: case WriteImplKind::InheritedWithObservers: case WriteImplKind::Set: { - auto *setter = storage->getAccessor(AccessorKind::Set); + auto *setter = storage->getParsedAccessor(AccessorKind::Set); if (setter) result = setter->isMutating(); @@ -2347,7 +2347,7 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator, // coroutine, check that it has the same mutatingness as the setter. // TODO: arguably this should require the spelling to match even when // it's the implied value. - auto modifyAccessor = storage->getAccessor(AccessorKind::Modify); + auto modifyAccessor = storage->getParsedAccessor(AccessorKind::Modify); if (impl.getReadWriteImpl() == ReadWriteImplKind::Modify && modifyAccessor != nullptr) { @@ -2369,11 +2369,11 @@ IsSetterMutatingRequest::evaluate(Evaluator &evaluator, } case WriteImplKind::MutableAddress: - return storage->getAccessor(AccessorKind::MutableAddress) + return storage->getParsedAccessor(AccessorKind::MutableAddress) ->isMutating(); case WriteImplKind::Modify: - return storage->getAccessor(AccessorKind::Modify) + return storage->getParsedAccessor(AccessorKind::Modify) ->isMutating(); } llvm_unreachable("bad storage kind"); @@ -2635,11 +2635,9 @@ class DeclChecker : public DeclVisitor { // FIXME: Temporary hack until capture computation has been request-ified. if (VD->getDeclContext()->isLocalContext()) { - VD->visitExpectedOpaqueAccessors([&](AccessorKind kind) { - auto accessor = VD->getAccessor(kind); - if (!accessor) return; - - TC.definedFunctions.push_back(accessor); + VD->visitOpaqueAccessors([&](AccessorDecl *accessor) { + if (accessor->isImplicit()) + TC.definedFunctions.push_back(accessor); }); } diff --git a/lib/Sema/TypeCheckDeclOverride.cpp b/lib/Sema/TypeCheckDeclOverride.cpp index fd4b6097386ee..4675c254130f9 100644 --- a/lib/Sema/TypeCheckDeclOverride.cpp +++ b/lib/Sema/TypeCheckDeclOverride.cpp @@ -1041,7 +1041,7 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl, // Otherwise, if this is a subscript, validate that covariance is ok. // If the parent is non-mutable, it's okay to be covariant. auto parentSubscript = cast(baseDecl); - if (parentSubscript->getAccessor(AccessorKind::Set)) { + if (parentSubscript->supportsMutation()) { diags.diagnose(subscript, diag::override_mutable_covariant_subscript, declTy, baseTy); diags.diagnose(baseDecl, diag::subscript_override_here); @@ -1088,7 +1088,7 @@ bool OverrideMatcher::checkOverride(ValueDecl *baseDecl, IsSilentDifference = true; // The overridden property must not be mutable. - if (cast(baseDecl)->getAccessor(AccessorKind::Set) && + if (cast(baseDecl)->supportsMutation() && !IsSilentDifference) { diags.diagnose(property, diag::override_mutable_covariant_property, property->getName(), parentPropertyTy, propertyTy); @@ -1472,8 +1472,8 @@ isRedundantAccessorOverrideAvailabilityDiagnostic(ValueDecl *override, // Returns true if we will already diagnose a bad override // on the property's accessor of the given kind. auto accessorOverrideAlreadyDiagnosed = [&](AccessorKind kind) { - FuncDecl *overrideAccessor = overrideASD->getAccessor(kind); - FuncDecl *baseAccessor = baseASD->getAccessor(kind); + FuncDecl *overrideAccessor = overrideASD->getOpaqueAccessor(kind); + FuncDecl *baseAccessor = baseASD->getOpaqueAccessor(kind); if (overrideAccessor && baseAccessor && !isAvailabilitySafeForOverride(overrideAccessor, baseAccessor)) { return true; @@ -1557,8 +1557,8 @@ static bool checkSingleOverride(ValueDecl *override, ValueDecl *base) { // Make sure that the overriding property doesn't have storage. if ((overrideASD->hasStorage() || overrideASD->getAttrs().hasAttribute()) && - !(overrideASD->getAccessor(AccessorKind::WillSet) || - overrideASD->getAccessor(AccessorKind::DidSet))) { + !(overrideASD->getParsedAccessor(AccessorKind::WillSet) || + overrideASD->getParsedAccessor(AccessorKind::DidSet))) { bool downgradeToWarning = false; if (!ctx.isSwiftVersionAtLeast(5) && overrideASD->getAttrs().hasAttribute()) { @@ -1889,12 +1889,13 @@ OverriddenDeclsRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const { continue; // Find the base accessor; if there isn't one, we're done. - auto baseAccessor = baseASD->getAccessor(kind); - if (!baseAccessor) continue; - - if (baseAccessor->hasForcedStaticDispatch()) + auto baseAccessor = baseASD->getOpaqueAccessor(kind); + if (!baseAccessor) continue; + assert(!baseAccessor->hasForcedStaticDispatch() && + "opaque accessor with forced static dispatch?"); + switch (kind) { case AccessorKind::Get: case AccessorKind::Read: diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 615ed3ce7df1f..f14045fb70ba3 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -238,15 +238,17 @@ static bool checkObjCWitnessSelector(TypeChecker &tc, ValueDecl *req, // FIXME: Check property names! // Check the getter. - if (auto reqGetter = reqStorage->getAccessor(AccessorKind::Get)) { - auto *witnessGetter = witnessStorage->getAccessor(AccessorKind::Get); + if (auto reqGetter = reqStorage->getParsedAccessor(AccessorKind::Get)) { + auto *witnessGetter = + witnessStorage->getSynthesizedAccessor(AccessorKind::Get); if (checkObjCWitnessSelector(tc, reqGetter, witnessGetter)) return true; } // Check the setter. - if (auto reqSetter = reqStorage->getAccessor(AccessorKind::Set)) { - auto *witnessSetter = witnessStorage->getAccessor(AccessorKind::Set); + if (auto reqSetter = reqStorage->getParsedAccessor(AccessorKind::Set)) { + auto *witnessSetter = + witnessStorage->getSynthesizedAccessor(AccessorKind::Set); if (checkObjCWitnessSelector(tc, reqSetter, witnessSetter)) return true; } @@ -264,48 +266,40 @@ static ParameterList *getParameterList(ValueDecl *value) { // Find a standin declaration to place the diagnostic at for the // given accessor kind. -static ValueDecl *getStandinForAccessor(AbstractStorageDecl *witnessStorage, +static ValueDecl *getStandinForAccessor(AbstractStorageDecl *witness, AccessorKind requirementKind) { - auto getExplicitAccessor = [&](AccessorKind kind) -> AccessorDecl* { - if (auto accessor = witnessStorage->getAccessor(kind)) { - if (!accessor->isImplicit()) - return accessor; - } - return nullptr; - }; - // If the storage actually explicitly provides that accessor, great. - if (auto accessor = getExplicitAccessor(requirementKind)) + if (auto accessor = witness->getParsedAccessor(requirementKind)) return accessor; // If it didn't, check to see if it provides something else that corresponds // to the requirement. switch (requirementKind) { case AccessorKind::Get: - if (auto read = getExplicitAccessor(AccessorKind::Read)) + if (auto read = witness->getParsedAccessor(AccessorKind::Read)) return read; - if (auto addressor = getExplicitAccessor(AccessorKind::Address)) + if (auto addressor = witness->getParsedAccessor(AccessorKind::Address)) return addressor; break; case AccessorKind::Read: - if (auto getter = getExplicitAccessor(AccessorKind::Get)) + if (auto getter = witness->getParsedAccessor(AccessorKind::Get)) return getter; - if (auto addressor = getExplicitAccessor(AccessorKind::Address)) + if (auto addressor = witness->getParsedAccessor(AccessorKind::Address)) return addressor; break; case AccessorKind::Modify: - if (auto setter = getExplicitAccessor(AccessorKind::Set)) + if (auto setter = witness->getParsedAccessor(AccessorKind::Set)) return setter; - if (auto addressor = getExplicitAccessor(AccessorKind::MutableAddress)) + if (auto addressor = witness->getParsedAccessor(AccessorKind::MutableAddress)) return addressor; break; case AccessorKind::Set: - if (auto modify = getExplicitAccessor(AccessorKind::Modify)) + if (auto modify = witness->getParsedAccessor(AccessorKind::Modify)) return modify; - if (auto addressor = getExplicitAccessor(AccessorKind::MutableAddress)) + if (auto addressor = witness->getParsedAccessor(AccessorKind::MutableAddress)) return addressor; break; @@ -317,7 +311,7 @@ static ValueDecl *getStandinForAccessor(AbstractStorageDecl *witnessStorage, } // Otherwise, just diagnose starting at the storage declaration itself. - return witnessStorage; + return witness; } RequirementMatch @@ -5040,9 +5034,9 @@ void TypeChecker::checkConformancesInContext(DeclContext *dc, sf->ObjCUnsatisfiedOptReqs.emplace_back(dc, funcReq); } else { auto storageReq = cast(req); - if (auto getter = storageReq->getAccessor(AccessorKind::Get)) + if (auto getter = storageReq->getParsedAccessor(AccessorKind::Get)) sf->ObjCUnsatisfiedOptReqs.emplace_back(dc, getter); - if (auto setter = storageReq->getAccessor(AccessorKind::Set)) + if (auto setter = storageReq->getParsedAccessor(AccessorKind::Set)) sf->ObjCUnsatisfiedOptReqs.emplace_back(dc, setter); } } @@ -5151,7 +5145,7 @@ swift::findWitnessedObjCRequirements(const ValueDecl *witness, auto *storageReq = dyn_cast(req); if (!storageReq) continue; - req = storageReq->getAccessor(*accessorKind); + req = storageReq->getOpaqueAccessor(*accessorKind); if (!req) continue; } @@ -5169,10 +5163,10 @@ swift::findWitnessedObjCRequirements(const ValueDecl *witness, auto *storageFound = dyn_cast_or_null(found); if (!storageReq || !storageFound) continue; - req = storageReq->getAccessor(*accessorKind); + req = storageReq->getOpaqueAccessor(*accessorKind); if (!req) continue; - found = storageFound->getAccessor(*accessorKind); + found = storageFound->getOpaqueAccessor(*accessorKind); } // Determine whether the witness for this conformance is in fact diff --git a/lib/Serialization/Serialization.cpp b/lib/Serialization/Serialization.cpp index 3f932e6791434..6b7d4f86994e0 100644 --- a/lib/Serialization/Serialization.cpp +++ b/lib/Serialization/Serialization.cpp @@ -4891,15 +4891,6 @@ void Serializer::writeAST(ModuleOrSourceFile DC, orderedTopLevelDecls.push_back(addDeclRef(D)); - // If this is a global variable, force the accessors to be - // serialized. - if (auto VD = dyn_cast(D)) { - if (auto *getter = VD->getAccessor(swift::AccessorKind::Get)) - addDeclRef(getter); - if (auto *setter = VD->getAccessor(swift::AccessorKind::Set)) - addDeclRef(setter); - } - // If this nominal type has associated top-level decls for a // derived conformance (for example, ==), force them to be // serialized. diff --git a/lib/TBDGen/TBDGen.cpp b/lib/TBDGen/TBDGen.cpp index c310794403ac5..6767de6a59f93 100644 --- a/lib/TBDGen/TBDGen.cpp +++ b/lib/TBDGen/TBDGen.cpp @@ -159,8 +159,8 @@ void TBDGenVisitor::addConformances(DeclContext *DC) { auto witnessStorage = cast(witnessDecl); storage->visitOpaqueAccessors([&](AccessorDecl *reqtAccessor) { auto witnessAccessor = - witnessStorage->getAccessor(reqtAccessor->getAccessorKind()); - assert(witnessAccessor && "no corresponding witness accessor?"); + witnessStorage->getSynthesizedAccessor( + reqtAccessor->getAccessorKind()); addSymbolIfNecessary(reqtAccessor, witnessAccessor); }); } From c12e0041561430021ca4050c06ee4d88a8dd7f4c Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 2 Aug 2019 00:02:58 -0400 Subject: [PATCH 2/7] Sema: Fix multi-file edge case with accessor availability checking When checking availability of referenced accessors, we may not have synthesized the accessors yet. This meant that we didn't diagnose references to internal(set) properties from inlinable contexts if the property was defined in another file. --- lib/Sema/TypeCheckAvailability.cpp | 12 ++++++------ test/multifile/Inputs/inlinable-other.swift | 4 ++++ test/multifile/inlinable.swift | 10 ++++++++++ 3 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 test/multifile/Inputs/inlinable-other.swift create mode 100644 test/multifile/inlinable.swift diff --git a/lib/Sema/TypeCheckAvailability.cpp b/lib/Sema/TypeCheckAvailability.cpp index 6f90ac8cc62ea..82fce21a28820 100644 --- a/lib/Sema/TypeCheckAvailability.cpp +++ b/lib/Sema/TypeCheckAvailability.cpp @@ -2519,31 +2519,31 @@ class AvailabilityWalker : public ASTWalker { if (!D) return; - if (!D->hasAnyAccessors()) { + if (!D->requiresOpaqueAccessors()) { return; } - + // Check availability of accessor functions. // TODO: if we're talking about an inlineable storage declaration, // this probably needs to be refined to not assume that the accesses are // specifically using the getter/setter. switch (AccessContext) { case MemberAccessContext::Getter: - diagAccessorAvailability(D->getAccessor(AccessorKind::Get), + diagAccessorAvailability(D->getOpaqueAccessor(AccessorKind::Get), ReferenceRange, ReferenceDC, None); break; case MemberAccessContext::Setter: - diagAccessorAvailability(D->getAccessor(AccessorKind::Set), + diagAccessorAvailability(D->getOpaqueAccessor(AccessorKind::Set), ReferenceRange, ReferenceDC, None); break; case MemberAccessContext::InOut: - diagAccessorAvailability(D->getAccessor(AccessorKind::Get), + diagAccessorAvailability(D->getOpaqueAccessor(AccessorKind::Get), ReferenceRange, ReferenceDC, DeclAvailabilityFlag::ForInout); - diagAccessorAvailability(D->getAccessor(AccessorKind::Set), + diagAccessorAvailability(D->getOpaqueAccessor(AccessorKind::Set), ReferenceRange, ReferenceDC, DeclAvailabilityFlag::ForInout); break; diff --git a/test/multifile/Inputs/inlinable-other.swift b/test/multifile/Inputs/inlinable-other.swift new file mode 100644 index 0000000000000..95d47830594c8 --- /dev/null +++ b/test/multifile/Inputs/inlinable-other.swift @@ -0,0 +1,4 @@ +public struct OtherStruct { + public internal(set) static var staticProp = 123 + // expected-note@-1 {{setter for 'staticProp' is not '@usableFromInline' or public}} +} diff --git a/test/multifile/inlinable.swift b/test/multifile/inlinable.swift new file mode 100644 index 0000000000000..08ec4b959fabe --- /dev/null +++ b/test/multifile/inlinable.swift @@ -0,0 +1,10 @@ +// RUN: %target-swift-frontend -typecheck -verify %s %S/Inputs/inlinable-other.swift + +@frozen public struct HasInitExpr { + public var hasInlinableInit = mutatingFunc(&OtherStruct.staticProp) + // expected-warning@-1 {{setter for 'staticProp' is internal and should not be referenced from a property initializer in a '@frozen' type}} +} + +public func mutatingFunc(_: inout Int) -> Int { + return 0 +} From 42901546fb16f0d34ba8d5fe2d9c531b98d775b3 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Thu, 1 Aug 2019 23:52:13 -0400 Subject: [PATCH 3/7] Sema: Add multi-file test case for @objc conformance checking Make sure we test checkObjCWitnessSelector() in the multi-file case. I made some changes that regressed a source compatibility project but the regression was not caught by our test suite, so make sure we have a test for this now. --- .../Inputs/protocol-conformance-objc-other.swift | 5 +++++ test/multifile/protocol-conformance-objc.swift | 12 ++++++++++++ 2 files changed, 17 insertions(+) create mode 100644 test/multifile/Inputs/protocol-conformance-objc-other.swift create mode 100644 test/multifile/protocol-conformance-objc.swift diff --git a/test/multifile/Inputs/protocol-conformance-objc-other.swift b/test/multifile/Inputs/protocol-conformance-objc-other.swift new file mode 100644 index 0000000000000..2e7142bc48b86 --- /dev/null +++ b/test/multifile/Inputs/protocol-conformance-objc-other.swift @@ -0,0 +1,5 @@ +import Foundation + +public class Pony { + public var hindEnd: Int = 0 +} diff --git a/test/multifile/protocol-conformance-objc.swift b/test/multifile/protocol-conformance-objc.swift new file mode 100644 index 0000000000000..b90ffc4617a6d --- /dev/null +++ b/test/multifile/protocol-conformance-objc.swift @@ -0,0 +1,12 @@ +// RUN: %target-swift-frontend -sdk %sdk -emit-ir -primary-file %s %S/Inputs/protocol-conformance-objc-other.swift -module-name test + +// REQUIRES: objc_interop + +import Foundation + +@objc protocol Horse { + var hindEnd: Int { get set } +} + +extension Pony : Horse {} + From 2be7573aa0660ef497eecdb8633b28d1cf1c1c37 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 30 Jul 2019 23:39:05 -0400 Subject: [PATCH 4/7] Sema: Remove synthesizeWitnessAccessorsForStorage() We can lazily synthesize accessor witnesses from SILGen now. --- lib/Sema/CodeSynthesis.cpp | 16 ---------------- lib/Sema/TypeCheckProtocol.cpp | 12 ------------ lib/Sema/TypeChecker.h | 6 ------ .../Inputs/protocol-conformance-let-other.swift | 1 - 4 files changed, 35 deletions(-) diff --git a/lib/Sema/CodeSynthesis.cpp b/lib/Sema/CodeSynthesis.cpp index 513db05b7e1ed..a90bdcd0bafbf 100644 --- a/lib/Sema/CodeSynthesis.cpp +++ b/lib/Sema/CodeSynthesis.cpp @@ -1320,22 +1320,6 @@ synthesizeModifyCoroutineSetterBody(AccessorDecl *setter, ASTContext &ctx) { setter->getStorage(), ctx); } -/// The specified AbstractStorageDecl was just found to satisfy a -/// protocol property requirement. Ensure that it has the full -/// complement of accessors. -void TypeChecker::synthesizeWitnessAccessorsForStorage( - AbstractStorageDecl *requirement, - AbstractStorageDecl *storage) { - // Make sure the protocol requirement itself has the right accessors. - // FIXME: This should be a request kicked off by SILGen. - DeclsToFinalize.insert(requirement); - - requirement->visitExpectedOpaqueAccessors([&](AccessorKind kind) { - // Force synthesis if necessary. - (void) storage->getSynthesizedAccessor(kind); - }); -} - /// Given a VarDecl with a willSet: and/or didSet: specifier, synthesize the /// setter which calls them. static std::pair diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index f14045fb70ba3..2b55a43cc2694 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -2208,12 +2208,6 @@ void ConformanceChecker::recordWitness(ValueDecl *requirement, // Record this witness in the conformance. auto witness = match.getWitness(TC.Context); Conformance->setWitness(requirement, witness); - - // Synthesize accessors for the protocol witness table to use. - if (auto storage = dyn_cast(witness.getDecl())) - TC.synthesizeWitnessAccessorsForStorage( - cast(requirement), - storage); } void ConformanceChecker::recordOptionalWitness(ValueDecl *requirement) { @@ -5337,12 +5331,6 @@ void DefaultWitnessChecker::recordWitness( const RequirementMatch &match) { Proto->setDefaultWitness(requirement, match.getWitness(TC.Context)); - - // Synthesize accessors for the protocol witness table to use. - if (auto storage = dyn_cast(match.Witness)) - TC.synthesizeWitnessAccessorsForStorage( - cast(requirement), - storage); } void TypeChecker::inferDefaultWitnesses(ProtocolDecl *proto) { diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index ca4ac206a3b0e..4e2ec66ba25ef 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -1172,12 +1172,6 @@ class TypeChecker final : public LazyResolver { /// target. void synthesizeMemberForLookup(NominalTypeDecl *target, DeclName member); - /// The specified AbstractStorageDecl \c storage was just found to satisfy - /// the protocol property \c requirement. Ensure that it has the full - /// complement of accessors. - void synthesizeWitnessAccessorsForStorage(AbstractStorageDecl *requirement, - AbstractStorageDecl *storage); - /// Pre-check the expression, validating any types that occur in the /// expression and folding sequence expressions. bool preCheckExpression(Expr *&expr, DeclContext *dc); diff --git a/test/multifile/Inputs/protocol-conformance-let-other.swift b/test/multifile/Inputs/protocol-conformance-let-other.swift index 82a353093b657..847398b3ac021 100644 --- a/test/multifile/Inputs/protocol-conformance-let-other.swift +++ b/test/multifile/Inputs/protocol-conformance-let-other.swift @@ -1,4 +1,3 @@ public protocol P { let x: Int - // expected-error@-1 {{immutable property requirement must be declared as 'var' with a '{ get }' specifier}} } From 6694afa299a5274320e967053f4e1870ce85e07d Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Tue, 30 Jul 2019 22:29:12 -0400 Subject: [PATCH 5/7] Sema: finalizeDecl() no longer forces stored property lowering of structs --- lib/Sema/TypeCheckDecl.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index e32dd6630107d..43ae0c0d14828 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -4628,10 +4628,10 @@ static void finalizeType(TypeChecker &TC, NominalTypeDecl *nominal) { // affects vtable layout. TC.addImplicitConstructors(CD); CD->addImplicitDestructor(); - } - // Force lowering of stored properties. - (void) nominal->getStoredProperties(); + // Force lowering of stored properties. + (void) nominal->getStoredProperties(); + } if (isa(nominal) || isa(nominal)) { for (auto *D : nominal->getMembers()) { From 1631f2f8fe4bc0e3ffb00f25a07cad5cb3f6d064 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Wed, 31 Jul 2019 16:34:28 -0400 Subject: [PATCH 6/7] Sema: finalizeDecl() no longer forces accessor synthesis Previously we would synthesize accessors for any referenced storage, as well as any storage members of classes and protocols. Now that synthesis is sufficiently lazy this is no longer needed. The only remaining work done in finalizeDecl() is adding certain implicit members to ClassDecls; no other declaration kind ends up in DeclsToFinalize anymore. --- lib/Sema/TypeCheckDecl.cpp | 136 ++++++++++----------------------- lib/Sema/TypeCheckPattern.cpp | 4 +- lib/Sema/TypeCheckProtocol.cpp | 5 -- lib/Sema/TypeChecker.h | 6 +- 4 files changed, 45 insertions(+), 106 deletions(-) diff --git a/lib/Sema/TypeCheckDecl.cpp b/lib/Sema/TypeCheckDecl.cpp index 43ae0c0d14828..388e32abe1b82 100644 --- a/lib/Sema/TypeCheckDecl.cpp +++ b/lib/Sema/TypeCheckDecl.cpp @@ -2472,11 +2472,6 @@ class DeclChecker : public DeclVisitor { (void) VD->isObjC(); (void) VD->isDynamic(); - // Make sure we finalize this declaration. - if (isa(VD) || isa(VD) || - isa(VD)) - TC.DeclsToFinalize.insert(VD); - // If this is a member of a nominal type, don't allow it to have a name of // "Type" or "Protocol" since we reserve the X.Type and X.Protocol // expressions to mean something builtin to the language. We *do* allow @@ -3155,6 +3150,8 @@ class DeclChecker : public DeclVisitor { void visitClassDecl(ClassDecl *CD) { + TC.DeclsToFinalize.insert(CD); + TC.checkDeclAttributesEarly(CD); checkUnsupportedNestedType(CD); @@ -3499,7 +3496,7 @@ class DeclChecker : public DeclVisitor { if (auto nominal = ED->getExtendedNominal()) { TC.validateDecl(nominal); if (auto *classDecl = dyn_cast(nominal)) - TC.requestNominalLayout(classDecl); + TC.requestClassLayout(classDecl); // Check the raw values of an enum, since we might synthesize // RawRepresentable while checking conformances on this extension. @@ -4143,14 +4140,6 @@ void TypeChecker::validateDecl(ValueDecl *D) { validateAttributes(*this, D); - // FIXME: IRGen likes to emit @objc protocol descriptors even if the - // protocol comes from a different module or translation unit. - // - // It would be nice if it didn't have to do that, then we could remove - // this case. - if (proto->isObjC()) - requestNominalLayout(proto); - break; } @@ -4522,14 +4511,6 @@ void TypeChecker::validateDeclForNameLookup(ValueDecl *D) { return; proto->computeType(); - // FIXME: IRGen likes to emit @objc protocol descriptors even if the - // protocol comes from a different module or translation unit. - // - // It would be nice if it didn't have to do that, then we could remove - // this case. - if (proto->isObjC()) - requestNominalLayout(proto); - break; } case DeclKind::AssociatedType: { @@ -4594,96 +4575,59 @@ void TypeChecker::validateDeclForNameLookup(ValueDecl *D) { void TypeChecker::requestMemberLayout(ValueDecl *member) { auto *dc = member->getDeclContext(); if (auto *classDecl = dyn_cast(dc)) - requestNominalLayout(classDecl); - if (auto *protocolDecl = dyn_cast(dc)) - requestNominalLayout(protocolDecl); - - // If this represents (abstract) storage, form the appropriate accessors. - if (auto storage = dyn_cast(member)) - DeclsToFinalize.insert(storage); + requestClassLayout(classDecl); } -void TypeChecker::requestNominalLayout(NominalTypeDecl *nominalDecl) { - if (isa(nominalDecl->getModuleScopeContext())) - DeclsToFinalize.insert(nominalDecl); +void TypeChecker::requestClassLayout(ClassDecl *classDecl) { + if (isa(classDecl->getModuleScopeContext())) + DeclsToFinalize.insert(classDecl); } void TypeChecker::requestSuperclassLayout(ClassDecl *classDecl) { if (auto *superclassDecl = classDecl->getSuperclassDecl()) { if (superclassDecl) - requestNominalLayout(superclassDecl); + requestClassLayout(superclassDecl); } } -/// "Finalize" the type so that SILGen can make copies of it, call -/// methods on it, etc. This requires forcing enough computation so -/// that (for example) a class can layout its vtable or a struct can -/// be laid out in memory. -static void finalizeType(TypeChecker &TC, NominalTypeDecl *nominal) { - assert(!nominal->hasClangNode()); - assert(isa(nominal->getModuleScopeContext())); - - if (auto *CD = dyn_cast(nominal)) { - // We need to add implicit initializers and dtors because it - // affects vtable layout. - TC.addImplicitConstructors(CD); - CD->addImplicitDestructor(); - - // Force lowering of stored properties. - (void) nominal->getStoredProperties(); - } - - if (isa(nominal) || isa(nominal)) { - for (auto *D : nominal->getMembers()) { - if (auto *ASD = dyn_cast(D)) - TC.DeclsToFinalize.insert(ASD); - } - } - - if (auto *CD = dyn_cast(nominal)) { - // We need the superclass vtable layout as well. - TC.requestSuperclassLayout(CD); +void TypeChecker::finalizeDecl(ClassDecl *CD) { + if (Context.Stats) + Context.Stats->getFrontendCounters().NumDeclsFinalized++; - auto forceConformance = [&](ProtocolDecl *protocol) { - if (auto ref = TypeChecker::conformsToProtocol( - CD->getDeclaredInterfaceType(), protocol, CD, - ConformanceCheckFlags::SkipConditionalRequirements, - SourceLoc())) { - auto conformance = ref->getConcrete(); - if (conformance->getDeclContext() == CD && - conformance->getState() == ProtocolConformanceState::Incomplete) { - TC.checkConformance(conformance->getRootNormalConformance()); - } - } - }; + assert(!CD->hasClangNode()); + assert(isa(CD->getModuleScopeContext())); - // If the class is Encodable, Decodable or Hashable, force those - // conformances to ensure that the synthesized members appear in the vtable. - // - // FIXME: Generalize this to other protocols for which - // we can derive conformances. - forceConformance(TC.Context.getProtocol(KnownProtocolKind::Decodable)); - forceConformance(TC.Context.getProtocol(KnownProtocolKind::Encodable)); - forceConformance(TC.Context.getProtocol(KnownProtocolKind::Hashable)); - } + validateDecl(CD); - if (auto PD = dyn_cast(nominal)) { - for (auto *inherited : PD->getInheritedProtocols()) - TC.requestNominalLayout(inherited); - } -} + // We need to add implicit initializers and dtors because it + // affects vtable layout. + addImplicitConstructors(CD); + CD->addImplicitDestructor(); -void TypeChecker::finalizeDecl(ValueDecl *decl) { - if (Context.Stats) - Context.Stats->getFrontendCounters().NumDeclsFinalized++; + // We need the superclass vtable layout as well. + requestSuperclassLayout(CD); - validateDecl(decl); + auto forceConformance = [&](ProtocolDecl *protocol) { + if (auto ref = TypeChecker::conformsToProtocol( + CD->getDeclaredInterfaceType(), protocol, CD, + ConformanceCheckFlags::SkipConditionalRequirements, + SourceLoc())) { + auto conformance = ref->getConcrete(); + if (conformance->getDeclContext() == CD && + conformance->getState() == ProtocolConformanceState::Incomplete) { + checkConformance(conformance->getRootNormalConformance()); + } + } + }; - if (auto nominal = dyn_cast(decl)) { - finalizeType(*this, nominal); - } else if (auto storage = dyn_cast(decl)) { - addExpectedOpaqueAccessorsToStorage(storage); - } + // If the class is Encodable, Decodable or Hashable, force those + // conformances to ensure that the synthesized members appear in the vtable. + // + // FIXME: Generalize this to other protocols for which + // we can derive conformances. + forceConformance(Context.getProtocol(KnownProtocolKind::Decodable)); + forceConformance(Context.getProtocol(KnownProtocolKind::Encodable)); + forceConformance(Context.getProtocol(KnownProtocolKind::Hashable)); } /// Determine whether this is a "pass-through" typealias, which has the diff --git a/lib/Sema/TypeCheckPattern.cpp b/lib/Sema/TypeCheckPattern.cpp index 1862a9b2b5243..429b4deec2a7c 100644 --- a/lib/Sema/TypeCheckPattern.cpp +++ b/lib/Sema/TypeCheckPattern.cpp @@ -809,8 +809,8 @@ static void requestLayoutForMetadataSources(TypeChecker &tc, Type type) { // parameter is of dependent type then the body of a function with said // parameter could potentially require the generic type's layout to // recover them. - if (auto *nominalDecl = type->getAnyNominal()) { - tc.requestNominalLayout(nominalDecl); + if (auto *classDecl = type->getClassOrBoundGenericClass()) { + tc.requestClassLayout(classDecl); } }); } diff --git a/lib/Sema/TypeCheckProtocol.cpp b/lib/Sema/TypeCheckProtocol.cpp index 2b55a43cc2694..a9377a6504f85 100644 --- a/lib/Sema/TypeCheckProtocol.cpp +++ b/lib/Sema/TypeCheckProtocol.cpp @@ -3693,11 +3693,6 @@ void ConformanceChecker::resolveValueWitnesses() { auto witness = Conformance->getWitness(requirement).getDecl(); if (!witness) return; - // Make sure that we finalize the witness, so we can emit this - // witness table. - if (isa(witness)) - TC.DeclsToFinalize.insert(witness); - // Objective-C checking for @objc requirements. if (requirement->isObjC() && requirement->getFullName() == witness->getFullName() && diff --git a/lib/Sema/TypeChecker.h b/lib/Sema/TypeChecker.h index 4e2ec66ba25ef..a480b69d41d8a 100644 --- a/lib/Sema/TypeChecker.h +++ b/lib/Sema/TypeChecker.h @@ -556,7 +556,7 @@ class TypeChecker final : public LazyResolver { /// The list of declarations that we've done at least partial validation /// of during type-checking, but which will need to be finalized before /// we can hand them off to SILGen etc. - llvm::SetVector DeclsToFinalize; + llvm::SetVector DeclsToFinalize; /// Track the index of the next declaration that needs to be finalized, /// from the \c DeclsToFinalize set. @@ -810,7 +810,7 @@ class TypeChecker final : public LazyResolver { /// Request that the given class needs to have all members validated /// after everything in the translation unit has been processed. - void requestNominalLayout(NominalTypeDecl *nominalDecl); + void requestClassLayout(ClassDecl *classDecl); /// Request that the superclass of the given class, if any, needs to have /// all members validated after everything in the translation unit has @@ -819,7 +819,7 @@ class TypeChecker final : public LazyResolver { /// Perform final validation of a declaration after everything in the /// translation unit has been processed. - void finalizeDecl(ValueDecl *D); + void finalizeDecl(ClassDecl *CD); /// Resolve a reference to the given type declaration within a particular /// context. From 24b20a3604671f6754ba8a8df50c776dfe3888b1 Mon Sep 17 00:00:00 2001 From: Slava Pestov Date: Fri, 2 Aug 2019 01:14:53 -0400 Subject: [PATCH 7/7] Update stability-stdlib-abi.asserts.additional.swift.expected This is a false positive, since the '_read' accessor is synthesized on demand and emitted with shared linkage. See . --- .../stability-stdlib-abi.asserts.additional.swift.expected | 1 + 1 file changed, 1 insertion(+) diff --git a/test/api-digester/Outputs/stability-stdlib-abi.asserts.additional.swift.expected b/test/api-digester/Outputs/stability-stdlib-abi.asserts.additional.swift.expected index 05c686d12cf91..46dec10978ec8 100644 --- a/test/api-digester/Outputs/stability-stdlib-abi.asserts.additional.swift.expected +++ b/test/api-digester/Outputs/stability-stdlib-abi.asserts.additional.swift.expected @@ -6,3 +6,4 @@ Struct _GlobalRuntimeFunctionCountersState is a new API without @available attri Struct _ObjectRuntimeFunctionCountersState is a new API without @available attribute Struct _RuntimeFunctionCounters is a new API without @available attribute Func _measureRuntimeFunctionCountersDiffs(objects:_:) is a new API without @available attribute +Accessor Slice.subscript(_:).Read() has been removed