Skip to content

Lazy accessor synthesis for storage in secondary files #26461

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Aug 3, 2019
Merged

Lazy accessor synthesis for storage in secondary files #26461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0c5d52d
AST: Introduce AbstractStorageDecl::get{Parsed,Opaque}Accessor()
slavapestov Jul 31, 2019
c12e004
Sema: Fix multi-file edge case with accessor availability checking
slavapestov Aug 2, 2019
4290154
Sema: Add multi-file test case for @objc conformance checking
slavapestov Aug 2, 2019
2be7573
Sema: Remove synthesizeWitnessAccessorsForStorage()
slavapestov Jul 31, 2019
6694afa
Sema: finalizeDecl() no longer forces stored property lowering of str…
slavapestov Jul 31, 2019
1631f2f
Sema: finalizeDecl() no longer forces accessor synthesis
slavapestov Jul 31, 2019
24b20a3
Update stability-stdlib-abi.asserts.additional.swift.expected
slavapestov Aug 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Return an accessor that's known to be part of the set..."

Technically, it's possible to distinguish between opaque accessors, which are guaranteed-to-exist ABI-stable implementations of the core access operations, and accessors that might be part of the (non-stable or frozen) ABI. For example, we might want to do funny things with didSet in the future. Not sure if this is worth spelling out in the comment.

///
/// 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<void (AccessorKind)>) const;
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/ASTPrinter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
52 changes: 23 additions & 29 deletions lib/AST/ASTVerifier.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1876,7 +1876,7 @@ class Verifier : public ASTWalker {
if (auto *baseIOT = E->getBase()->getType()->getAs<InOutType>()) {
if (!baseIOT->getObjectType()->is<ArchetypeType>()) {
auto *VD = dyn_cast<VarDecl>(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";
Expand Down Expand Up @@ -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.
Expand All @@ -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();
}
}
}
}

Expand All @@ -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();
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions lib/AST/AccessRequests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
37 changes: 30 additions & 7 deletions lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<void (AccessorKind)> visit) const {
if (!requiresOpaqueAccessors())
Expand All @@ -1993,8 +2015,9 @@ void AbstractStorageDecl::visitExpectedOpaqueAccessors(
void AbstractStorageDecl::visitOpaqueAccessors(
llvm::function_ref<void (AccessorDecl*)> 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);
});
}
Expand Down Expand Up @@ -4029,7 +4052,7 @@ ClassDecl::findOverridingDecl(const AbstractFunctionDecl *Method) const {
auto *Storage = Accessor->getStorage();
if (auto *Derived = ::findOverridingDecl(this, Storage)) {
auto *DerivedStorage = cast<AbstractStorageDecl>(Derived);
return DerivedStorage->getAccessor(Accessor->getAccessorKind());
return DerivedStorage->getOpaqueAccessor(Accessor->getAccessorKind());
}

return nullptr;
Expand Down Expand Up @@ -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<DynamicReplacementAttr>();
if (auto *func = getAccessor(AccessorKind::WillSet))
if (auto *func = getParsedAccessor(AccessorKind::WillSet))
return func->getAttrs().hasAttribute<DynamicReplacementAttr>();
return false;
}
Expand Down Expand Up @@ -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<ObjCAttr>(), preferredName))
return *name;
Expand Down Expand Up @@ -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<ObjCAttr>()
: nullptr;
if (auto name = getNameFromObjcAttribute(objcAttr, DeclName(preferredName))) {
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/SwiftNameTranslation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ getObjCNameForSwiftDecl(const ValueDecl *VD, DeclName PreferredName){
return {BaseName, ObjCSelector()};
return {VAD->getObjCPropertyName(), ObjCSelector()};
} else if (auto *SD = dyn_cast<SubscriptDecl>(VD)) {
return getObjCNameForSwiftDecl(SD->getAccessor(AccessorKind::Get),
return getObjCNameForSwiftDecl(SD->getParsedAccessor(AccessorKind::Get),
PreferredName);
} else if (auto *EL = dyn_cast<EnumElementDecl>(VD)) {
SmallString<64> Buffer;
Expand Down
2 changes: 1 addition & 1 deletion lib/AST/Type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2556,7 +2556,7 @@ bool ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
return true;
}
} else if (auto *asd = dyn_cast<AbstractStorageDecl>(namingDecl)) {
auto *getter = asd->getAccessor(AccessorKind::Get);
auto *getter = asd->getOpaqueAccessor(AccessorKind::Get);
if (getter &&
getter->getResilienceExpansion() == ResilienceExpansion::Minimal) {
return true;
Expand Down
18 changes: 8 additions & 10 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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)
Expand Down
5 changes: 2 additions & 3 deletions lib/IDE/Formatting.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,9 +268,8 @@ class FormatContext {
// return 0; <- No indentation added because of the getter.
// }
if (auto VD = dyn_cast_or_null<VarDecl>(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;
}
Expand Down
8 changes: 4 additions & 4 deletions lib/IRGen/GenClass.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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);
}
};
Expand Down
13 changes: 6 additions & 7 deletions lib/IRGen/GenObjC.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,7 @@ static llvm::Constant *getObjCGetterPointer(IRGenModule &IGM,
if (isa<ProtocolDecl>(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();

Expand All @@ -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);
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion lib/IRGen/Linking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<AbstractStorageDecl>(getDecl())
->getAccessor(AccessorKind::Get);
->getOpaqueAccessor(AccessorKind::Get);
return getSILLinkage(getDeclLinkage(getterDecl), forDefinition);
}

Expand Down
Loading