Skip to content

Prepare to disable parser lookup #34158

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
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
28 changes: 17 additions & 11 deletions include/swift/AST/ASTScope.h
Original file line number Diff line number Diff line change
Expand Up @@ -1023,10 +1023,8 @@ class AbstractPatternEntryScope : public ASTScopeImpl {
public:
PatternBindingDecl *const decl;
const unsigned patternEntryIndex;
const bool isLocalBinding;

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

const PatternBindingEntry &getPatternEntry() const;
Expand All @@ -1041,10 +1039,14 @@ class AbstractPatternEntryScope : public ASTScopeImpl {
};

class PatternEntryDeclScope final : public AbstractPatternEntryScope {
const bool isLocalBinding;
Optional<SourceLoc> endLoc;

public:
PatternEntryDeclScope(PatternBindingDecl *pbDecl, unsigned entryIndex,
bool isLocalBinding)
: AbstractPatternEntryScope(pbDecl, entryIndex, isLocalBinding) {}
bool isLocalBinding, Optional<SourceLoc> endLoc)
: AbstractPatternEntryScope(pbDecl, entryIndex),
isLocalBinding(isLocalBinding), endLoc(endLoc) {}
virtual ~PatternEntryDeclScope() {}

protected:
Expand All @@ -1070,9 +1072,8 @@ class PatternEntryInitializerScope final : public AbstractPatternEntryScope {
Expr *initAsWrittenWhenCreated;

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

Expand Down Expand Up @@ -1400,7 +1401,8 @@ class WhileStmtScope final : public LabeledConditionalStmtScope {
class GuardStmtScope final : public LabeledConditionalStmtScope {
public:
GuardStmt *const stmt;
GuardStmtScope(GuardStmt *e) : stmt(e) {}
SourceLoc endLoc;
GuardStmtScope(GuardStmt *e, SourceLoc endLoc) : stmt(e), endLoc(endLoc) {}
virtual ~GuardStmtScope() {}

protected:
Expand All @@ -1413,6 +1415,8 @@ class GuardStmtScope final : public LabeledConditionalStmtScope {
public:
std::string getClassName() const override;
LabeledConditionalStmt *getLabeledConditionalStmt() const override;
SourceRange
getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
};

/// A scope after a guard statement that follows lookups into the conditions
Expand All @@ -1427,9 +1431,11 @@ class LookupParentDiversionScope final : public ASTScopeImpl {
public:
ASTScopeImpl *const lookupParent;
const SourceLoc startLoc;
const SourceLoc endLoc;

LookupParentDiversionScope(ASTScopeImpl *lookupParent, SourceLoc startLoc)
: lookupParent(lookupParent), startLoc(startLoc) {}
LookupParentDiversionScope(ASTScopeImpl *lookupParent,
SourceLoc startLoc, SourceLoc endLoc)
: lookupParent(lookupParent), startLoc(startLoc), endLoc(endLoc) {}

SourceRange
getSourceRangeOfThisASTNode(bool omitAssertions = false) const override;
Expand Down
6 changes: 5 additions & 1 deletion include/swift/Option/Options.td
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,11 @@ def scan_clang_dependencies : Flag<["-"], "scan-clang-dependencies">,

def disable_parser_lookup : Flag<["-"], "disable-parser-lookup">,
Flags<[FrontendOption]>,
HelpText<"Disable parser lookup & use ast scope lookup only (experimental)">;
HelpText<"Disable parser lookup & use ASTScope lookup only (experimental)">;

def enable_parser_lookup : Flag<["-"], "enable-parser-lookup">,
Flags<[FrontendOption]>,
HelpText<"Enable parser lookup">;

def enable_request_based_incremental_dependencies : Flag<["-"],
"enable-request-based-incremental-dependencies">,
Expand Down
136 changes: 90 additions & 46 deletions lib/AST/ASTScopeCreation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -213,14 +213,17 @@ class ScopeCreator final {

/// Given an array of ASTNodes or Decl pointers, add them
/// Return the resultant insertionPoint
///
/// \param endLoc The end location for any "scopes until the end" that
/// we introduce here, such as PatternEntryDeclScope and GuardStmtScope
ASTScopeImpl *
addSiblingsToScopeTree(ASTScopeImpl *const insertionPoint,
ASTScopeImpl *const organicInsertionPoint,
ArrayRef<ASTNode> nodesOrDeclsToAdd) {
ArrayRef<ASTNode> nodesOrDeclsToAdd,
Optional<SourceLoc> endLoc) {
auto *ip = insertionPoint;
for (auto nd : sortBySourceRange(cull(nodesOrDeclsToAdd))) {
for (auto nd : nodesOrDeclsToAdd) {
auto *const newIP =
addToScopeTreeAndReturnInsertionPoint(nd, ip).getPtrOr(ip);
addToScopeTreeAndReturnInsertionPoint(nd, ip, endLoc).getPtrOr(ip);
ip = newIP;
}
return ip;
Expand All @@ -229,12 +232,16 @@ class ScopeCreator final {
public:
/// For each of searching, call this unless the insertion point is needed
void addToScopeTree(ASTNode n, ASTScopeImpl *parent) {
(void)addToScopeTreeAndReturnInsertionPoint(n, parent);
(void)addToScopeTreeAndReturnInsertionPoint(n, parent, None);
}
/// Return new insertion point if the scope was not a duplicate
/// For ease of searching, don't call unless insertion point is needed
///
/// \param endLoc The end location for any "scopes until the end" that
/// we introduce here, such as PatternEntryDeclScope and GuardStmtScope
NullablePtr<ASTScopeImpl>
addToScopeTreeAndReturnInsertionPoint(ASTNode, ASTScopeImpl *parent);
addToScopeTreeAndReturnInsertionPoint(ASTNode, ASTScopeImpl *parent,
Optional<SourceLoc> endLoc);

bool isWorthTryingToCreateScopeFor(ASTNode n) const {
if (!n)
Expand Down Expand Up @@ -419,9 +426,18 @@ class ScopeCreator final {
void addChildrenForKnownAttributes(ValueDecl *decl,
ASTScopeImpl *parent);

public:
/// Add PatternEntryDeclScopes for each pattern binding entry.
///
/// Returns the new insertion point.
///
/// \param endLoc Must be valid iff the pattern binding is in a local
/// scope, in which case this is the last source location where the
/// pattern bindings are going to be visible.
NullablePtr<ASTScopeImpl>
addPatternBindingToScopeTree(PatternBindingDecl *patternBinding,
ASTScopeImpl *parent,
Optional<SourceLoc> endLoc);

private:
/// Remove VarDecls because we'll find them when we expand the
/// PatternBindingDecls. Remove EnunCases
/// because they overlap EnumElements and AST includes the elements in the
Expand Down Expand Up @@ -463,7 +479,6 @@ class ScopeCreator final {
return -1 == signum;
}

public:
SWIFT_DEBUG_DUMP { print(llvm::errs()); }

void print(raw_ostream &out) const {
Expand Down Expand Up @@ -545,7 +560,10 @@ class NodeAdder
: public ASTVisitor<NodeAdder, NullablePtr<ASTScopeImpl>,
NullablePtr<ASTScopeImpl>, NullablePtr<ASTScopeImpl>,
void, void, void, ASTScopeImpl *, ScopeCreator &> {
Optional<SourceLoc> endLoc;

public:
explicit NodeAdder(Optional<SourceLoc> endLoc) : endLoc(endLoc) {}

#pragma mark ASTNodes that do not create scopes

Expand Down Expand Up @@ -637,7 +655,9 @@ class NodeAdder
// the deferred nodes.
NullablePtr<ASTScopeImpl> visitGuardStmt(GuardStmt *e, ASTScopeImpl *p,
ScopeCreator &scopeCreator) {
return scopeCreator.ifUniqueConstructExpandAndInsert<GuardStmtScope>(p, e);
ASTScopeAssert(endLoc.hasValue(), "GuardStmt outside of a BraceStmt?");
return scopeCreator.ifUniqueConstructExpandAndInsert<GuardStmtScope>(
p, e, *endLoc);
}
NullablePtr<ASTScopeImpl> visitTopLevelCodeDecl(TopLevelCodeDecl *d,
ASTScopeImpl *p,
Expand Down Expand Up @@ -698,28 +718,8 @@ class NodeAdder
visitPatternBindingDecl(PatternBindingDecl *patternBinding,
ASTScopeImpl *parentScope,
ScopeCreator &scopeCreator) {
if (auto *var = patternBinding->getSingleVar())
scopeCreator.addChildrenForKnownAttributes(var, parentScope);

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)
.getPtrOr(insertionPoint);

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

return insertionPoint;
return scopeCreator.addPatternBindingToScopeTree(
patternBinding, parentScope, endLoc);
}

NullablePtr<ASTScopeImpl> visitEnumElementDecl(EnumElementDecl *eed,
Expand Down Expand Up @@ -773,15 +773,18 @@ class NodeAdder
// NodeAdder
NullablePtr<ASTScopeImpl>
ScopeCreator::addToScopeTreeAndReturnInsertionPoint(ASTNode n,
ASTScopeImpl *parent) {
ASTScopeImpl *parent,
Optional<SourceLoc> endLoc) {
if (!isWorthTryingToCreateScopeFor(n))
return parent;

NodeAdder adder(endLoc);
if (auto *p = n.dyn_cast<Decl *>())
return NodeAdder().visit(p, parent, *this);
return adder.visit(p, parent, *this);
if (auto *p = n.dyn_cast<Expr *>())
return NodeAdder().visit(p, parent, *this);
return adder.visit(p, parent, *this);
auto *p = n.get<Stmt *>();
return NodeAdder().visit(p, parent, *this);
return adder.visit(p, parent, *this);
}

void ScopeCreator::addChildrenForAllLocalizableAccessorsInSourceOrder(
Expand Down Expand Up @@ -831,6 +834,41 @@ void ScopeCreator::addChildrenForKnownAttributes(ValueDecl *decl,
}
}

NullablePtr<ASTScopeImpl>
ScopeCreator::addPatternBindingToScopeTree(PatternBindingDecl *patternBinding,
ASTScopeImpl *parentScope,
Optional<SourceLoc> endLoc) {
if (auto *var = patternBinding->getSingleVar())
addChildrenForKnownAttributes(var, parentScope);

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

Optional<SourceLoc> endLocForBinding = None;
if (isLocalBinding) {
endLocForBinding = endLoc;
ASTScopeAssert(endLoc.hasValue() && endLoc->isValid(),
"PatternBindingDecl in local context outside of BraceStmt?");
}

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

Comment on lines +858 to +863
Copy link
Contributor

Choose a reason for hiding this comment

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

Glad we spoke about this offline. Makes perfect sense.

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

return insertionPoint;
}

#pragma mark creation helpers

void ASTScopeImpl::addChild(ASTScopeImpl *child, ASTContext &ctx) {
Expand Down Expand Up @@ -950,7 +988,10 @@ ASTSourceFileScope::expandAScopeThatCreatesANewInsertionPoint(
// Assume that decls are only added at the end, in source order
std::vector<ASTNode> newNodes(decls.begin(), decls.end());
insertionPoint =
scopeCreator.addSiblingsToScopeTree(insertionPoint, this, newNodes);
scopeCreator.addSiblingsToScopeTree(insertionPoint,
scopeCreator.sortBySourceRange(
scopeCreator.cull(newNodes)),
None);
// Too slow to perform all the time:
// ASTScopeAssert(scopeCreator->containsAllDeclContextsFromAST(),
// "ASTScope tree missed some DeclContexts or made some up");
Expand Down Expand Up @@ -991,7 +1032,7 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
"Original inits are always after the '='");
scopeCreator
.constructExpandAndInsertUncheckable<PatternEntryInitializerScope>(
this, decl, patternEntryIndex, isLocalBinding);
this, decl, patternEntryIndex);
}

// Add accessors for the variables in this pattern.
Expand Down Expand Up @@ -1050,7 +1091,7 @@ GuardStmtScope::expandAScopeThatCreatesANewInsertionPoint(ScopeCreator &
auto *const lookupParentDiversionScope =
scopeCreator
.constructExpandAndInsertUncheckable<LookupParentDiversionScope>(
this, conditionLookupParent, stmt->getEndLoc());
this, conditionLookupParent, stmt->getEndLoc(), endLoc);
return {lookupParentDiversionScope,
"Succeeding code must be in scope of guard variables"};
}
Expand All @@ -1068,7 +1109,11 @@ BraceStmtScope::expandAScopeThatCreatesANewInsertionPoint(
// TODO: remove the sort after fixing parser to create brace statement
// elements in source order
auto *insertionPoint =
scopeCreator.addSiblingsToScopeTree(this, this, stmt->getElements());
scopeCreator.addSiblingsToScopeTree(this,
scopeCreator.sortBySourceRange(
scopeCreator.cull(
stmt->getElements())),
stmt->getEndLoc());
if (auto *s = scopeCreator.getASTContext().Stats)
++s->getFrontendCounters().NumBraceStmtASTScopeExpansions;
return {
Expand All @@ -1082,7 +1127,7 @@ TopLevelCodeScope::expandAScopeThatCreatesANewInsertionPoint(ScopeCreator &

if (auto *body =
scopeCreator
.addToScopeTreeAndReturnInsertionPoint(decl->getBody(), this)
.addToScopeTreeAndReturnInsertionPoint(decl->getBody(), this, None)
.getPtrOrNull())
return {body, "So next top level code scope and put its decls in its body "
"under a guard statement scope (etc) from the last top level "
Expand Down Expand Up @@ -1377,10 +1422,8 @@ ASTScopeImpl *LabeledConditionalStmtScope::createNestedConditionalClauseScopes(
}

AbstractPatternEntryScope::AbstractPatternEntryScope(
PatternBindingDecl *declBeingScoped, unsigned entryIndex,
bool isLocalBinding)
: decl(declBeingScoped), patternEntryIndex(entryIndex),
isLocalBinding(isLocalBinding) {
PatternBindingDecl *declBeingScoped, unsigned entryIndex)
: decl(declBeingScoped), patternEntryIndex(entryIndex) {
ASTScopeAssert(entryIndex < declBeingScoped->getPatternList().size(),
"out of bounds");
}
Expand Down Expand Up @@ -1414,7 +1457,8 @@ void GenericTypeOrExtensionScope::expandBody(ScopeCreator &) {}

void IterableTypeScope::expandBody(ScopeCreator &scopeCreator) {
auto nodes = asNodeVector(getIterableDeclContext().get()->getMembers());
scopeCreator.addSiblingsToScopeTree(this, this, nodes);
nodes = scopeCreator.sortBySourceRange(scopeCreator.cull(nodes));
scopeCreator.addSiblingsToScopeTree(this, nodes, None);
if (auto *s = scopeCreator.getASTContext().Stats)
++s->getFrontendCounters().NumIterableTypeBodyASTScopeExpansions;
}
Expand Down
15 changes: 13 additions & 2 deletions lib/AST/ASTScopeSourceRange.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,13 @@ SourceRange DefaultArgumentInitializerScope::getSourceRangeOfThisASTNode(

SourceRange PatternEntryDeclScope::getSourceRangeOfThisASTNode(
const bool omitAssertions) const {
return getPatternEntry().getSourceRange();
SourceRange range = getPatternEntry().getSourceRange();
if (endLoc.hasValue()) {
ASTScopeAssert(endLoc->isValid(),
"BraceStmt ends before pattern binding entry?");
range.End = *endLoc;
}
return range;
}

SourceRange PatternEntryInitializerScope::getSourceRangeOfThisASTNode(
Expand Down Expand Up @@ -445,9 +451,14 @@ SourceRange AttachedPropertyWrapperScope::getSourceRangeOfThisASTNode(
return sourceRangeWhenCreated;
}

SourceRange GuardStmtScope::getSourceRangeOfThisASTNode(
const bool omitAssertions) const {
return SourceRange(getStmt()->getStartLoc(), endLoc);
}

SourceRange LookupParentDiversionScope::getSourceRangeOfThisASTNode(
const bool omitAssertions) const {
return SourceRange(startLoc);
return SourceRange(startLoc, endLoc);
}

#pragma mark source range caching
Expand Down
1 change: 1 addition & 0 deletions lib/Driver/ToolChains.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ void ToolChain::addCommonFrontendArgs(const OutputInfo &OI,
inputArgs.AddLastArg(arguments, options::OPT_print_educational_notes);
inputArgs.AddLastArg(arguments, options::OPT_diagnostic_style);
inputArgs.AddLastArg(arguments, options::OPT_disable_parser_lookup);
inputArgs.AddLastArg(arguments, options::OPT_enable_parser_lookup);
inputArgs.AddLastArg(arguments,
options::OPT_enable_experimental_concise_pound_file);
inputArgs.AddLastArg(
Expand Down
4 changes: 3 additions & 1 deletion lib/Frontend/CompilerInvocation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,9 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
= A->getOption().matches(OPT_enable_target_os_checking);
}

Opts.DisableParserLookup |= Args.hasArg(OPT_disable_parser_lookup);
Opts.DisableParserLookup |= Args.hasFlag(OPT_disable_parser_lookup,
OPT_enable_parser_lookup,
/*default*/ false);
Opts.EnableNewOperatorLookup = Args.hasFlag(OPT_enable_new_operator_lookup,
OPT_disable_new_operator_lookup,
/*default*/ false);
Expand Down
Loading