Skip to content

Revert "Implement backward-compatible closure capture behavior with parser lookup disabled" #34163

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

Closed
wants to merge 1 commit into from
Closed
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
31 changes: 10 additions & 21 deletions include/swift/AST/ASTScope.h
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
#define SWIFT_AST_AST_SCOPE_H

#include "swift/AST/ASTNode.h"
#include "swift/AST/NameLookup.h"
#include "swift/AST/NameLookup.h" // for DeclVisibilityKind
#include "swift/AST/SimpleRequest.h"
#include "swift/Basic/Compiler.h"
#include "swift/Basic/Debug.h"
Expand Down Expand Up @@ -444,6 +444,7 @@ class ASTScopeImpl {
// It is not an instance variable or inherited type.

static bool lookupLocalBindingsInPattern(const Pattern *p,
DeclVisibilityKind vis,
DeclConsumer consumer);

/// When lookup must stop before the outermost scope, return the scope to stop
Expand Down Expand Up @@ -1023,10 +1024,10 @@ class AbstractPatternEntryScope : public ASTScopeImpl {
public:
PatternBindingDecl *const decl;
const unsigned patternEntryIndex;
const bool isLocalBinding;
const DeclVisibilityKind vis;

AbstractPatternEntryScope(PatternBindingDecl *, unsigned entryIndex,
bool);
DeclVisibilityKind);
virtual ~AbstractPatternEntryScope() {}

const PatternBindingEntry &getPatternEntry() const;
Expand All @@ -1043,8 +1044,8 @@ class AbstractPatternEntryScope : public ASTScopeImpl {
class PatternEntryDeclScope final : public AbstractPatternEntryScope {
public:
PatternEntryDeclScope(PatternBindingDecl *pbDecl, unsigned entryIndex,
bool isLocalBinding)
: AbstractPatternEntryScope(pbDecl, entryIndex, isLocalBinding) {}
DeclVisibilityKind vis)
: AbstractPatternEntryScope(pbDecl, entryIndex, vis) {}
virtual ~PatternEntryDeclScope() {}

protected:
Expand All @@ -1071,8 +1072,8 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {

public:
PatternEntryInitializerScope(PatternBindingDecl *pbDecl, unsigned entryIndex,
bool isLocalBinding)
: AbstractPatternEntryScope(pbDecl, entryIndex, isLocalBinding),
DeclVisibilityKind vis)
: AbstractPatternEntryScope(pbDecl, entryIndex, vis),
initAsWrittenWhenCreated(pbDecl->getOriginalInit(entryIndex)) {}
virtual ~PatternEntryInitializerScope() {}

Expand Down Expand Up @@ -1657,22 +1658,10 @@ class CaseStmtBodyScope final : public ASTScopeImpl {
};

class BraceStmtScope final : public AbstractStmtScope {
BraceStmt *const stmt;

/// Declarations which are in scope from the beginning of the statement.
SmallVector<ValueDecl *, 2> localFuncsAndTypes;

/// Declarations that are normally in scope only after their
/// definition.
SmallVector<VarDecl *, 2> localVars;

public:
BraceStmtScope(BraceStmt *e,
SmallVector<ValueDecl *, 2> localFuncsAndTypes,
SmallVector<VarDecl *, 2> localVars)
: stmt(e),
localFuncsAndTypes(localFuncsAndTypes),
localVars(localVars) {}
BraceStmt *const stmt;
BraceStmtScope(BraceStmt *e) : stmt(e) {}
virtual ~BraceStmtScope() {}

protected:
Expand Down
12 changes: 2 additions & 10 deletions include/swift/AST/NameLookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -603,7 +603,7 @@ class AbstractASTScopeDeclConsumer {
/// of type -vs- instance lookup results.
///
/// \return true if the lookup should be stopped at this point.
virtual bool consume(ArrayRef<ValueDecl *> values,
virtual bool consume(ArrayRef<ValueDecl *> values, DeclVisibilityKind vis,
NullablePtr<DeclContext> baseDC = nullptr) = 0;

/// Look for members of a nominal type or extension scope.
Expand All @@ -613,14 +613,6 @@ class AbstractASTScopeDeclConsumer {
lookInMembers(DeclContext *const scopeDC,
NominalTypeDecl *const nominal) = 0;

/// Called for local VarDecls that might not yet be in scope.
///
/// Note that the set of VarDecls visited here are going to be a
/// superset of those visited in consume().
virtual bool consumePossiblyNotInScope(ArrayRef<VarDecl *> values) {
return false;
}

/// Called right before looking at the parent scope of a BraceStmt.
///
/// \return true if the lookup should be stopped at this point.
Expand All @@ -644,7 +636,7 @@ class ASTScopeDeclGatherer : public AbstractASTScopeDeclConsumer {
public:
virtual ~ASTScopeDeclGatherer() = default;

bool consume(ArrayRef<ValueDecl *> values,
bool consume(ArrayRef<ValueDecl *> values, DeclVisibilityKind vis,
NullablePtr<DeclContext> baseDC = nullptr) override;

/// Eventually this functionality should move into ASTScopeLookup
Expand Down
51 changes: 15 additions & 36 deletions lib/AST/ASTScopeCreation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -667,30 +667,10 @@ class NodeAdder

NullablePtr<ASTScopeImpl> visitBraceStmt(BraceStmt *bs, ASTScopeImpl *p,
ScopeCreator &scopeCreator) {
SmallVector<ValueDecl *, 2> localFuncsAndTypes;
SmallVector<VarDecl *, 2> localVars;

// All types and functions are visible anywhere within a brace statement
// scope. When ordering matters (i.e. var decl) we will have split the brace
// statement into nested scopes.
for (auto braceElement : bs->getElements()) {
if (auto localBinding = braceElement.dyn_cast<Decl *>()) {
if (auto *vd = dyn_cast<ValueDecl>(localBinding)) {
if (isa<FuncDecl>(vd) || isa<TypeDecl>(vd)) {
localFuncsAndTypes.push_back(vd);
} else if (auto *var = dyn_cast<VarDecl>(localBinding)) {
localVars.push_back(var);
}
}
}
}

auto maybeBraceScope =
scopeCreator.ifUniqueConstructExpandAndInsert<BraceStmtScope>(
p, bs, std::move(localFuncsAndTypes), std::move(localVars));
scopeCreator.ifUniqueConstructExpandAndInsert<BraceStmtScope>(p, bs);
if (auto *s = scopeCreator.getASTContext().Stats)
++s->getFrontendCounters().NumBraceStmtASTScopes;

return maybeBraceScope.getPtrOr(p);
}

Expand All @@ -701,24 +681,24 @@ class NodeAdder
if (auto *var = patternBinding->getSingleVar())
scopeCreator.addChildrenForKnownAttributes(var, parentScope);

const bool isLocalBinding = patternBinding->getDeclContext()->isLocalContext();

const DeclVisibilityKind vis =
isLocalBinding ? DeclVisibilityKind::LocalVariable
: DeclVisibilityKind::MemberOfCurrentNominal;
auto *insertionPoint = parentScope;
for (auto i : range(patternBinding->getNumPatternEntries())) {
bool isLocalBinding = false;
if (auto *varDecl = patternBinding->getAnchoringVarDecl(i)) {
isLocalBinding = varDecl->getDeclContext()->isLocalContext();
}

insertionPoint =
scopeCreator
.ifUniqueConstructExpandAndInsert<PatternEntryDeclScope>(
insertionPoint, patternBinding, i, isLocalBinding)
insertionPoint, patternBinding, i, vis)
.getPtrOr(insertionPoint);

ASTScopeAssert(isLocalBinding || insertionPoint == parentScope,
"Bindings at the top-level or members of types should "
"not change the insertion point");
}

ASTScopeAssert(isLocalBinding || insertionPoint == parentScope,
"Bindings at the top-level or members of types should "
"not change the insertion point");

return insertionPoint;
}

Expand Down Expand Up @@ -991,7 +971,7 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
"Original inits are always after the '='");
scopeCreator
.constructExpandAndInsertUncheckable<PatternEntryInitializerScope>(
this, decl, patternEntryIndex, isLocalBinding);
this, decl, patternEntryIndex, vis);
}

// Add accessors for the variables in this pattern.
Expand All @@ -1002,7 +982,7 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
// In local context, the PatternEntryDeclScope becomes the insertion point, so
// that all any bindings introduecd by the pattern are in scope for subsequent
// lookups.
if (isLocalBinding)
if (vis == DeclVisibilityKind::LocalVariable)
return {this, "All code that follows is inside this scope"};

return {getParent().get(), "Global and type members do not introduce scopes"};
Expand Down Expand Up @@ -1378,9 +1358,8 @@ ASTScopeImpl *LabeledConditionalStmtScope::createNestedConditionalClauseScopes(

AbstractPatternEntryScope::AbstractPatternEntryScope(
PatternBindingDecl *declBeingScoped, unsigned entryIndex,
bool isLocalBinding)
: decl(declBeingScoped), patternEntryIndex(entryIndex),
isLocalBinding(isLocalBinding) {
DeclVisibilityKind vis)
: decl(declBeingScoped), patternEntryIndex(entryIndex), vis(vis) {
ASTScopeAssert(entryIndex < declBeingScoped->getPatternList().size(),
"out of bounds");
}
Expand Down
62 changes: 40 additions & 22 deletions lib/AST/ASTScopeLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ bool ASTScopeImpl::lookInGenericParametersOf(
SmallVector<ValueDecl *, 32> bindings;
for (auto *param : paramList.get()->getParams())
bindings.push_back(param);
if (consumer.consume(bindings))
if (consumer.consume(bindings, DeclVisibilityKind::GenericParameter))
return true;
return false;
}
Expand Down Expand Up @@ -289,26 +289,28 @@ PatternEntryInitializerScope::getLookupParent() const {

bool GenericParamScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
auto *param = paramList->getParams()[index];
return consumer.consume({param});
return consumer.consume({param}, DeclVisibilityKind::GenericParameter);
}

bool PatternEntryDeclScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
if (!isLocalBinding)
return false;
return lookupLocalBindingsInPattern(getPattern(), consumer);
if (vis != DeclVisibilityKind::LocalVariable)
return false; // look in self type will find this later
return lookupLocalBindingsInPattern(getPattern(), vis, consumer);
}

bool ForEachPatternScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
return lookupLocalBindingsInPattern(stmt->getPattern(), consumer);
return lookupLocalBindingsInPattern(
stmt->getPattern(), DeclVisibilityKind::LocalVariable, consumer);
}

bool CaseLabelItemScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
return lookupLocalBindingsInPattern(item.getPattern(), consumer);
return lookupLocalBindingsInPattern(
item.getPattern(), DeclVisibilityKind::LocalVariable, consumer);
}

bool CaseStmtBodyScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
for (auto *var : stmt->getCaseBodyVariablesOrEmptyArray())
if (consumer.consume({var}))
if (consumer.consume({var}, DeclVisibilityKind::LocalVariable))
return true;

return false;
Expand All @@ -318,12 +320,13 @@ bool FunctionBodyScope::lookupLocalsOrMembers(
DeclConsumer consumer) const {
if (auto *paramList = decl->getParameters()) {
for (auto *paramDecl : *paramList)
if (consumer.consume({paramDecl}))
if (consumer.consume({paramDecl}, DeclVisibilityKind::FunctionParameter))
return true;
}

if (decl->getDeclContext()->isTypeContext()) {
return consumer.consume({decl->getImplicitSelfDecl()});
return consumer.consume({decl->getImplicitSelfDecl()},
DeclVisibilityKind::FunctionParameter);
}

// Consider \c var t: T { (did/will/)get/set { ... t }}
Expand All @@ -332,7 +335,7 @@ bool FunctionBodyScope::lookupLocalsOrMembers(
// then t needs to be found as a local binding:
if (auto *accessor = dyn_cast<AccessorDecl>(decl)) {
if (auto *storage = accessor->getStorage())
if (consumer.consume({storage}))
if (consumer.consume({storage}, DeclVisibilityKind::LocalVariable))
return true;
}

Expand All @@ -343,7 +346,7 @@ bool SpecializeAttributeScope::lookupLocalsOrMembers(
DeclConsumer consumer) const {
if (auto *params = whatWasSpecialized->getGenericParams())
for (auto *param : params->getParams())
if (consumer.consume({param}))
if (consumer.consume({param}, DeclVisibilityKind::GenericParameter))
return true;
return false;
}
Expand All @@ -353,7 +356,7 @@ bool DifferentiableAttributeScope::lookupLocalsOrMembers(
auto visitAbstractFunctionDecl = [&](AbstractFunctionDecl *afd) {
if (auto *params = afd->getGenericParams())
for (auto *param : params->getParams())
if (consumer.consume({param}))
if (consumer.consume({param}, DeclVisibilityKind::GenericParameter))
return true;
return false;
};
Expand All @@ -368,10 +371,20 @@ bool DifferentiableAttributeScope::lookupLocalsOrMembers(
}

bool BraceStmtScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
if (consumer.consume(localFuncsAndTypes))
return true;

if (consumer.consumePossiblyNotInScope(localVars))
// All types and functions are visible anywhere within a brace statement
// scope. When ordering matters (i.e. var decl) we will have split the brace
// statement into nested scopes.
//
// Don't stop at the first one, there may be local funcs with same base name
// and want them all.
SmallVector<ValueDecl *, 32> localBindings;
for (auto braceElement : stmt->getElements()) {
if (auto localBinding = braceElement.dyn_cast<Decl *>()) {
if (auto *vd = dyn_cast<ValueDecl>(localBinding))
localBindings.push_back(vd);
}
}
if (consumer.consume(localBindings, DeclVisibilityKind::LocalVariable))
return true;

if (consumer.finishLookupInBraceStmt(stmt))
Expand All @@ -387,15 +400,18 @@ bool PatternEntryInitializerScope::lookupLocalsOrMembers(
decl->getInitContext(0));
if (initContext) {
if (auto *selfParam = initContext->getImplicitSelfDecl()) {
return consumer.consume({selfParam});
return consumer.consume({selfParam},
DeclVisibilityKind::FunctionParameter);
}
}
return false;
}

bool CaptureListScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
for (auto &e : expr->getCaptureList()) {
if (consumer.consume({e.Var}))
if (consumer.consume(
{e.Var},
DeclVisibilityKind::LocalVariable)) // or FunctionParameter??
return true;
}
return false;
Expand All @@ -404,24 +420,26 @@ bool CaptureListScope::lookupLocalsOrMembers(DeclConsumer consumer) const {
bool ClosureParametersScope::lookupLocalsOrMembers(
DeclConsumer consumer) const {
for (auto param : *closureExpr->getParameters())
if (consumer.consume({param}))
if (consumer.consume({param}, DeclVisibilityKind::FunctionParameter))
return true;
return false;
}

bool ConditionalClausePatternUseScope::lookupLocalsOrMembers(
DeclConsumer consumer) const {
return lookupLocalBindingsInPattern(pattern, consumer);
return lookupLocalBindingsInPattern(
pattern, DeclVisibilityKind::LocalVariable, consumer);
}

bool ASTScopeImpl::lookupLocalBindingsInPattern(const Pattern *p,
DeclVisibilityKind vis,
DeclConsumer consumer) {
if (!p)
return false;
bool isDone = false;
p->forEachVariable([&](VarDecl *var) {
if (!isDone)
isDone = consumer.consume({var});
isDone = consumer.consume({var}, vis);
});
return isDone;
}
Expand Down
3 changes: 1 addition & 2 deletions lib/AST/Decl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1447,8 +1447,7 @@ void PatternBindingEntry::setInit(Expr *E) {
VarDecl *PatternBindingEntry::getAnchoringVarDecl() const {
SmallVector<VarDecl *, 8> variables;
getPattern()->collectVariables(variables);
if (variables.empty())
return nullptr;
assert(!variables.empty());
return variables[0];
}

Expand Down
Loading