Skip to content

Commit bb0dafa

Browse files
author
David Ungar
committed
Many fixes for large app, eager primary tree creation, memberCount fix.
Compensate for lazy member parsing Fix memberCount right. add event Better freezing typo Format Debug code stress test add range-matching for debugging Fix eager & rm debugging code undo debugging hack Silence warnings for vars used only in assertions. Don't eagerly build scope tree if disabled. Lazy function bodies Fix no brace bug Fixes for SIL files & try not reevaluating patterns More fixes Fix typo, and try recursion fix Recursion fixes Fix in ASTGen.cpp When no brace and ends with ""
1 parent ec862c5 commit bb0dafa

21 files changed

+210
-41
lines changed

include/swift/AST/ASTScope.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,10 @@ class ASTScopeImpl {
215215

216216
bool checkSourceRangeOfThisASTNode() const;
217217

218+
/// For debugging
219+
bool doesRangeMatch(unsigned start, unsigned end, StringRef file = "",
220+
StringRef className = "");
221+
218222
private:
219223
SourceRange computeSourceRangeOfScope(bool omitAssertions = false) const;
220224
SourceRange
@@ -264,7 +268,7 @@ class ASTScopeImpl {
264268
getSourceRangeOfEnclosedParamsOfASTNode(bool omitAssertions) const;
265269

266270
private:
267-
bool checkSourceRangeAfterExpansion() const;
271+
bool checkSourceRangeAfterExpansion(const ASTContext &) const;
268272

269273
#pragma mark common queries
270274
public:
@@ -318,9 +322,9 @@ class ASTScopeImpl {
318322
private:
319323
/// Compare the pre-expasion range with the post-expansion range and return
320324
/// false if lazyiness couild miss lookups.
321-
bool checkLazySourceRange() const;
325+
bool checkLazySourceRange(const ASTContext &) const;
322326

323-
protected:
327+
public:
324328
/// Some scopes can be expanded lazily.
325329
/// Such scopes must: not change their source ranges after expansion, and
326330
/// their expansion must return an insertion point outside themselves.
@@ -517,6 +521,7 @@ class ASTSourceFileScope final : public ASTScopeImpl {
517521
NullablePtr<DeclContext> getDeclContext() const override;
518522

519523
void addNewDeclsToScopeTree();
524+
void buildScopeTreeEagerly();
520525

521526
const SourceFile *getSourceFile() const override;
522527
NullablePtr<const void> addressForPrinting() const override { return SF; }
@@ -1002,6 +1007,10 @@ class AbstractFunctionBodyScope : public ASTScopeImpl {
10021007
DeclConsumer) const override;
10031008
Optional<bool>
10041009
resolveIsCascadingUseForThisScope(Optional<bool>) const override;
1010+
1011+
public:
1012+
NullablePtr<ASTScopeImpl> insertionPointForDeferredExpansion() override;
1013+
SourceRange sourceRangeForDeferredExpansion() const override;
10051014
};
10061015

10071016
/// Body of methods, functions in types.

include/swift/AST/Decl.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4341,6 +4341,10 @@ class ProtocolDecl final : public NominalTypeDecl {
43414341
/// with the Objective-C runtime.
43424342
StringRef getObjCRuntimeName(llvm::SmallVectorImpl<char> &buffer) const;
43434343

4344+
/// Create the generic parameters of this protocol if they haven't been
4345+
/// created yet.
4346+
void createGenericParamsIfMissing();
4347+
43444348
/// Retrieve the requirements that describe this protocol.
43454349
///
43464350
/// These are the requirements including any inherited protocols

include/swift/AST/DeclContext.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,7 @@ class IterableDeclContext {
760760
void addMember(Decl *member, Decl *hint = nullptr);
761761

762762
/// See \c MemberCount
763-
unsigned getMemberCount() const { return MemberCount; }
763+
unsigned getMemberCount() const;
764764

765765
/// Check whether there are lazily-loaded members.
766766
bool hasLazyMembers() const {

include/swift/AST/ExperimentalDependencies.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,7 @@ class BiIndexedTwoStageMap {
222222
bool insert(const Key1 &k1, const Key2 &k2, Value &v) {
223223
const bool r1 = map1.insert(k1, k2, v);
224224
const bool r2 = map2.insert(k2, k1, v);
225+
(void)r2;
225226
assertConsistent(r1, r2);
226227
return r1;
227228
}

include/swift/AST/Module.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1241,6 +1241,8 @@ class SourceFile final : public FileUnit {
12411241

12421242
bool canBeParsedInFull() const;
12431243

1244+
bool isSuitableForASTScopes() const { return canBeParsedInFull(); }
1245+
12441246
syntax::SourceFileSyntax getSyntaxRoot() const;
12451247
void setSyntaxRoot(syntax::SourceFileSyntax &&Root);
12461248
bool hasSyntaxRoot() const;

include/swift/AST/NameLookup.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -592,6 +592,10 @@ class ASTScope {
592592
public:
593593
ASTScope(SourceFile *);
594594

595+
/// Cannot be lazy during type-checking because it mutates the AST.
596+
/// So build eagerly before type-checking
597+
void buildScopeTreeEagerly();
598+
595599
/// \return the scopes traversed
596600
static llvm::SmallVector<const ast_scope::ASTScopeImpl *, 0>
597601
unqualifiedLookup(SourceFile *, DeclName, SourceLoc,

include/swift/Basic/LangOptions.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,14 +246,17 @@ namespace swift {
246246
/// Default is in \c ParseLangArgs
247247
///
248248
/// This is a staging flag; eventually it will be removed.
249-
bool EnableASTScopeLookup = false;
249+
bool EnableASTScopeLookup = true;
250250

251251
/// Someday, ASTScopeLookup will supplant lookup in the parser
252252
bool DisableParserLookup = false;
253253

254254
/// Should we compare to ASTScope-based resolution for debugging?
255255
bool CompareToASTScopeLookup = false;
256256

257+
/// Should we stress ASTScope-based resolution for debugging?
258+
bool StressASTScopeLookup = false;
259+
257260
/// Since some tests fail if the warning is output, use a flag to decide
258261
/// whether it is. The warning is useful for testing.
259262
bool WarnIfASTScopeLookup = false;

include/swift/Driver/ExperimentalDependencyDriverGraph.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,7 @@ class ModuleDepGraph {
223223
ModuleDepGraphNode *nodeActuallyErased = nodeMap.findAndErase(
224224
nodeToErase->getSwiftDeps().getValueOr(std::string()),
225225
nodeToErase->getKey());
226+
(void)nodeActuallyErased;
226227
assert(
227228
nodeToErase == nodeActuallyErased ||
228229
mapCorruption("Node found from key must be same as node holding key."));

include/swift/Option/FrontendOptions.td

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,9 @@ def disable_target_os_checking :
124124

125125
def compare_to_astscope_lookup : Flag<["-"], "compare-to-astscope-lookup">,
126126
HelpText<"Compare legacy to ASTScope-based unqualified name lookup (for debugging)">;
127+
128+
def stress_astscope_lookup : Flag<["-"], "stress-astscope-lookup">,
129+
HelpText<"Stress ASTScope-based unqualified name lookup (for testing)">;
127130

128131
def warn_if_astscope_lookup : Flag<["-"], "warn-if-astscope-lookup">,
129132
HelpText<"Print a warning if ASTScope lookup is used">;

lib/AST/ASTScopeCreation.cpp

Lines changed: 96 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,26 @@ static void dumpRangeable(SpecializeAttr *r, llvm::raw_ostream &f) {
9393
llvm::errs() << "SpecializeAttr\n";
9494
}
9595

96+
/// For Debugging
97+
template <typename T>
98+
bool doesRangeableRangeMatch(const T *x, const SourceManager &SM,
99+
unsigned start, unsigned end,
100+
StringRef file = "") {
101+
auto const r = getRangeableSourceRange(x);
102+
if (r.isInvalid())
103+
return false;
104+
if (start && SM.getLineNumber(r.Start) != start)
105+
return false;
106+
if (end && SM.getLineNumber(r.End) != end)
107+
return false;
108+
if (file.empty())
109+
return true;
110+
const auto buf = SM.findBufferContainingLoc(r.Start);
111+
return SM.getIdentifierForBuffer(buf).endswith(file);
112+
}
113+
114+
#pragma mark end of rangeable
115+
96116
static std::vector<ASTNode> asNodeVector(DeclRange dr) {
97117
std::vector<ASTNode> nodes;
98118
llvm::transform(dr, std::back_inserter(nodes),
@@ -165,9 +185,27 @@ class ScopeCreator final {
165185
/// For allocating scopes.
166186
ASTContext &ctx;
167187

188+
public:
189+
/// Because type checking can mutate the AST, eagerly build the tree, then
190+
/// freeze it
191+
enum class Temperature {
192+
Warm, // Can be lazy
193+
Freezing, // Should expand everything eagerly
194+
Frozen // No more changes, except when Decls are added to the source file
195+
};
196+
197+
private:
198+
/// Because type checking can mutate the AST, eagerly build the tree, then
199+
/// freeze it
200+
Temperature temperature = Temperature::Warm;
201+
168202
public:
169203
ASTSourceFileScope *const sourceFileScope;
170204
ASTContext &getASTContext() const { return ctx; }
205+
bool getIsFrozen() const { return temperature == Temperature::Frozen; }
206+
bool getIsFreezing() const { return temperature == Temperature::Freezing; }
207+
void beFreezing() { temperature = Temperature::Freezing; }
208+
void beFrozen() { temperature = Temperature::Frozen; }
171209

172210
/// The AST can have duplicate nodes, and we don't want to create scopes for
173211
/// those.
@@ -366,7 +404,7 @@ class ScopeCreator final {
366404
parent, vd);
367405
});
368406
}
369-
// HERE
407+
370408
public:
371409
/// Create the matryoshka nested generic param scopes (if any)
372410
/// that are subscopes of the receiver. Return
@@ -607,7 +645,9 @@ class ScopeCreator final {
607645
return !n.isDecl(DeclKind::Var);
608646
}
609647

610-
bool shouldBeLazy() const { return ctx.LangOpts.LazyASTScopes; }
648+
bool shouldBeLazy() const {
649+
return !getIsFreezing() && ctx.LangOpts.LazyASTScopes;
650+
}
611651

612652
public:
613653
/// For debugging. Return true if scope tree contains all the decl contexts in
@@ -617,9 +657,11 @@ class ScopeCreator final {
617657
auto allDeclContexts = findLocalizableDeclContextsInAST();
618658
llvm::DenseMap<const DeclContext *, const ASTScopeImpl *> bogusDCs;
619659
bool rebuilt = false;
620-
sourceFileScope->preOrderDo([&](ASTScopeImpl *scope) {
621-
rebuilt |= scope->reexpandIfObsolete(*this);
622-
});
660+
if (!getIsFrozen()) {
661+
sourceFileScope->preOrderDo([&](ASTScopeImpl *scope) {
662+
rebuilt |= scope->reexpandIfObsolete(*this);
663+
});
664+
}
623665
sourceFileScope->postOrderDo([&](ASTScopeImpl *scope) {
624666
if (auto *dc = scope->getDeclContext().getPtrOrNull()) {
625667
auto iter = allDeclContexts.find(dc);
@@ -699,12 +741,24 @@ class ScopeCreator final {
699741

700742
ASTScope::ASTScope(SourceFile *SF) : impl(createScopeTree(SF)) {}
701743

744+
void ASTScope::buildScopeTreeEagerly() {
745+
impl->buildScopeTreeEagerly();
746+
}
747+
702748
ASTSourceFileScope *ASTScope::createScopeTree(SourceFile *SF) {
703749
ScopeCreator *scopeCreator = new (SF->getASTContext()) ScopeCreator(SF);
704750
scopeCreator->sourceFileScope->addNewDeclsToScopeTree();
705751
return scopeCreator->sourceFileScope;
706752
}
707753

754+
void ASTSourceFileScope::buildScopeTreeEagerly() {
755+
scopeCreator->beFreezing();
756+
// Eagerly expand any decls already in the tree.
757+
preOrderDo([&](ASTScopeImpl *s) { s->reexpandIfObsolete(*scopeCreator); });
758+
addNewDeclsToScopeTree();
759+
scopeCreator->beFrozen();
760+
}
761+
708762
void ASTSourceFileScope::addNewDeclsToScopeTree() {
709763
assert(SF && scopeCreator);
710764
ArrayRef<Decl *> decls = SF->Decls;
@@ -1011,6 +1065,9 @@ void ASTScopeImpl::addChild(ASTScopeImpl *child, ASTContext &ctx) {
10111065
assert(!child->getParent() && "child should not already have parent");
10121066
child->parent = this;
10131067
clearCachedSourceRangesOfMeAndAncestors();
1068+
// When adding a ProtocolDecl, createGenericParamsIfMissing() can call back into this tree.
1069+
// So make sure childrenCountWhenLastExpanded is up to date.
1070+
setChildrenCountWhenLastExpanded();
10141071
}
10151072

10161073
void ASTScopeImpl::removeChildren() {
@@ -1036,8 +1093,7 @@ ASTScopeImpl *ASTScopeImpl::expandAndBeCurrent(ScopeCreator &scopeCreator) {
10361093
insertionPointForDeferredExpansion().get() == insertionPoint);
10371094
}
10381095
beCurrent();
1039-
setChildrenCountWhenLastExpanded();
1040-
assert(checkSourceRangeAfterExpansion());
1096+
assert(checkSourceRangeAfterExpansion(scopeCreator.getASTContext()));
10411097
return insertionPoint;
10421098
}
10431099

@@ -1448,25 +1504,30 @@ void AttachedPropertyWrapperScope::
14481504

14491505
ASTScopeImpl *GenericTypeOrExtensionWholePortion::expandScope(
14501506
GenericTypeOrExtensionScope *scope, ScopeCreator &scopeCreator) const {
1507+
// Get now in case recursion emancipates scope
1508+
auto *const ip = scope->getParent().get();
1509+
14511510
// Prevent circular request bugs caused by illegal input and
14521511
// doing lookups that getExtendedNominal in the midst of getExtendedNominal.
14531512
// rdar://53972776
14541513
if (scope->shouldHaveABody() && !scope->doesDeclHaveABody())
1455-
return scope->getParent().get();
1514+
return ip;
14561515

14571516
auto *deepestScope = scopeCreator.addNestedGenericParamScopesToTree(
14581517
scope->getDecl(), scope->getGenericContext()->getGenericParams(), scope);
14591518
if (scope->getGenericContext()->getTrailingWhereClause())
14601519
scope->createTrailingWhereClauseScope(deepestScope, scopeCreator);
14611520
scope->createBodyScope(deepestScope, scopeCreator);
1462-
return scope->getParent().get();
1521+
return ip;
14631522
}
14641523

14651524
ASTScopeImpl *
14661525
IterableTypeBodyPortion::expandScope(GenericTypeOrExtensionScope *scope,
14671526
ScopeCreator &scopeCreator) const {
1527+
// Get it now in case of recursion and this one gets emancipated
1528+
auto *const ip = scope->getParent().get();
14681529
scope->expandBody(scopeCreator);
1469-
return scope->getParent().get();
1530+
return ip;
14701531
}
14711532

14721533
ASTScopeImpl *GenericTypeOrExtensionWherePortion::expandScope(
@@ -1670,7 +1731,9 @@ void IterableTypeScope::expandBody(ScopeCreator &scopeCreator) {
16701731
#pragma mark - reexpandIfObsolete
16711732

16721733
bool ASTScopeImpl::reexpandIfObsolete(ScopeCreator &scopeCreator) {
1673-
if (isCurrent())
1734+
if (scopeCreator.getIsFrozen() ||
1735+
(isCurrent() &&
1736+
!scopeCreator.getASTContext().LangOpts.StressASTScopeLookup))
16741737
return false;
16751738
reexpand(scopeCreator);
16761739
return true;
@@ -1731,6 +1794,11 @@ NullablePtr<ASTScopeImpl> ASTScopeImpl::insertionPointForDeferredExpansion() {
17311794
return nullptr;
17321795
}
17331796

1797+
NullablePtr<ASTScopeImpl>
1798+
AbstractFunctionBodyScope::insertionPointForDeferredExpansion() {
1799+
return getParent().get();
1800+
}
1801+
17341802
NullablePtr<ASTScopeImpl>
17351803
IterableTypeScope::insertionPointForDeferredExpansion() {
17361804
return portion->insertionPointForDeferredExpansion(this);
@@ -1782,20 +1850,29 @@ bool TopLevelCodeScope::isCurrent() const {
17821850
return bodyWhenLastExpanded == decl->getBody();
17831851
}
17841852

1853+
// Try to avoid the work of counting
1854+
static const bool assumeVarsDoNotGetAdded = true;
1855+
1856+
static unsigned countVars(const PatternBindingEntry &entry) {
1857+
unsigned varCount = 0;
1858+
entry.getPattern()->forEachVariable([&](VarDecl *) { ++varCount; });
1859+
return varCount;
1860+
}
1861+
17851862
void PatternEntryDeclScope::beCurrent() {
17861863
initWhenLastExpanded = getPatternEntry().getOriginalInit();
1787-
unsigned varCount = 0;
1788-
getPatternEntry().getPattern()->forEachVariable(
1789-
[&](VarDecl *) { ++varCount; });
1790-
varCountWhenLastExpanded = varCount;
1864+
if (assumeVarsDoNotGetAdded && varCountWhenLastExpanded)
1865+
return;
1866+
varCountWhenLastExpanded = countVars(getPatternEntry());
17911867
}
17921868
bool PatternEntryDeclScope::isCurrent() const {
17931869
if (initWhenLastExpanded != getPatternEntry().getOriginalInit())
17941870
return false;
1795-
unsigned varCount = 0;
1796-
getPatternEntry().getPattern()->forEachVariable(
1797-
[&](VarDecl *) { ++varCount; });
1798-
return varCount == varCountWhenLastExpanded;
1871+
if (assumeVarsDoNotGetAdded && varCountWhenLastExpanded) {
1872+
assert(varCountWhenLastExpanded == countVars(getPatternEntry()));
1873+
return true;
1874+
}
1875+
return countVars(getPatternEntry()) == varCountWhenLastExpanded;
17991876
}
18001877

18011878
void WholeClosureScope::beCurrent() {

0 commit comments

Comments
 (0)