Skip to content

ASTScope: Redo 'selfDC' computation #33989

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
merged 7 commits into from
Sep 19, 2020
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
61 changes: 3 additions & 58 deletions include/swift/AST/ASTScope.h
Original file line number Diff line number Diff line change
Expand Up @@ -474,16 +474,6 @@ class ASTScopeImpl {
NullablePtr<const GenericParamList> lastListSearched,
DeclConsumer consumer) const;

public:
/// Returns the SelfDC for parent (and possibly ancestor) scopes.
/// A return of None indicates that the previous child (in history) should be
/// asked.
virtual Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const;

/// Returns the context that should be used when a nested scope (e.g. a
/// closure) captures self explicitly.
virtual NullablePtr<DeclContext> capturedSelfDC() const;

protected:
/// Find either locals or members (no scope has both)
/// \param history The scopes visited since the start of lookup (including
Expand Down Expand Up @@ -691,27 +681,6 @@ class Portion {
bool lookupMembersOf(const GenericTypeOrExtensionScope *scope,
ArrayRef<const ASTScopeImpl *>,
ASTScopeImpl::DeclConsumer consumer) const override;

private:
/// A client needs to know if a lookup result required the dynamic implicit
/// self value. It is required if the lookup originates from a method body
/// or a lazy pattern initializer. So, one approach would be to call the
/// consumer to find members right from those scopes. However, because
/// members aren't the first things searched, generics are, that approache
/// ends up duplicating code from the \c GenericTypeOrExtensionScope. So we
/// take the approach of doing those lookups there, and using this function
/// to compute the selfDC from the history.
static NullablePtr<DeclContext>
computeSelfDC(ArrayRef<const ASTScopeImpl *> history);

/// If we find a lookup result that requires the dynamic implict self value,
/// we need to check the nested scopes to see if any closures explicitly
/// captured \c self. In that case, the appropriate selfDC is that of the
/// innermost closure which captures a \c self value from one of this type's
/// methods.
static NullablePtr<DeclContext>
checkNestedScopesForSelfCapture(ArrayRef<const ASTScopeImpl *> history,
size_t start);
};

