Skip to content

Commit dd7ae44

Browse files
DougGregorDougGregor
authored
Merge pull request #4770 from DougGregor/scope-map-fixes
[Scope map] Miscellaneous improvements (and a hack) for unqualified name lookup
2 parents 76c4550 + 4c45885 commit dd7ae44

File tree

10 files changed

+101
-80
lines changed

10 files changed

+101
-80
lines changed

‎include/swift/AST/Decl.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1709,7 +1709,10 @@ class PatternBindingEntry {
17091709
void setInitContext(DeclContext *dc) { InitContext = dc; }
17101710

17111711
/// Retrieve the source range covered by this pattern binding.
1712-
SourceRange getSourceRange() const;
1712+
///
1713+
/// \param omitAccessors Whether the computation should omit the accessors
1714+
/// from the source range.
1715+
SourceRange getSourceRange(bool omitAccessors = false) const;
17131716
};
17141717

17151718
/// \brief This decl contains a pattern and optional initializer for a set

‎lib/AST/ASTScope.cpp

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,18 @@ void ASTScope::expand() const {
277277
return stoleContinuation;
278278
};
279279

280+
// Local function to add the accessors of the variables in the given pattern
281+
// as children.
282+
auto addAccessors = [&](Pattern *pattern) {
283+
// Create children for the accessors of any variables in the pattern that
284+
// have them.
285+
pattern->forEachVariable([&](VarDecl *var) {
286+
if (hasAccessors(var)) {
287+
addChild(new (ctx) ASTScope(this, var));
288+
}
289+
});
290+
};
291+
280292
if (!parentAndExpanded.getInt()) {
281293
// Expand the children in the current scope.
282294
switch (getKind()) {
@@ -365,39 +377,20 @@ void ASTScope::expand() const {
365377
patternBinding.decl->getPatternList()[patternBinding.entry];
366378

367379
// Create a child for the initializer, if present.
368-
ASTScope *initChild = nullptr;
369380
if (patternEntry.getInitAsWritten() &&
370381
patternEntry.getInitAsWritten()->getSourceRange().isValid()) {
371-
initChild = new (ctx) ASTScope(ASTScopeKind::PatternInitializer, this,
372-
patternBinding.decl, patternBinding.entry);
382+
addChild(new (ctx) ASTScope(ASTScopeKind::PatternInitializer, this,
383+
patternBinding.decl, patternBinding.entry));
373384
}
374385

375-
// Create children for the accessors of any variables in the pattern that
376-
// have them.
377-
patternEntry.getPattern()->forEachVariable([&](VarDecl *var) {
378-
if (hasAccessors(var)) {
379-
// If there is an initializer child that precedes this node (the
380-
// normal case), add teh initializer child first.
381-
if (initChild &&
382-
ctx.SourceMgr.isBeforeInBuffer(
383-
patternEntry.getInitAsWritten()->getEndLoc(),
384-
var->getBracesRange().Start)) {
385-
addChild(initChild);
386-
initChild = nullptr;
387-
}
388-
389-
addChild(new (ctx) ASTScope(this, var));
390-
}
391-
});
392-
393-
// If an initializer child remains, add it now.
394-
if (initChild)
395-
addChild(initChild);
396-
397386
// If there is an active continuation, nest the remaining pattern bindings.
398387
if (getActiveContinuation()) {
388+
// Note: the accessors will follow the pattern binding.
399389
addChild(new (ctx) ASTScope(ASTScopeKind::AfterPatternBinding, this,
400390
patternBinding.decl, patternBinding.entry));
391+
} else {
392+
// Otherwise, add the accessors immediately.
393+
addAccessors(patternEntry.getPattern());
401394
}
402395
break;
403396
}
@@ -413,6 +406,12 @@ void ASTScope::expand() const {
413406
}
414407

415408
case ASTScopeKind::AfterPatternBinding: {
409+
const auto &patternEntry =
410+
patternBinding.decl->getPatternList()[patternBinding.entry];
411+
412+
// Add accessors for the variables in this pattern.
413+
addAccessors(patternEntry.getPattern());
414+
416415
// Create a child for the next pattern binding.
417416
if (auto child = createIfNeeded(this, patternBinding.decl))
418417
addChild(child);
@@ -1504,10 +1503,6 @@ SourceRange ASTScope::getSourceRangeImpl() const {
15041503
const auto &patternEntry =
15051504
patternBinding.decl->getPatternList()[patternBinding.entry];
15061505

1507-
// We expect a continuation here.
1508-
assert(!patternBinding.decl->getDeclContext()->isLocalContext() ||
1509-
getHistoricalContinuation());
1510-
15111506
return patternEntry.getSourceRange();
15121507
}
15131508

@@ -1522,11 +1517,8 @@ SourceRange ASTScope::getSourceRangeImpl() const {
15221517
const auto &patternEntry =
15231518
patternBinding.decl->getPatternList()[patternBinding.entry];
15241519

1525-
// We expect a continuation here.
1526-
assert(getHistoricalContinuation());
1527-
1528-
// The scope of the binding begins at the end of the binding.
1529-
return SourceRange(patternEntry.getSourceRange().End);
1520+
return SourceRange(patternEntry.getSourceRange(/*omitAccessors*/true).End,
1521+
patternEntry.getSourceRange().End);
15301522
}
15311523

15321524
case ASTScopeKind::BraceStmt:
@@ -1546,8 +1538,6 @@ SourceRange ASTScope::getSourceRangeImpl() const {
15461538
// For a guard continuation, the scope extends from the end of the 'else'
15471539
// to the end of the continuation.
15481540
if (conditionalClause.isGuardContinuation) {
1549-
// We expect a continuation here.
1550-
assert(getHistoricalContinuation());
15511541
const ASTScope *guard = this;
15521542
do {
15531543
guard = guard->getParent();

‎lib/AST/Decl.cpp

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -918,26 +918,20 @@ VarDecl *PatternBindingEntry::getAnchoringVarDecl() const {
918918
return variables[0];
919919
}
920920

921-
SourceRange PatternBindingEntry::getSourceRange() const {
922-
ASTContext *ctx = nullptr;
923-
924-
SourceLoc endLoc;
925-
getPattern()->forEachVariable([&](VarDecl *var) {
926-
auto accessorsEndLoc = var->getBracesRange().End;
927-
if (accessorsEndLoc.isValid()) {
928-
endLoc = accessorsEndLoc;
929-
if (!ctx) ctx = &var->getASTContext();
930-
}
931-
});
932-
933-
// Check the initializer.
934-
SourceLoc initEndLoc = getOrigInitRange().End;
935-
if (initEndLoc.isValid() &&
936-
(endLoc.isInvalid() ||
937-
(ctx && ctx->SourceMgr.isBeforeInBuffer(endLoc, initEndLoc))))
938-
endLoc = initEndLoc;
921+
SourceRange PatternBindingEntry::getSourceRange(bool omitAccessors) const {
922+
// Patterns end at the initializer, if present.
923+
SourceLoc endLoc = getOrigInitRange().End;
924+
925+
// If we're not banned from handling accessors, they follow the initializer.
926+
if (!omitAccessors) {
927+
getPattern()->forEachVariable([&](VarDecl *var) {
928+
auto accessorsEndLoc = var->getBracesRange().End;
929+
if (accessorsEndLoc.isValid())
930+
endLoc = accessorsEndLoc;
931+
});
932+
}
939933

940-
// Check the pattern.
934+
// If we didn't find an end yet, check the pattern.
941935
if (endLoc.isInvalid())
942936
endLoc = getPattern()->getEndLoc();
943937

‎lib/AST/NameLookup.cpp

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,9 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
457457

458458
SmallVector<UnqualifiedLookupResult, 4> UnavailableInnerResults;
459459

460-
if (Loc.isValid() && Ctx.LangOpts.EnableASTScopeLookup) {
460+
if (Loc.isValid() &&
461+
DC->getParentSourceFile()->Kind != SourceFileKind::REPL &&
462+
Ctx.LangOpts.EnableASTScopeLookup) {
461463
// Find the source file in which we are performing the lookup.
462464
SourceFile &sourceFile = *DC->getParentSourceFile();
463465

@@ -599,6 +601,13 @@ UnqualifiedLookup::UnqualifiedLookup(DeclName Name, DeclContext *DC,
599601
// Dig out the type we're looking into.
600602
// FIXME: We shouldn't need to compute a type to perform this lookup.
601603
Type lookupType = dc->getSelfTypeInContext();
604+
605+
// FIXME: Hack to deal with missing 'Self' archetypes.
606+
if (!lookupType) {
607+
if (auto proto = dc->getAsProtocolOrProtocolExtensionContext())
608+
lookupType = proto->getDeclaredType();
609+
}
610+
602611
if (!lookupType || lookupType->is<ErrorType>()) continue;
603612

604613
// If we're performing a static lookup, use the metatype.

‎lib/Parse/ParsePattern.cpp

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -187,8 +187,7 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
187187
if (!hasSpecifier) {
188188
if (Tok.is(tok::kw_let)) {
189189
diagnose(Tok, diag::parameter_let_as_attr)
190-
.fixItRemove(Tok.getLoc());
191-
param.isInvalid = true;
190+
.fixItRemove(Tok.getLoc());
192191
} else {
193192
// We handle the var error in sema for a better fixit and inout is
194193
// handled later in this function for better fixits.
@@ -201,9 +200,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
201200
// Redundant specifiers are fairly common, recognize, reject, and recover
202201
// from this gracefully.
203202
diagnose(Tok, diag::parameter_inout_var_let_repeated)
204-
.fixItRemove(Tok.getLoc());
203+
.fixItRemove(Tok.getLoc());
205204
consumeToken();
206-
param.isInvalid = true;
207205
}
208206
}
209207

@@ -252,9 +250,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
252250
hasValidInOut = true;
253251
if (hasSpecifier) {
254252
diagnose(Tok.getLoc(), diag::parameter_inout_var_let_repeated)
255-
.fixItRemove(param.LetVarInOutLoc);
253+
.fixItRemove(param.LetVarInOutLoc);
256254
consumeToken(tok::kw_inout);
257-
param.isInvalid = true;
258255
} else {
259256
hasSpecifier = true;
260257
param.LetVarInOutLoc = consumeToken(tok::kw_inout);
@@ -263,9 +260,8 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
263260
}
264261
if (!hasValidInOut && hasDeprecatedInOut) {
265262
diagnose(Tok.getLoc(), diag::inout_as_attr_disallowed)
266-
.fixItRemove(param.LetVarInOutLoc)
267-
.fixItInsert(postColonLoc, "inout ");
268-
param.isInvalid = true;
263+
.fixItRemove(param.LetVarInOutLoc)
264+
.fixItInsert(postColonLoc, "inout ");
269265
}
270266

271267
auto type = parseType(diag::expected_parameter_type);
@@ -322,14 +318,13 @@ Parser::parseParameterClause(SourceLoc &leftParenLoc,
322318
if (param.Type) {
323319
diagnose(typeStartLoc, diag::parameter_unnamed)
324320
.fixItInsert(typeStartLoc, "_: ");
325-
} else {
326-
param.isInvalid = true;
327321
}
328322
} else {
329323
// Otherwise, we're not sure what is going on, but this doesn't smell
330324
// like a parameter.
331325
diagnose(Tok, diag::expected_parameter_name);
332326
param.isInvalid = true;
327+
param.FirstNameLoc = Tok.getLoc();
333328
}
334329
}
335330

‎lib/Sema/CodeSynthesis.cpp

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ void swift::convertStoredVarInProtocolToComputed(VarDecl *VD, TypeChecker &TC) {
317317
auto *Get = createGetterPrototype(VD, TC);
318318

319319
// Okay, we have both the getter and setter. Set them in VD.
320-
VD->makeComputed(VD->getLoc(), Get, nullptr, nullptr, VD->getLoc());
320+
VD->makeComputed(SourceLoc(), Get, nullptr, nullptr, SourceLoc());
321321

322322
// We've added some members to our containing class, add them to the members
323323
// list.
@@ -983,7 +983,7 @@ static void convertNSManagedStoredVarToComputed(VarDecl *VD, TypeChecker &TC) {
983983
if (VD->hasAccessorFunctions()) return;
984984

985985
// Okay, we have both the getter and setter. Set them in VD.
986-
VD->makeComputed(VD->getLoc(), Get, Set, nullptr, VD->getLoc());
986+
VD->makeComputed(SourceLoc(), Get, Set, nullptr, SourceLoc());
987987

988988
TC.validateDecl(Get);
989989
TC.validateDecl(Set);
@@ -1736,7 +1736,7 @@ void swift::maybeAddAccessorsToVariable(VarDecl *var, TypeChecker &TC) {
17361736
setter->setAccessibility(var->getFormalAccess());
17371737
}
17381738

1739-
var->makeComputed(var->getLoc(), getter, setter, nullptr, var->getLoc());
1739+
var->makeComputed(SourceLoc(), getter, setter, nullptr, SourceLoc());
17401740

17411741
// Save the conformance and 'value' decl for later type checking.
17421742
behavior->Conformance = conformance;
@@ -1841,8 +1841,7 @@ void swift::maybeAddAccessorsToVariable(VarDecl *var, TypeChecker &TC) {
18411841

18421842
ParamDecl *newValueParam = nullptr;
18431843
auto *setter = createSetterPrototype(var, newValueParam, TC);
1844-
var->makeComputed(var->getLoc(), getter, setter, nullptr,
1845-
var->getLoc());
1844+
var->makeComputed(SourceLoc(), getter, setter, nullptr, SourceLoc());
18461845
var->setIsBeingTypeChecked(false);
18471846

18481847
TC.validateDecl(getter);

‎lib/Sema/TypeCheckConstraints.cpp

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,8 +472,14 @@ resolveDeclRefExpr(UnresolvedDeclRefExpr *UDRE, DeclContext *DC) {
472472
}
473473

474474
ValueDecl *D = Result.Decl;
475-
if (!D->hasType()) {
476-
assert(D->getDeclContext()->isLocalContext());
475+
if (!D->hasType()) validateDecl(D);
476+
477+
// FIXME: The source-location checks won't make sense once
478+
// EnableASTScopeLookup is the default.
479+
if (Loc.isValid() && D->getLoc().isValid() &&
480+
D->getDeclContext()->isLocalContext() &&
481+
D->getDeclContext() == DC &&
482+
Context.SourceMgr.isBeforeInBuffer(Loc, D->getLoc())) {
477483
if (!D->isInvalid()) {
478484
diagnose(Loc, diag::use_local_before_declaration, Name);
479485
diagnose(D, diag::decl_declared_here, Name);

‎test/NameBinding/scope_map.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -366,13 +366,14 @@ class LazyProperties {
366366

367367
// CHECK-EXPANDED: {{^}} `-AbstractFunctionParams {{.*}} funcWithComputedProperties(i:) param 0:0 [142:36 - 155:1] expanded
368368
// CHECK-EXPANDED: {{^}} `-BraceStmt {{.*}} [142:41 - 155:1] expanded
369-
// CHECK-EXPANDED: {{^}} |-Accessors {{.*}} scope_map.(file).func decl.computed@{{.*}}scope_map.swift:143:7 [143:21 - 149:3] expanded
369+
// CHECK-EXPANDED-NEXT: {{^}} `-PatternBinding {{.*}} entry 0 [143:7 - 155:1] expanded
370+
// CHECK-EXPANDED-NEXT: {{^}} `-AfterPatternBinding {{.*}} entry 0 [143:17 - 155:1] expanded
371+
// CHECK-EXPANDED-NEXT: {{^}} |-Accessors {{.*}} scope_map.(file).func decl.computed@{{.*}}scope_map.swift:143:7 [143:21 - 149:3] expanded
370372
// CHECK-EXPANDED-NEXT: {{^}} |-AbstractFunctionDecl {{.*}} _ [144:5 - 145:5] expanded
371373
// CHECK-EXPANDED-NEXT: {{^}} `-AbstractFunctionParams {{.*}} _ param 0:0 [144:5 - 145:5] expanded
372374
// CHECK-EXPANDED: {{^}} `-BraceStmt {{.*}} [144:9 - 145:5] expanded
373375
// CHECK-EXPANDED-NEXT: {{^}} `-AbstractFunctionDecl {{.*}} _ [146:5 - 148:5] expanded
374376
// CHECK-EXPANDED: {{^}} `-BraceStmt {{.*}} [146:9 - 148:5] expanded
375-
// CHECK-EXPANDED: {{^}} `-AfterPatternBinding {{.*}} entry 0 [149:3 - 155:1] expanded
376377
// CHECK-EXPANDED: {{^}} `-AfterPatternBinding {{.*}} entry 1 [149:36 - 155:1] expanded
377378
// CHECK-EXPANDED: {{^}} `-AfterPatternBinding {{.*}} entry 2 [150:21 - 155:1] expanded
378379
// CHECK-EXPANDED-NEXT: {{^}} `-AbstractFunctionDecl {{.*}} _ [150:25 - 155:1] expanded
@@ -493,6 +494,5 @@ class LazyProperties {
493494
// CHECK-SEARCHES: -TypeOrExtensionBody {{.*}} 'LazyProperties' [190:22 - 194:1] expanded
494495
// CHECK-SEARCHES-NEXT: |-PatternBinding {{.*}} entry 0 [191:7 - 191:20] unexpanded
495496
// CHECK-SEARCHES-NEXT: `-PatternBinding {{.*}} entry 0 [193:12 - 193:29] expanded
496-
// CHECK-SEARCHES-NEXT: |-Accessors {{.*}} scope_map.(file).LazyProperties.prop@{{.*}}scope_map.swift:193:12 [193:12 - 193:12] unexpanded
497497
// CHECK-SEARCHES-NEXT: `-PatternInitializer {{.*}} entry 0 [193:24 - 193:29] expanded
498498
// CHECK-SEARCHES-NOT: {{ expanded}}

‎test/NameBinding/scope_map_lookup.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@ protocol P1 {
2222
associatedtype A = Self
2323
}
2424

25+
// Protocols involving associated types.
26+
protocol AProtocol {
27+
associatedtype e : e // expected-error {{inheritance from non-protocol, non-class type 'Self.e'}}
28+
}
29+
2530
// Extensions.
2631
protocol P2 {
2732
}
@@ -78,6 +83,19 @@ extension PConstrained4 where Self : Superclass {
7883
}
7984
}
8085

86+
// Local computed properties.
87+
func localComputedProperties() {
88+
var localProperty: Int {
89+
get {
90+
return localProperty // expected-warning{{attempting to access 'localProperty' within its own getter}}
91+
}
92+
set {
93+
print(localProperty)
94+
}
95+
}
96+
{ print(localProperty) }()
97+
}
98+
8199
// Top-level code.
82100
func topLevel() { }
83101

@@ -91,3 +109,7 @@ protocol Fooable {
91109

92110
var foo: Foo { get }
93111
}
112+
113+
let a = b ; let b = a // expected-error{{could not infer type for 'a'}}
114+
// expected-error@-1 {{'a' used within its own type}}
115+
// FIXME: That second error is bogus.

‎test/expr/capture/order.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,11 +78,14 @@ func outOfOrderEnum() {
7878
}
7979

8080
func captureInClosure() {
81-
var x = { (i: Int) in
82-
currentTotal += i // expected-error{{use of local variable 'currentTotal' before its declaration}}
81+
let x = { (i: Int) in
82+
currentTotal += i // expected-error{{cannot capture 'currentTotal' before it is declared}}
8383
}
8484

8585
var currentTotal = 0 // expected-note{{'currentTotal' declared here}}
86+
87+
_ = x
88+
currentTotal += 1
8689
}
8790

8891
class X {

0 commit comments

Comments
 (0)