Skip to content

Commit 93c37b4

Browse files
hamishknighthamishknight
authored
Merge pull request #81028 from hamishknight/fix-completion-sourceranges
[IDE] Avoid uses of `isBeforeInBuffer` in `TypeCheckASTNodeAtLocRequest`
2 parents 40b5709 + d00f45a commit 93c37b4

File tree

13 files changed

+130
-91
lines changed

13 files changed

+130
-91
lines changed

‎include/swift/AST/Decl.h

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2523,9 +2523,7 @@ class PatternBindingDecl final : public Decl,
25232523
Pattern *Pat, Expr *E,
25242524
DeclContext *Parent);
25252525

2526-
SourceLoc getStartLoc() const {
2527-
return StaticLoc.isValid() ? StaticLoc : VarLoc;
2528-
}
2526+
SourceLoc getStartLoc() const;
25292527
SourceRange getSourceRange() const;
25302528

25312529
unsigned getNumPatternEntries() const {
@@ -8417,9 +8415,8 @@ class FuncDecl : public AbstractFunctionDecl {
84178415
SourceLoc getStaticLoc() const { return StaticLoc; }
84188416
SourceLoc getFuncLoc() const { return FuncLoc; }
84198417

8420-
SourceLoc getStartLoc() const {
8421-
return StaticLoc.isValid() ? StaticLoc : FuncLoc;
8422-
}
8418+
SourceLoc getStartLoc() const;
8419+
SourceLoc getEndLoc() const;
84238420
SourceRange getSourceRange() const;
84248421

84258422
TypeRepr *getResultTypeRepr() const { return FnRetType.getTypeRepr(); }

‎lib/AST/Decl.cpp

Lines changed: 69 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -2528,6 +2528,19 @@ StringRef PatternBindingEntry::getInitStringRepresentation(
25282528
return extractInlinableText(ctx, init, scratch);
25292529
}
25302530

2531+
SourceLoc PatternBindingDecl::getStartLoc() const {
2532+
if (StaticLoc.isValid())
2533+
return StaticLoc;
2534+
2535+
if (VarLoc.isValid())
2536+
return VarLoc;
2537+
2538+
if (getPatternList().empty())
2539+
return SourceLoc();
2540+
2541+
return getPatternList().front().getStartLoc();
2542+
}
2543+
25312544
SourceRange PatternBindingDecl::getSourceRange() const {
25322545
SourceLoc startLoc = getStartLoc();
25332546
SourceLoc endLoc = getPatternList().empty()
@@ -10323,30 +10336,20 @@ SourceRange AbstractFunctionDecl::getSignatureSourceRange() const {
1032310336
if (isImplicit())
1032410337
return SourceRange();
1032510338

10326-
SourceLoc endLoc;
10327-
10328-
// name(parameter list...) async throws(E)
10339+
SourceRange thrownTypeRange;
1032910340
if (auto *typeRepr = getThrownTypeRepr())
10330-
endLoc = typeRepr->getSourceRange().End;
10331-
if (endLoc.isInvalid())
10332-
endLoc = getThrowsLoc();
10333-
if (endLoc.isInvalid())
10334-
endLoc = getAsyncLoc();
10341+
thrownTypeRange = typeRepr->getSourceRange();
1033510342

10336-
if (endLoc.isInvalid())
10337-
return getParameterListSourceRange();
10338-
return SourceRange(getNameLoc(), endLoc);
10343+
// name(parameter list...) async throws(E)
10344+
return SourceRange::combine(getParameterListSourceRange(), getAsyncLoc(),
10345+
getThrowsLoc(), thrownTypeRange);
1033910346
}
1034010347

1034110348
SourceRange AbstractFunctionDecl::getParameterListSourceRange() const {
1034210349
if (isImplicit())
1034310350
return SourceRange();
1034410351

10345-
auto endLoc = getParameters()->getSourceRange().End;
10346-
if (endLoc.isInvalid())
10347-
return getNameLoc();
10348-
10349-
return SourceRange(getNameLoc(), endLoc);
10352+
return SourceRange::combine(getNameLoc(), getParameters()->getSourceRange());
1035010353
}
1035110354

1035210355
std::optional<Fingerprint> AbstractFunctionDecl::getBodyFingerprint() const {
@@ -11453,33 +11456,62 @@ DestructorDecl *DestructorDecl::getSuperDeinit() const {
1145311456
return nullptr;
1145411457
}
1145511458

11456-
SourceRange FuncDecl::getSourceRange() const {
11457-
SourceLoc startLoc = getStartLoc();
11459+
SourceLoc FuncDecl::getStartLoc() const {
11460+
if (StaticLoc)
11461+
return StaticLoc;
1145811462

11459-
if (startLoc.isInvalid())
11460-
return SourceRange();
11463+
if (FuncLoc)
11464+
return FuncLoc;
1146111465

11462-
if (getBodyKind() == BodyKind::Unparsed)
11463-
return { startLoc, BodyRange.End };
11466+
auto nameLoc = getNameLoc();
11467+
if (nameLoc)
11468+
return nameLoc;
1146411469

11465-
SourceLoc endLoc = getOriginalBodySourceRange().End;
11466-
if (endLoc.isInvalid()) {
11467-
if (isa<AccessorDecl>(this))
11468-
return startLoc;
11470+
auto sigStart = getSignatureSourceRange().Start;
11471+
if (sigStart)
11472+
return sigStart;
1146911473

11470-
if (getBodyKind() == BodyKind::Synthesize)
11471-
return SourceRange();
11474+
auto resultTyStart = getResultTypeSourceRange().Start;
11475+
if (resultTyStart)
11476+
return resultTyStart;
1147211477

11473-
endLoc = getGenericTrailingWhereClauseSourceRange().End;
11474-
}
11475-
if (endLoc.isInvalid())
11476-
endLoc = getResultTypeSourceRange().End;
11477-
if (endLoc.isInvalid())
11478-
endLoc = getSignatureSourceRange().End;
11479-
if (endLoc.isInvalid())
11480-
endLoc = startLoc;
11478+
auto genericWhereStart = getGenericTrailingWhereClauseSourceRange().Start;
11479+
if (genericWhereStart)
11480+
return genericWhereStart;
1148111481

11482-
return { startLoc, endLoc };
11482+
auto bodyStart = getOriginalBodySourceRange().Start;
11483+
if (bodyStart)
11484+
return bodyStart;
11485+
11486+
return SourceLoc();
11487+
}
11488+
11489+
SourceLoc FuncDecl::getEndLoc() const {
11490+
auto bodyEnd = getOriginalBodySourceRange().End;
11491+
if (bodyEnd)
11492+
return bodyEnd;
11493+
11494+
auto genericWhereEnd = getGenericTrailingWhereClauseSourceRange().End;
11495+
if (genericWhereEnd)
11496+
return genericWhereEnd;
11497+
11498+
auto resultTyEnd = getResultTypeSourceRange().End;
11499+
if (resultTyEnd)
11500+
return resultTyEnd;
11501+
11502+
auto sigEnd = getSignatureSourceRange().End;
11503+
if (sigEnd)
11504+
return sigEnd;
11505+
11506+
return getStartLoc();
11507+
}
11508+
11509+
SourceRange FuncDecl::getSourceRange() const {
11510+
SourceLoc startLoc = getStartLoc();
11511+
if (startLoc.isInvalid())
11512+
return SourceRange();
11513+
11514+
return { startLoc, getEndLoc() };
1148311515
}
1148411516

1148511517
EnumElementDecl::EnumElementDecl(SourceLoc IdentifierLoc, DeclName Name,

‎lib/Basic/SourceLoc.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ namespace {
482482

483483
std::optional<unsigned>
484484
SourceManager::findBufferContainingLocInternal(SourceLoc Loc) const {
485-
assert(Loc.isValid());
485+
ASSERT(Loc.isValid());
486486

487487
// If the cache is out-of-date, update it now.
488488
unsigned numBuffers = LLVMSourceMgr.getNumBuffers();

‎lib/IDE/PostfixCompletion.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,9 +110,6 @@ getClosureActorIsolation(const Solution &S, AbstractClosureExpr *ACE) {
110110
if (auto Ty = target->getClosureContextualType())
111111
return Ty;
112112
}
113-
if (!S.hasType(E)) {
114-
return Type();
115-
}
116113
return getTypeForCompletion(S, E);
117114
};
118115
auto getClosureActorIsolationThunk = [&S](AbstractClosureExpr *ACE) {

‎lib/IDE/TypeCheckCompletionCallback.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ Type swift::ide::getTypeForCompletion(const constraints::Solution &S,
5858
}
5959

6060
if (!S.hasType(Node)) {
61-
assert(false && "Expression wasn't type checked?");
61+
CONDITIONAL_ASSERT(false && "Expression wasn't type checked?");
6262
return nullptr;
6363
}
6464

‎lib/Parse/ParseDecl.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9009,7 +9009,7 @@ Parser::parseAbstractFunctionBodyDelayed(AbstractFunctionDecl *AFD) {
90099009
auto bodyRange = AFD->getBodySourceRange();
90109010
auto BeginParserPosition = getParserPosition(bodyRange.Start,
90119011
/*previousLoc*/ SourceLoc());
9012-
auto EndLexerState = L->getStateForEndOfTokenLoc(AFD->getEndLoc());
9012+
auto EndLexerState = L->getStateForEndOfTokenLoc(bodyRange.End);
90139013

90149014
// ParserPositionRAII needs a primed parser to restore to.
90159015
if (Tok.is(tok::NUM_TOKENS))

‎lib/Parse/ParseExpr.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3639,7 +3639,7 @@ Parser::parseExprCollectionElement(std::optional<bool> &isDictionary) {
36393639
} else {
36403640
diagnose(Tok, diag::expected_colon_in_dictionary_literal);
36413641
Value = makeParserResult(makeParserError(),
3642-
new (Context) ErrorExpr(SourceRange()));
3642+
new (Context) ErrorExpr(PreviousLoc));
36433643
}
36443644

36453645
// Make a tuple of Key Value pair.

‎lib/Sema/TypeCheckStmt.cpp

Lines changed: 38 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -2462,12 +2462,29 @@ bool TypeCheckASTNodeAtLocRequest::evaluate(
24622462
return MacroWalking::ArgumentsAndExpansion;
24632463
}
24642464

2465+
/// Checks whether the given range, when treated as a character range,
2466+
/// contains the searched location.
2467+
bool charRangeContainsLoc(SourceRange range) {
2468+
if (!range)
2469+
return false;
2470+
2471+
if (SM.isBefore(Loc, range.Start))
2472+
return false;
2473+
2474+
// NOTE: We need to check the character loc here because the target
2475+
// loc can be inside the last token of the node. i.e. interpolated
2476+
// string.
2477+
return SM.isBefore(Loc, Lexer::getLocForEndOfToken(SM, range.End));
2478+
}
2479+
24652480
PreWalkResult<Stmt *> walkToStmtPre(Stmt *S) override {
24662481
if (auto *brace = dyn_cast<BraceStmt>(S)) {
2467-
auto braceCharRange = Lexer::getCharSourceRangeFromSourceRange(
2468-
SM, brace->getSourceRange());
2482+
auto braceRange = brace->getSourceRange();
2483+
auto braceCharRange = SourceRange(
2484+
braceRange.Start, Lexer::getLocForEndOfToken(SM, braceRange.End));
2485+
24692486
// Unless this brace contains the loc, there's nothing to do.
2470-
if (!braceCharRange.contains(Loc))
2487+
if (!SM.containsLoc(braceCharRange, Loc))
24712488
return Action::SkipNode(S);
24722489

24732490
// Reset the node found in a parent context if it's not part of this
@@ -2477,22 +2494,22 @@ bool TypeCheckASTNodeAtLocRequest::evaluate(
24772494
// syntactically part of the brace stmt's range but won't be walked as
24782495
// a child of the brace stmt.
24792496
if (!brace->isImplicit() && FoundNode) {
2480-
auto foundNodeCharRange = Lexer::getCharSourceRangeFromSourceRange(
2481-
SM, FoundNode->getSourceRange());
2482-
if (!braceCharRange.contains(foundNodeCharRange)) {
2497+
auto foundRange = FoundNode->getSourceRange();
2498+
auto foundCharRange = SourceRange(
2499+
foundRange.Start, Lexer::getLocForEndOfToken(SM, foundRange.End));
2500+
if (!SM.encloses(braceCharRange, foundCharRange))
24832501
FoundNode = nullptr;
2484-
}
24852502
}
24862503

24872504
for (ASTNode &node : brace->getElements()) {
2488-
if (SM.isBeforeInBuffer(Loc, node.getStartLoc()))
2505+
auto range = node.getSourceRange();
2506+
if (SM.isBefore(Loc, range.Start))
24892507
break;
24902508

24912509
// NOTE: We need to check the character loc here because the target
24922510
// loc can be inside the last token of the node. i.e. interpolated
24932511
// string.
2494-
SourceLoc endLoc = Lexer::getLocForEndOfToken(SM, node.getEndLoc());
2495-
if (SM.isBeforeInBuffer(endLoc, Loc) || endLoc == Loc)
2512+
if (!SM.isBefore(Loc, Lexer::getLocForEndOfToken(SM, range.End)))
24962513
continue;
24972514

24982515
// 'node' may be the target node, except 'CaseStmt' which cannot be
@@ -2509,13 +2526,11 @@ bool TypeCheckASTNodeAtLocRequest::evaluate(
25092526
return Action::Stop();
25102527
} else if (auto Conditional = dyn_cast<LabeledConditionalStmt>(S)) {
25112528
for (StmtConditionElement &Cond : Conditional->getCond()) {
2512-
if (SM.isBeforeInBuffer(Loc, Cond.getStartLoc())) {
2529+
auto range = Cond.getSourceRange();
2530+
if (SM.isBefore(Loc, range.Start))
25132531
break;
2514-
}
2515-
SourceLoc endLoc = Lexer::getLocForEndOfToken(SM, Cond.getEndLoc());
2516-
if (SM.isBeforeInBuffer(endLoc, Loc) || endLoc == Loc) {
2532+
if (!SM.isBefore(Loc, Lexer::getLocForEndOfToken(SM, range.End)))
25172533
continue;
2518-
}
25192534

25202535
FoundNodeStorage = ASTNode(&Cond);
25212536
FoundNode = &FoundNodeStorage;
@@ -2527,11 +2542,7 @@ bool TypeCheckASTNodeAtLocRequest::evaluate(
25272542
}
25282543

25292544
PreWalkResult<Expr *> walkToExprPre(Expr *E) override {
2530-
if (SM.isBeforeInBuffer(Loc, E->getStartLoc()))
2531-
return Action::SkipNode(E);
2532-
2533-
SourceLoc endLoc = Lexer::getLocForEndOfToken(SM, E->getEndLoc());
2534-
if (SM.isBeforeInBuffer(endLoc, Loc))
2545+
if (!charRangeContainsLoc(E->getSourceRange()))
25352546
return Action::SkipNode(E);
25362547

25372548
// Don't walk into 'TapExpr'. They should be type checked with parent
@@ -2546,9 +2557,7 @@ bool TypeCheckASTNodeAtLocRequest::evaluate(
25462557
if (auto *SVE = dyn_cast<SingleValueStmtExpr>(E)) {
25472558
SmallVector<Expr *> scratch;
25482559
for (auto *result : SVE->getResultExprs(scratch)) {
2549-
auto resultCharRange = Lexer::getCharSourceRangeFromSourceRange(
2550-
SM, result->getSourceRange());
2551-
if (resultCharRange.contains(Loc)) {
2560+
if (charRangeContainsLoc(result->getSourceRange())) {
25522561
if (!result->walk(*this))
25532562
return Action::Stop();
25542563

@@ -2570,20 +2579,15 @@ bool TypeCheckASTNodeAtLocRequest::evaluate(
25702579
}
25712580

25722581
PreWalkAction walkToDeclPre(Decl *D) override {
2582+
if (!charRangeContainsLoc(D->getSourceRange()))
2583+
return Action::SkipNode();
2584+
25732585
if (auto *newDC = dyn_cast<DeclContext>(D))
25742586
DC = newDC;
25752587

2576-
if (!SM.isBeforeInBuffer(Loc, D->getStartLoc())) {
2577-
// NOTE: We need to check the character loc here because the target
2578-
// loc can be inside the last token of the node. i.e. interpolated
2579-
// string.
2580-
SourceLoc endLoc = Lexer::getLocForEndOfToken(SM, D->getEndLoc());
2581-
if (!(SM.isBeforeInBuffer(endLoc, Loc) || endLoc == Loc)) {
2582-
if (!isa<TopLevelCodeDecl>(D)) {
2583-
FoundNodeStorage = ASTNode(D);
2584-
FoundNode = &FoundNodeStorage;
2585-
}
2586-
}
2588+
if (!isa<TopLevelCodeDecl>(D)) {
2589+
FoundNodeStorage = ASTNode(D);
2590+
FoundNode = &FoundNodeStorage;
25872591
}
25882592
return Action::Continue();
25892593
}

‎test/Concurrency/async_main_resolution.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,11 @@ extension MainProtocol {
6363
#endif
6464

6565
// CHECK-IS-SYNC-LABEL: "MyMain" interface_type="MyMain.Type"
66-
// CHECK-IS-SYNC: (func_decl {{.*}}implicit "$main()" interface_type="(MyMain.Type) -> () -> ()"
66+
// CHECK-IS-SYNC: (func_decl {{.*}}implicit range={{.*}} "$main()" interface_type="(MyMain.Type) -> () -> ()"
6767
// CHECK-IS-SYNC: (declref_expr implicit type="(MyMain.Type) -> () -> ()"
6868

6969
// CHECK-IS-ASYNC-LABEL: "MyMain" interface_type="MyMain.Type"
70-
// CHECK-IS-ASYNC: (func_decl {{.*}}implicit "$main()" interface_type="(MyMain.Type) -> () async -> ()"
70+
// CHECK-IS-ASYNC: (func_decl {{.*}}implicit range={{.*}} "$main()" interface_type="(MyMain.Type) -> () async -> ()"
7171
// CHECK-IS-ASYNC: (declref_expr implicit type="(MyMain.Type) -> () async -> ()"
7272

7373
// CHECK-IS-ERROR1: error: 'MyMain' is annotated with '@main' and must provide a main static function of type {{\(\) -> Void or \(\) throws -> Void|\(\) -> Void, \(\) throws -> Void, \(\) async -> Void, or \(\) async throws -> Void}}

‎test/Concurrency/where_clause_main_resolution.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,19 @@ protocol App {
2525
// CHECK-SAME: interface_type="<Self where Self : App> (Self.Type) -> () async -> ()"
2626

2727
extension App where Configuration == Config1 {
28-
// CHECK-CONFIG1: (func_decl {{.*}}implicit "$main()" interface_type="(MainType.Type) -> () -> ()"
28+
// CHECK-CONFIG1: (func_decl {{.*}}implicit range=[{{.*}}:[[@LINE+20]]:1 - line:[[@LINE+20]]:1] "$main()" interface_type="(MainType.Type) -> () -> ()"
2929
// CHECK-CONFIG1: [[SOURCE_FILE]]:[[# @LINE+1 ]]
3030
static func main() { }
3131
}
3232

3333
extension App where Configuration == Config2 {
34-
// CHECK-CONFIG2: (func_decl {{.*}}implicit "$main()" interface_type="(MainType.Type) -> () async -> ()"
34+
// CHECK-CONFIG2: (func_decl {{.*}}implicit range=[{{.*}}:[[@LINE+14]]:1 - line:[[@LINE+14]]:1] "$main()" interface_type="(MainType.Type) -> () async -> ()"
3535
// CHECK-CONFIG2: [[SOURCE_FILE]]:[[# @LINE+1 ]]
3636
static func main() async { }
3737
}
3838

3939
extension App where Configuration == Config3 {
40-
// CHECK-CONFIG3-ASYNC: (func_decl {{.*}}implicit "$main()" interface_type="(MainType.Type) -> () async -> ()"
40+
// CHECK-CONFIG3-ASYNC: (func_decl {{.*}}implicit range=[{{.*}}:[[@LINE+8]]:1 - line:[[@LINE+8]]:1] "$main()" interface_type="(MainType.Type) -> () async -> ()"
4141
// CHECK-CONFIG3-ASYNC: [[SOURCE_FILE]]:[[DEFAULT_ASYNCHRONOUS_MAIN_LINE]]
4242
}
4343

‎test/attr/ApplicationMain/attr_main_throws.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ struct MyBase {
77
}
88
}
99

10-
// CHECK-AST: (func_decl {{.*}} implicit "$main()" interface_type="(MyBase.Type) -> () throws -> ()" access=internal static
10+
// CHECK-AST: (func_decl {{.*}} implicit range=[{{.*}}:[[@LINE-6]]:1 - line:[[@LINE-6]]:1] "$main()" interface_type="(MyBase.Type) -> () throws -> ()" access=internal static
1111
// CHECK-AST-NEXT: (parameter "self" {{.*}})
1212
// CHECK-AST-NEXT: (parameter_list)
1313
// CHECK-AST-NEXT: (brace_stmt implicit

‎test/expr/capture/top-level-guard.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ let closureCapture: () -> Void = { [x] in
3939
}
4040

4141
// CHECK-LABEL: (defer_stmt
42-
// CHECK-NEXT: (func_decl{{.*}}implicit "$defer()" interface_type="() -> ()" access=fileprivate captures=(x<direct><noescape>)
42+
// CHECK-NEXT: (func_decl{{.*}}implicit range={{.*}} "$defer()" interface_type="() -> ()" access=fileprivate captures=(x<direct><noescape>)
4343
defer {
4444
_ = x
4545
}

0 commit comments

Comments
 (0)