/// Behavior specific to representing the trailing where clause of a
Expand Down Expand Up @@ -810,7 +779,6 @@ class GenericTypeOrExtensionScope : public ASTScopeImpl {
virtual std::string declKindName() const = 0;
virtual bool doesDeclHaveABody() const;
const char *portionName() const { return portion->portionName; }
Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const override;

protected:
Optional<bool> resolveIsCascadingUseForThisScope(
Expand Down Expand Up @@ -1037,8 +1005,6 @@ class AbstractFunctionDeclScope final : public ASTScopeImpl {

NullablePtr<const void> getReferrent() const override;

static bool shouldCreateAccessorScope(const AccessorDecl *);

protected:
SourceRange
getSourceRangeOfEnclosedParamsOfASTNode(bool omitAssertions) const override;
Expand Down Expand Up @@ -1115,7 +1081,6 @@ class AbstractFunctionBodyScope : public ASTScopeImpl {
}
virtual NullablePtr<Decl> getDeclIfAny() const override { return decl; }
Decl *getDecl() const { return decl; }
static bool isAMethod(const AbstractFunctionDecl *);

NullablePtr<ASTScopeImpl> getParentOfASTAncestorScopesToBeRescued() override;

Expand All @@ -1130,21 +1095,10 @@ class AbstractFunctionBodyScope : public ASTScopeImpl {
SourceRange sourceRangeForDeferredExpansion() const override;
};

/// Body of methods, functions in types.
class MethodBodyScope final : public AbstractFunctionBodyScope {
/// Body of functions and methods.
class FunctionBodyScope final : public AbstractFunctionBodyScope {
public:
MethodBodyScope(AbstractFunctionDecl *e) : AbstractFunctionBodyScope(e) {}
std::string getClassName() const override;
bool lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
DeclConsumer consumer) const override;

Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const override;
};

/// Body of "pure" functions, functions without an implicit "self".
class PureFunctionBodyScope final : public AbstractFunctionBodyScope {
public:
PureFunctionBodyScope(AbstractFunctionDecl *e)
FunctionBodyScope(AbstractFunctionDecl *e)
: AbstractFunctionBodyScope(e) {}
std::string getClassName() const override;
bool lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
Expand Down Expand Up @@ -1317,8 +1271,6 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
virtual NullablePtr<DeclContext> getDeclContext() const override;

Optional<NullablePtr<DeclContext>> computeSelfDCForParent() const override;

protected:
bool lookupLocalsOrMembers(ArrayRef<const ASTScopeImpl *>,
DeclConsumer) const override;
Expand Down Expand Up @@ -1430,13 +1382,6 @@ class ClosureParametersScope final : public ASTScopeImpl {
std::string getClassName() const override;
SourceRange
getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;

/// Since explicit captures of \c self by closures enable the use of implicit
/// \c self, we need to make sure that the appropriate \c self is used as the
/// base decl for these uses (otherwise, the capture would be marked as
/// unused. \c ClosureParametersScope::capturedSelfDC() checks if we have such
/// a capture of self.
NullablePtr<DeclContext> capturedSelfDC() const override;

NullablePtr<ClosureExpr> getClosureIfClosureScope() const override {
return closureExpr;
Expand Down
50 changes: 25 additions & 25 deletions include/swift/AST/Decl.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ namespace swift {
class DynamicSelfType;
class Type;
class Expr;
class CaptureListExpr;
class DeclRefExpr;
class ForeignAsyncConvention;
class ForeignErrorConvention;
Expand Down Expand Up @@ -352,21 +353,13 @@ class alignas(1 << DeclAlignInBits) Decl {
IsStatic : 1
);

SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+1+1+1+1+1+1+1,
SWIFT_INLINE_BITFIELD(VarDecl, AbstractStorageDecl, 1+1+1+1+1+1,
/// Encodes whether this is a 'let' binding.
Introducer : 1,

/// Whether this declaration was an element of a capture list.
IsCaptureList : 1,

/// Whether this declaration captures the 'self' param under the same name.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice clean up.

IsSelfParamCapture : 1,

/// Whether this vardecl has an initial value bound to it in a way
/// that isn't represented in the AST with an initializer in the pattern
/// binding. This happens in cases like "for i in ...", switch cases, etc.
HasNonPatternBindingInit : 1,

/// Whether this is a property used in expressions in the debugger.
/// It is up to the debugger to instruct SIL how to access this variable.
IsDebuggerVar : 1,
Expand Down Expand Up @@ -4892,16 +4885,19 @@ class VarDecl : public AbstractStorageDecl {
};

protected:
PointerUnion<PatternBindingDecl *, Stmt *, VarDecl *> Parent;
PointerUnion<PatternBindingDecl *,
Stmt *,
VarDecl *,
CaptureListExpr *> Parent;

VarDecl(DeclKind kind, bool isStatic, Introducer introducer,
bool isCaptureList, SourceLoc nameLoc, Identifier name,
DeclContext *dc, StorageIsMutable_t supportsMutation);
SourceLoc nameLoc, Identifier name, DeclContext *dc,
StorageIsMutable_t supportsMutation);

public:
VarDecl(bool isStatic, Introducer introducer, bool isCaptureList,
VarDecl(bool isStatic, Introducer introducer,
SourceLoc nameLoc, Identifier name, DeclContext *dc)
: VarDecl(DeclKind::Var, isStatic, introducer, isCaptureList, nameLoc,
: VarDecl(DeclKind::Var, isStatic, introducer, nameLoc,
name, dc, StorageIsMutable_t(introducer == Introducer::Var)) {}

SourceRange getSourceRange() const;
Expand Down Expand Up @@ -5091,25 +5087,29 @@ class VarDecl : public AbstractStorageDecl {
Bits.VarDecl.Introducer = uint8_t(value);
}

CaptureListExpr *getParentCaptureList() const {
if (!Parent)
return nullptr;
return Parent.dyn_cast<CaptureListExpr *>();
}

/// Set \p v to be the pattern produced VarDecl that is the parent of this
/// var decl.
void setParentCaptureList(CaptureListExpr *expr) {
assert(expr != nullptr);
Parent = expr;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can Parent be a const or do VarDecls move from family to family?

}
/// Is this an element in a capture list?
bool isCaptureList() const { return Bits.VarDecl.IsCaptureList; }
bool isCaptureList() const {
return getParentCaptureList() != nullptr;
}

/// Is this a capture of the self param?
bool isSelfParamCapture() const { return Bits.VarDecl.IsSelfParamCapture; }
void setIsSelfParamCapture(bool IsSelfParamCapture = true) {
Bits.VarDecl.IsSelfParamCapture = IsSelfParamCapture;
}

/// Return true if this vardecl has an initial value bound to it in a way
/// that isn't represented in the AST with an initializer in the pattern
/// binding. This happens in cases like "for i in ...", switch cases, etc.
bool hasNonPatternBindingInit() const {
return Bits.VarDecl.HasNonPatternBindingInit;
}
void setHasNonPatternBindingInit(bool V = true) {
Bits.VarDecl.HasNonPatternBindingInit = V;
}

/// Determines if this var has an initializer expression that should be
/// exposed to clients.
/// There's a very narrow case when we would: if the decl is an instance
Expand Down
6 changes: 0 additions & 6 deletions include/swift/AST/Expr.h
Original file line number Diff line number Diff line change
Expand Up @@ -3941,12 +3941,6 @@ class ClosureExpr : public AbstractClosureExpr {
VarDecl *getCapturedSelfDecl() const {
return CapturedSelfDecl;
}

/// Whether this closure captures the \c self param in its body in such a
/// way that implicit \c self is enabled within its body (i.e. \c self is
/// captured non-weakly).
bool capturesSelfEnablingImplictSelf() const;


/// Get the type checking state of this closure's body.
BodyState getBodyState() const {
Expand Down
4 changes: 2 additions & 2 deletions include/swift/AST/NameLookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -633,7 +633,7 @@ class AbstractASTScopeDeclConsumer {

/// Eventually this functionality should move into ASTScopeLookup
virtual bool
lookInMembers(NullablePtr<DeclContext> selfDC, DeclContext *const scopeDC,
lookInMembers(DeclContext *const scopeDC,
NominalTypeDecl *const nominal,
function_ref<bool(Optional<bool>)> calculateIsCascadingUse) = 0;

Expand All @@ -656,7 +656,7 @@ class ASTScopeDeclGatherer : public AbstractASTScopeDeclConsumer {
NullablePtr<DeclContext> baseDC = nullptr) override;

/// Eventually this functionality should move into ASTScopeLookup
bool lookInMembers(NullablePtr<DeclContext>, DeclContext *const,
bool lookInMembers(DeclContext *const,
NominalTypeDecl *const,
function_ref<bool(Optional<bool>)>) override {
return false;
Expand Down
8 changes: 0 additions & 8 deletions include/swift/AST/Pattern.h
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,6 @@ class alignas(8) Pattern {

bool isNeverDefaultInitializable() const;

/// Mark all vardecls in this pattern as having non-pattern initial
/// values bound into them.
void markHasNonPatternBindingInit() {
forEachVariable([&](VarDecl *VD) {
VD->setHasNonPatternBindingInit();
});
}

/// Mark all vardecls in this pattern as having an owning statement for
/// the pattern.
void markOwnedByStatement(Stmt *S) {
Expand Down
2 changes: 2 additions & 0 deletions include/swift/AST/TypeAlignments.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace swift {
class ASTContext;
class AttributeBase;
class BraceStmt;
class CaptureListExpr;
class Decl;
class DeclContext;
class Expr;
Expand Down Expand Up @@ -112,6 +113,7 @@ LLVM_DECLARE_TYPE_ALIGNMENT(swift::BraceStmt, swift::StmtAlignInBits)
LLVM_DECLARE_TYPE_ALIGNMENT(swift::ASTContext, 2);
LLVM_DECLARE_TYPE_ALIGNMENT(swift::DeclContext, swift::DeclContextAlignInBits)
LLVM_DECLARE_TYPE_ALIGNMENT(swift::Expr, swift::ExprAlignInBits)
LLVM_DECLARE_TYPE_ALIGNMENT(swift::CaptureListExpr, swift::ExprAlignInBits)
LLVM_DECLARE_TYPE_ALIGNMENT(swift::AbstractClosureExpr, swift::ExprAlignInBits)
LLVM_DECLARE_TYPE_ALIGNMENT(swift::OpaqueValueExpr, swift::ExprAlignInBits)
LLVM_DECLARE_TYPE_ALIGNMENT(swift::ProtocolConformance, swift::DeclAlignInBits)
Expand Down
2 changes: 0 additions & 2 deletions lib/AST/ASTDumper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -842,8 +842,6 @@ namespace {
printCommon(VD, "var_decl");
if (VD->isLet())
PrintWithColorRAII(OS, DeclModifierColor) << " let";
if (VD->hasNonPatternBindingInit())
PrintWithColorRAII(OS, DeclModifierColor) << " non_pattern_init";
if (VD->getAttrs().hasAttribute<LazyAttr>())
PrintWithColorRAII(OS, DeclModifierColor) << " lazy";
printStorageImpl(VD);
Expand Down
17 changes: 1 addition & 16 deletions lib/AST/ASTScope.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,20 +118,6 @@ Stmt *LabeledConditionalStmtScope::getStmt() const {
return getLabeledConditionalStmt();
}

bool AbstractFunctionBodyScope::isAMethod(
const AbstractFunctionDecl *const afd) {
// What makes this interesting is that a method named "init" which is not
// in a nominal type or extension decl body still gets an implicit self
// parameter (even though the program is illegal).
// So when choosing between creating a MethodBodyScope and a
// PureFunctionBodyScope do we go by the enclosing Decl (i.e.
// "afd->getDeclContext()->isTypeContext()") or by
// "bool(afd->getImplicitSelfDecl())"?
//
// Since the code uses \c getImplicitSelfDecl, use that.
return afd->getImplicitSelfDecl();
}

#pragma mark getLabeledConditionalStmt
LabeledConditionalStmt *IfStmtScope::getLabeledConditionalStmt() const {
return stmt;
Expand Down Expand Up @@ -218,8 +204,7 @@ DEFINE_GET_CLASS_NAME(ASTSourceFileScope)
DEFINE_GET_CLASS_NAME(GenericParamScope)
DEFINE_GET_CLASS_NAME(AbstractFunctionDeclScope)
DEFINE_GET_CLASS_NAME(ParameterListScope)
DEFINE_GET_CLASS_NAME(MethodBodyScope)
DEFINE_GET_CLASS_NAME(PureFunctionBodyScope)
DEFINE_GET_CLASS_NAME(FunctionBodyScope)
DEFINE_GET_CLASS_NAME(DefaultArgumentInitializerScope)
DEFINE_GET_CLASS_NAME(AttachedPropertyWrapperScope)
DEFINE_GET_CLASS_NAME(PatternEntryDeclScope)
Expand Down
11 changes: 1 addition & 10 deletions lib/AST/ASTScopeCreation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1416,12 +1416,8 @@ void AbstractFunctionDeclScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
// We create body scopes when there is no body for source kit to complete
// erroneous code in bodies.
if (decl->getBodySourceRange().isValid()) {
if (AbstractFunctionBodyScope::isAMethod(decl))
scopeCreator.constructExpandAndInsertUncheckable<MethodBodyScope>(leaf,
scopeCreator.constructExpandAndInsertUncheckable<FunctionBodyScope>(leaf,
decl);
else
scopeCreator.constructExpandAndInsertUncheckable<PureFunctionBodyScope>(
leaf, decl);
}
}

Expand Down Expand Up @@ -2004,11 +2000,6 @@ ASTScopeImpl::rescueASTAncestorScopesForReuseFromMe() {
return astAncestorScopes;
}

bool AbstractFunctionDeclScope::shouldCreateAccessorScope(
const AccessorDecl *const ad) {
return isLocalizable(ad);
}

#pragma mark verification

namespace {
Expand Down
Loading