Skip to content

AST: Trigger HasStorageRequest when retrieving semantic attributes #69600

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.

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
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 3 additions & 1 deletion include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -5374,6 +5374,7 @@ class BuiltinTupleDecl final : public NominalTypeDecl {
/// AbstractStorageDecl - This is the common superclass for VarDecl and
/// SubscriptDecl, representing potentially settable memory locations.
class AbstractStorageDecl : public ValueDecl {
friend class HasStorageRequest;
friend class SetterAccessLevelRequest;
friend class IsGetterMutatingRequest;
friend class IsSetterMutatingRequest;
Expand Down Expand Up @@ -5440,6 +5441,8 @@ class AbstractStorageDecl : public ValueDecl {
llvm::PointerIntPair<AccessorRecord*, 3, OptionalEnum<AccessLevel>> Accessors;

struct {
unsigned HasStorageComputed : 1;
unsigned HasStorage : 1;
unsigned IsGetterMutatingComputed : 1;
unsigned IsGetterMutating : 1;
unsigned IsSetterMutatingComputed : 1;
Expand Down Expand Up @@ -5509,7 +5512,6 @@ class AbstractStorageDecl : public ValueDecl {
return getImplInfo().getReadWriteImpl();
}


/// Return true if this is a VarDecl that has storage associated with
/// it.
bool hasStorage() const;
Expand Down
10 changes: 6 additions & 4 deletions include/swift/AST/TypeCheckRequests.h
Original file line number Diff line number Diff line change
Expand Up @@ -1779,10 +1779,9 @@ class MemberwiseInitPropertiesRequest :
bool isCached() const { return true; }
};

class HasStorageRequest :
public SimpleRequest<HasStorageRequest,
bool(AbstractStorageDecl *),
RequestFlags::Cached> {
class HasStorageRequest
: public SimpleRequest<HasStorageRequest, bool(AbstractStorageDecl *),
RequestFlags::SeparatelyCached> {
public:
using SimpleRequest::SimpleRequest;

Expand All @@ -1793,7 +1792,10 @@ class HasStorageRequest :
bool evaluate(Evaluator &evaluator, AbstractStorageDecl *decl) const;

public:
// Separate caching.
bool isCached() const { return true; }
llvm::Optional<bool> getCachedResult() const;
void cacheResult(bool hasStorage) const;
};

class StorageImplInfoRequest :
Expand Down
2 changes: 1 addition & 1 deletion include/swift/AST/TypeCheckerTypeIDZone.def
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ SWIFT_REQUEST(TypeChecker, StorageImplInfoRequest,
StorageImplInfo(AbstractStorageDecl *), SeparatelyCached,
NoLocationInfo)
SWIFT_REQUEST(TypeChecker, HasStorageRequest,
bool(AbstractStorageDecl *), Cached,
bool(AbstractStorageDecl *), SeparatelyCached,
NoLocationInfo)
SWIFT_REQUEST(TypeChecker, StoredPropertiesAndMissingMembersRequest,
ArrayRef<Decl *>(NominalTypeDecl *), Cached, NoLocationInfo)
Expand Down
12 changes: 5 additions & 7 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6743,13 +6743,11 @@ void AbstractStorageDecl::setImplInfo(StorageImplInfo implInfo) {
cacheImplInfo(implInfo);

if (isImplicit()) {
auto &evaluator = getASTContext().evaluator;
HasStorageRequest request{this};
if (!evaluator.hasCachedResult(request))
evaluator.cacheOutput(request, implInfo.hasStorage());
else {
assert(
evaluateOrDefault(evaluator, request, false) == implInfo.hasStorage());
if (!LazySemanticInfo.HasStorageComputed) {
LazySemanticInfo.HasStorageComputed = true;
LazySemanticInfo.HasStorage = implInfo.hasStorage();
} else {
assert(LazySemanticInfo.HasStorage == implInfo.hasStorage());
}
}
}
Expand Down
3 changes: 3 additions & 0 deletions lib/AST/TypeCheckRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2077,7 +2077,10 @@ DeclAttributes SemanticDeclAttrsRequest::evaluate(Evaluator &evaluator,
// Trigger requests that cause additional semantic attributes to be added.
if (auto afd = dyn_cast<AbstractFunctionDecl>(decl)) {
(void)afd->isTransparent();
} else if (auto asd = dyn_cast<AbstractStorageDecl>(decl)) {
(void)asd->hasStorage();
}

return decl->getAttrs();
}

Expand Down
5 changes: 0 additions & 5 deletions lib/Sema/TypeCheckDeclPrimary.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2331,11 +2331,6 @@ class DeclChecker : public DeclVisitor<DeclChecker> {
this->visitBoundVariable(var);
});

// Add the '@_hasStorage' attribute if this property is stored.
if (VD->hasStorage() && !VD->getAttrs().hasAttribute<HasStorageAttr>())
VD->getAttrs().add(new (getASTContext())
HasStorageAttr(/*isImplicit=*/true));

// Reject cases where this is a variable that has storage but it isn't
// allowed.
if (VD->hasStorage()) {
Expand Down
25 changes: 24 additions & 1 deletion lib/Sema/TypeCheckStorage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3585,6 +3585,29 @@ bool HasStorageRequest::evaluate(Evaluator &evaluator,
return hasStorage;
}

llvm::Optional<bool> HasStorageRequest::getCachedResult() const {
AbstractStorageDecl *decl = std::get<0>(getStorage());
if (decl->LazySemanticInfo.HasStorageComputed)
return static_cast<bool>(decl->LazySemanticInfo.HasStorage);
return llvm::None;
}

void HasStorageRequest::cacheResult(bool hasStorage) const {
AbstractStorageDecl *decl = std::get<0>(getStorage());
decl->LazySemanticInfo.HasStorageComputed = true;
decl->LazySemanticInfo.HasStorage = hasStorage;

// Add an attribute for printing, but only to VarDecls.
if (isa<ParamDecl>(decl))
return;

if (auto varDecl = dyn_cast<VarDecl>(decl)) {
if (hasStorage && !varDecl->getAttrs().hasAttribute<HasStorageAttr>())
varDecl->getAttrs().add(new (varDecl->getASTContext())
HasStorageAttr(/*isImplicit=*/true));
}
}

StorageImplInfo
StorageImplInfoRequest::evaluate(Evaluator &evaluator,
AbstractStorageDecl *storage) const {
Expand All @@ -3598,7 +3621,7 @@ StorageImplInfoRequest::evaluate(Evaluator &evaluator,
if (auto *var = dyn_cast<VarDecl>(storage)) {
// Allow the @_hasStorage attribute to override all the accessors we parsed
// when making the final classification.
if (var->getAttrs().hasAttribute<HasStorageAttr>()) {
if (var->getParsedAttrs().hasAttribute<HasStorageAttr>()) {
// The SIL rules for @_hasStorage are slightly different from the non-SIL
// rules. In SIL mode, @_hasStorage marks that the type is simply stored,
// and the only thing that determines mutability is the existence of the
Expand Down
8 changes: 8 additions & 0 deletions test/Inputs/lazy_typecheck.swift
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,14 @@ public struct PublicGenericStruct<T> {
}
}

@frozen public struct FrozenPublicStruct {
private(set) var varWithPrivateSetter: Int = 1

public init(_ varWithPrivateSetter: Int) {
self.varWithPrivateSetter = varWithPrivateSetter
}
}

struct InternalStruct: NoTypecheckProto {
var x: NoTypecheck

Expand Down
4 changes: 3 additions & 1 deletion test/Inputs/lazy_typecheck_client.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func testGobalVars() {
let _: (Int, Int) = (publicGlobalVarInferredTuplePatX, publicGlobalVarInferredTuplePatY)
}

func testPublicStruct() {
func testPublicStructs() {
let s = PublicStruct(x: 1)
let _: Int = s.publicMethod()
let _: Int = s.publicProperty
Expand All @@ -42,6 +42,8 @@ func testPublicStruct() {
let _: Int = s.publicTransparentProperty
PublicStruct.publicStaticMethod()
PublicStruct.activeMethod()

let _ = FrozenPublicStruct(1)
}

func testPublicClasses() {
Expand Down
2 changes: 1 addition & 1 deletion test/attr/attr_objc.swift
Original file line number Diff line number Diff line change
Expand Up @@ -983,7 +983,7 @@ class infer_instanceVar1 {

@objc // access-note-move{{infer_instanceVar1.observingAccessorsVar1_}}
var observingAccessorsVar1_: Int {
// CHECK: {{^}} @objc @_hasStorage var observingAccessorsVar1_: Int {
// CHECK: {{^}} {{@_hasStorage @objc|@objc @_hasStorage}} var observingAccessorsVar1_: Int {
willSet {}
// CHECK-NEXT: {{^}} @objc get {
// CHECK-NEXT: return
Expand Down