Skip to content

Small IsFinalRequest cleanup #25426

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 4 commits into from
Jun 13, 2019
Merged

Small IsFinalRequest cleanup #25426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
729ad65
AST: Remove VarDecl::isREPLVar()
slavapestov Jun 13, 2019
ae4de47
Sema: Correctly mark backing storage for lazy properties and property…
slavapestov Jun 13, 2019
4a8e3fc
AST: Add VarDecl::isLazyStorageProperty() and set it
slavapestov Jun 13, 2019
95c221a
Sema: Replace makeFinal() calls in favor of smarter IsFinalRequest
slavapestov Jun 13, 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
20 changes: 10 additions & 10 deletions include/swift/AST/Decl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,9 +387,8 @@ class alignas(1 << DeclAlignInBits) Decl {
/// It is up to the debugger to instruct SIL how to access this variable.
IsDebuggerVar : 1,

/// Whether this is a property defined in the debugger's REPL.
/// FIXME: Remove this once LLDB has proper support for resilience.
IsREPLVar : 1,
/// Whether this is the backing storage for a lazy property.
IsLazyStorageProperty : 1,

/// Whether this is the backing storage for a property wrapper.
IsPropertyWrapperBackingProperty : 1
Expand Down Expand Up @@ -4806,7 +4805,7 @@ class VarDecl : public AbstractStorageDecl {
Bits.VarDecl.Specifier = static_cast<unsigned>(Sp);
Bits.VarDecl.IsCaptureList = IsCaptureList;
Bits.VarDecl.IsDebuggerVar = false;
Bits.VarDecl.IsREPLVar = false;
Bits.VarDecl.IsLazyStorageProperty = false;
Bits.VarDecl.HasNonPatternBindingInit = false;
Bits.VarDecl.IsPropertyWrapperBackingProperty = false;
}
Expand Down Expand Up @@ -5095,12 +5094,13 @@ class VarDecl : public AbstractStorageDecl {
void setDebuggerVar(bool IsDebuggerVar) {
Bits.VarDecl.IsDebuggerVar = IsDebuggerVar;
}

/// Is this a special debugger REPL variable?
/// FIXME: Remove this once LLDB has proper support for resilience.
bool isREPLVar() const { return Bits.VarDecl.IsREPLVar; }
void setREPLVar(bool IsREPLVar) {
Bits.VarDecl.IsREPLVar = IsREPLVar;

/// Is this the synthesized storage for a 'lazy' property?
bool isLazyStorageProperty() const {
return Bits.VarDecl.IsLazyStorageProperty;
}
void setLazyStorageProperty(bool IsLazyStorage) {
Bits.VarDecl.IsLazyStorageProperty = IsLazyStorage;
}

/// Retrieve the custom attribute that attaches a property wrapper to this
Expand Down
23 changes: 8 additions & 15 deletions lib/Sema/CodeSynthesis.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1625,7 +1625,9 @@ void swift::completeLazyVarImplementation(VarDecl *VD) {
StorageName,
VD->getDeclContext());
Storage->setInterfaceType(StorageInterfaceTy);
Storage->setLazyStorageProperty(true);
Storage->setUserAccessible(false);

addMemberToContextIfNeeded(Storage, VD->getDeclContext(), VD);

// Create the pattern binding decl for the storage decl. This will get
Expand All @@ -1642,12 +1644,7 @@ void swift::completeLazyVarImplementation(VarDecl *VD) {
VD->getGetter()->setBodySynthesizer(&synthesizeLazyGetterBody, Storage);
VD->getSetter()->setBodySynthesizer(&synthesizeLazySetterBody, Storage);

// Mark the vardecl to be final, implicit, and private. In a class, this
// prevents it from being dynamically dispatched. Note that we do this after
// the accessors are set up, because we don't want the setter for the lazy
// property to inherit these properties from the storage.
if (VD->getDeclContext()->getSelfClassDecl())
makeFinal(Context, Storage);
// The storage is implicit and private.
Storage->setImplicit();
Storage->overwriteAccess(AccessLevel::Private);
Storage->overwriteSetterAccess(AccessLevel::Private);
Expand Down Expand Up @@ -1796,6 +1793,11 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,
if (isInvalid)
backingVar->setInvalid();
backingVar->setOriginalWrappedProperty(var);

// The backing storage is 'private'.
backingVar->overwriteAccess(AccessLevel::Private);
backingVar->overwriteSetterAccess(AccessLevel::Private);

addMemberToContextIfNeeded(backingVar, dc, var);

// Create the pattern binding declaration for the backing property.
Expand All @@ -1819,15 +1821,6 @@ PropertyWrapperBackingPropertyInfoRequest::evaluate(Evaluator &evaluator,

tc.typeCheckPatternBinding(parentPBD, patternNumber);
}
// Mark the backing property as 'final'. There's no sensible way to override.
if (dc->getSelfClassDecl())
makeFinal(ctx, backingVar);

// The backing storage is 'private'.
backingVar->overwriteAccess(AccessLevel::Private);

// Determine setter access.
backingVar->overwriteSetterAccess(AccessLevel::Private);

Expr *originalInitialValue = nullptr;
if (Expr *init = parentPBD->getInit(patternNumber)) {
Expand Down
3 changes: 0 additions & 3 deletions lib/Sema/CodeSynthesis.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ class TypeChecker;

class ObjCReason;

// These are implemented in TypeCheckDecl.cpp.
void makeFinal(ASTContext &ctx, ValueDecl *D);

// Implemented in TypeCheckerOverride.cpp
bool checkOverrides(ValueDecl *decl);

Expand Down
36 changes: 18 additions & 18 deletions lib/Sema/TypeCheckDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1050,13 +1050,6 @@ static void validatePatternBindingEntries(TypeChecker &tc,
validatePatternBindingEntry(tc, binding, i);
}

void swift::makeFinal(ASTContext &ctx, ValueDecl *D) {
if (D && !D->isFinal()) {
assert(isa<ClassDecl>(D) || D->isPotentiallyOverridable());
D->getAttrs().add(new (ctx) FinalAttr(/*IsImplicit=*/true));
}
}

namespace {
// The raw values of this enum must be kept in sync with
// diag::implicitly_final_cannot_be_open.
Expand All @@ -1070,12 +1063,8 @@ enum class ImplicitlyFinalReason : unsigned {
};
}

static bool inferFinalAndDiagnoseIfNeeded(ValueDecl *D,
static bool inferFinalAndDiagnoseIfNeeded(ValueDecl *D, ClassDecl *cls,
StaticSpellingKind staticSpelling) {
auto cls = D->getDeclContext()->getSelfClassDecl();
if (!cls)
return false;

// Are there any reasons to infer 'final'? Prefer 'static' over the class
// being final for the purposes of diagnostics.
Optional<ImplicitlyFinalReason> reason;
Expand Down Expand Up @@ -1186,15 +1175,28 @@ doesAccessorNeedDynamicAttribute(AccessorDecl *accessor, Evaluator &evaluator) {

llvm::Expected<bool>
IsFinalRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
if (isa<ClassDecl>(decl))
return decl->getAttrs().hasAttribute<FinalAttr>();

auto cls = decl->getDeclContext()->getSelfClassDecl();
if (!cls)
return false;

switch (decl->getKind()) {
case DeclKind::Var: {
// Properties are final if they are declared 'static' or a 'let'
auto *VD = cast<VarDecl>(decl);

// Backing storage for 'lazy' or property wrappers is always final.
if (VD->isLazyStorageProperty() ||
VD->getOriginalWrappedProperty())
return true;

if (auto *nominalDecl = VD->getDeclContext()->getSelfClassDecl()) {
// If this variable is a class member, mark it final if the
// class is final, or if it was declared with 'let'.
auto *PBD = VD->getParentPatternBinding();
if (PBD && inferFinalAndDiagnoseIfNeeded(decl, PBD->getStaticSpelling()))
if (PBD && inferFinalAndDiagnoseIfNeeded(decl, cls, PBD->getStaticSpelling()))
return true;

if (VD->isLet()) {
Expand All @@ -1219,7 +1221,7 @@ IsFinalRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
case DeclKind::Func: {
// Methods declared 'static' are final.
auto staticSpelling = cast<FuncDecl>(decl)->getStaticSpelling();
if (inferFinalAndDiagnoseIfNeeded(decl, staticSpelling))
if (inferFinalAndDiagnoseIfNeeded(decl, cls, staticSpelling))
return true;
break;
}
Expand All @@ -1230,9 +1232,7 @@ IsFinalRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
case AccessorKind::DidSet:
case AccessorKind::WillSet:
// Observing accessors are marked final if in a class.
if (accessor->getDeclContext()->getSelfClassDecl())
return true;
break;
return true;

case AccessorKind::Read:
case AccessorKind::Modify:
Expand All @@ -1254,7 +1254,7 @@ IsFinalRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
case DeclKind::Subscript: {
// Member subscripts.
auto staticSpelling = cast<SubscriptDecl>(decl)->getStaticSpelling();
if (inferFinalAndDiagnoseIfNeeded(decl, staticSpelling))
if (inferFinalAndDiagnoseIfNeeded(decl, cls, staticSpelling))
return true;
break;
}
Expand Down
12 changes: 12 additions & 0 deletions test/SILGen/lazy_properties.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,15 @@ func test21057425() {
struct HasAnonymousParameters {
lazy var x = { $0 }(0)
}

class LazyClass {
lazy var x = 0
}

// CHECK-LABEL: sil hidden [ossa] @$s15lazy_properties9LazyClassC1xSivg : $@convention(method) (@guaranteed LazyClass) -> Int
// CHECK: ref_element_addr %0 : $LazyClass, #LazyClass.$__lazy_storage_$_x
// CHECK: return

// CHECK-LABEL: sil hidden [ossa] @$s15lazy_properties9LazyClassC1xSivs : $@convention(method) (Int, @guaranteed LazyClass) -> ()
// CHECK: ref_element_addr %1 : $LazyClass, #LazyClass.$__lazy_storage_$_x
// CHECK: return
14 changes: 7 additions & 7 deletions test/SILGen/property_wrappers.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ class ClassUsingWrapper {
extension ClassUsingWrapper {
// CHECK-LABEL: sil hidden [ossa] @$s17property_wrappers17ClassUsingWrapperC04testcdE01cyAC_tF : $@convention(method) (@guaranteed ClassUsingWrapper, @guaranteed ClassUsingWrapper) -> () {
func testClassUsingWrapper(c: ClassUsingWrapper) {
// CHECK: class_method [[GETTER:%.*]] : $ClassUsingWrapper, #ClassUsingWrapper.$x!getter.1
// CHECK: ref_element_addr %1 : $ClassUsingWrapper, #ClassUsingWrapper.$x
self.$x.test()
}
}
Expand Down Expand Up @@ -298,9 +298,9 @@ class UseWrapperWithDefaultInit {


// CHECK-LABEL: sil_vtable ClassUsingWrapper {
// CHECK: #ClassUsingWrapper.x!getter.1: (ClassUsingWrapper) -> () -> Int : @$s17property_wrappers17ClassUsingWrapperC1xSivg // ClassUsingWrapper.x.getter
// CHECK: #ClassUsingWrapper.x!setter.1: (ClassUsingWrapper) -> (Int) -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivs // ClassUsingWrapper.x.setter
// CHECK: #ClassUsingWrapper.x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivM // ClassUsingWrapper.x.modify
// CHECK: #ClassUsingWrapper.$x!getter.1: (ClassUsingWrapper) -> () -> WrapperWithInitialValue<Int> : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvg // ClassUsingWrapper.$x.getter
// CHECK: #ClassUsingWrapper.$x!setter.1: (ClassUsingWrapper) -> (WrapperWithInitialValue<Int>) -> () : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvs // ClassUsingWrapper.$x.setter
// CHECK: #ClassUsingWrapper.$x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC2$x33_{{.*}}16WithInitialValueVySiGvM // ClassUsingWrapper.$x.modify
// CHECK-NEXT: #ClassUsingWrapper.x!getter.1: (ClassUsingWrapper) -> () -> Int : @$s17property_wrappers17ClassUsingWrapperC1xSivg // ClassUsingWrapper.x.getter
// CHECK-NEXT: #ClassUsingWrapper.x!setter.1: (ClassUsingWrapper) -> (Int) -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivs // ClassUsingWrapper.x.setter
// CHECK-NEXT: #ClassUsingWrapper.x!modify.1: (ClassUsingWrapper) -> () -> () : @$s17property_wrappers17ClassUsingWrapperC1xSivM // ClassUsingWrapper.x.modify
// CHECK-NEXT: #ClassUsingWrapper.init!allocator.1: (ClassUsingWrapper.Type) -> () -> ClassUsingWrapper : @$s17property_wrappers17ClassUsingWrapperCACycfC
// CHECK-NEXT: #ClassUsingWrapper.deinit!deallocator.1: @$s17property_wrappers17ClassUsingWrapperCfD
// CHECK-NEXT: }