Skip to content

Commit 1ef97c0

Browse files
nkcsgexinkcsgexi
authored
RangeInfo: Rework the criterion for valid selection of multiple statements, i.e. they should have a common brace statement as parent. (#8129)
1 parent 60e54b9 commit 1ef97c0

File tree

4 files changed

+108
-50
lines changed

4 files changed

+108
-50
lines changed

‎include/swift/AST/ASTNode.h

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,10 @@ namespace swift {
2929
class SourceLoc;
3030
class SourceRange;
3131
class ASTWalker;
32-
32+
enum class ExprKind : uint8_t;
33+
enum class DeclKind : uint8_t;
34+
enum class StmtKind;
35+
3336
struct ASTNode : public llvm::PointerUnion3<Expr*, Stmt*, Decl*> {
3437
// Inherit the constructors from PointerUnion.
3538
using PointerUnion3::PointerUnion3;
@@ -51,8 +54,14 @@ namespace swift {
5154
/// \brief get the underlying entity as a decl context if it is one,
5255
/// otherwise, return nullptr;
5356
DeclContext *getAsDeclContext() const;
57+
58+
/// Provides some utilities to decide detailed node kind.
59+
#define FUNC(T) bool is##T(T##Kind Kind) const;
60+
FUNC(Stmt)
61+
FUNC(Expr)
62+
FUNC(Decl)
63+
#undef FUNC
5464
};
55-
5665
} // namespace swift
5766

5867
namespace llvm {

‎lib/AST/ASTNode.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,14 @@ void ASTNode::walk(SourceEntityWalker &Walker) {
7676
else
7777
llvm_unreachable("unsupported AST node");
7878
}
79+
80+
#define FUNC(T) \
81+
bool ASTNode::is##T(T##Kind Kind) const { \
82+
if (!is<T*>()) \
83+
return false; \
84+
return get<T*>()->getKind() == Kind; \
85+
}
86+
FUNC(Stmt)
87+
FUNC(Expr)
88+
FUNC(Decl)
89+
#undef FUNC

‎lib/IDE/SwiftSourceDocInfo.cpp

Lines changed: 44 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,11 @@ static bool hasUnhandledError(ArrayRef<ASTNode> Nodes) {
320320
});
321321
}
322322

323+
static bool
324+
hasNodeThat(ArrayRef<ASTNode> Nodes, llvm::function_ref<bool(ASTNode)> Pred) {
325+
return std::any_of(Nodes.begin(), Nodes.end(), Pred);
326+
}
327+
323328
struct RangeResolver::Implementation {
324329
SourceFile &File;
325330
ASTContext &Ctx;
@@ -342,37 +347,22 @@ struct RangeResolver::Implementation {
342347
std::vector<ASTNode> EndMatches;
343348
ContextInfo(ASTNode Parent, bool ContainedInRange) : Parent(Parent),
344349
ContainedInRange(ContainedInRange) {}
345-
private:
346-
bool hasStmtlikeNode(ArrayRef<ASTNode> Nodes) {
347-
for (auto N : Nodes) {
348-
if (N.is<Stmt*>())
349-
return true;
350-
// Expression with void type is statement-like.
351-
else if (N.is<Expr*>()) {
352-
auto *E = N.get<Expr*>();
353-
if (auto T = E->getType()) {
354-
if (T->isVoid())
355-
return true;
356-
}
357-
} else {
358-
// Decls are statement like.
359-
return true;
360-
}
361-
}
350+
351+
bool isMultiStatment() {
352+
if (StartMatches.empty() || EndMatches.empty())
353+
return false;
354+
355+
// Multi-statement should have a common parent of brace statement, this
356+
// can be implicit brace statement, e.g. in case statement.
357+
if (Parent.isStmt(StmtKind::Brace))
358+
return true;
359+
360+
// Explicitly allow the selection of multiple case statments.
361+
auto IsCase = [](ASTNode N) { return N.isStmt(StmtKind::Case); };
362+
if (hasNodeThat(StartMatches, IsCase) && hasNodeThat(EndMatches, IsCase))
363+
return true;
362364
return false;
363365
}
364-
public:
365-
bool hasStmtMatch(RangeMatchKind Kind) {
366-
switch(Kind) {
367-
case RangeMatchKind::NoneMatch:
368-
case RangeMatchKind::RangeMatch:
369-
llvm_unreachable("cannot answer these.");
370-
case RangeMatchKind::StartMatch:
371-
return hasStmtlikeNode(StartMatches);
372-
case RangeMatchKind::EndMatch:
373-
return hasStmtlikeNode(EndMatches);
374-
}
375-
}
376366
};
377367

378368
std::vector<Token> AllTokens;
@@ -395,17 +385,28 @@ struct RangeResolver::Implementation {
395385
std::vector<ASTNode> ContainedASTNodes;
396386

397387
/// Collect the type that an ASTNode should be evaluated to.
398-
Type resolveNodeType(ASTNode N) {
399-
if (N.is<Stmt*>()) {
400-
if (auto RS = dyn_cast<ReturnStmt>(N.get<Stmt*>())) {
401-
return resolveNodeType(RS->getResult());
388+
Type resolveNodeType(ASTNode N, RangeKind Kind) {
389+
switch(Kind) {
390+
case RangeKind::Invalid:
391+
case RangeKind::SingleDecl:
392+
llvm_unreachable("cannot get type.");
393+
394+
// For a single expression, its type is apparent.
395+
case RangeKind::SingleExpression:
396+
return N.get<Expr*>()->getType();
397+
398+
// For statements, we either resolve to the returning type or Void.
399+
case RangeKind::SingleStatement:
400+
case RangeKind::MultiStatement: {
401+
if (N.is<Stmt*>()) {
402+
if (auto RS = dyn_cast<ReturnStmt>(N.get<Stmt*>())) {
403+
return resolveNodeType(RS->getResult(), RangeKind::SingleExpression);
404+
}
402405
}
403406
// For other statements, the type should be void.
404407
return Ctx.getVoidDecl()->getDeclaredInterfaceType();
405-
} else if (N.is<Expr*>()) {
406-
return N.get<Expr*>()->getType();
407408
}
408-
return Type();
409+
}
409410
}
410411

411412
ResolvedRangeInfo getSingleNodeKind(ASTNode Node) {
@@ -417,14 +418,16 @@ struct RangeResolver::Implementation {
417418
OrphanKind Kind = getOrphanKind(ContainedASTNodes);
418419
if (Node.is<Expr*>())
419420
return ResolvedRangeInfo(RangeKind::SingleExpression,
420-
resolveNodeType(Node), Content,
421+
resolveNodeType(Node, RangeKind::SingleExpression),
422+
Content,
421423
getImmediateContext(), SingleEntry,
422424
UnhandledError, Kind,
423425
llvm::makeArrayRef(ContainedASTNodes),
424426
llvm::makeArrayRef(DeclaredDecls),
425427
llvm::makeArrayRef(ReferencedDecls));
426428
else if (Node.is<Stmt*>())
427-
return ResolvedRangeInfo(RangeKind::SingleStatement, resolveNodeType(Node),
429+
return ResolvedRangeInfo(RangeKind::SingleStatement,
430+
resolveNodeType(Node, RangeKind::SingleStatement),
428431
Content, getImmediateContext(), SingleEntry,
429432
UnhandledError, Kind,
430433
llvm::makeArrayRef(ContainedASTNodes),
@@ -685,15 +688,12 @@ struct RangeResolver::Implementation {
685688
}
686689
}
687690

688-
// Check if the start and end matches have statement-like entities; this
689-
// can avoid picking expressions like "a == b" in a list of selected
690-
// multi-statement at the start (or the end).
691-
if (DCInfo.hasStmtMatch(RangeMatchKind::StartMatch) &&
692-
DCInfo.hasStmtMatch(RangeMatchKind::EndMatch)) {
691+
if (DCInfo.isMultiStatment()) {
693692
postAnalysis(DCInfo.EndMatches.back());
694693
Result = {RangeKind::MultiStatement,
695694
/* Last node has the type */
696-
resolveNodeType(DCInfo.EndMatches.back()), Content,
695+
resolveNodeType(DCInfo.EndMatches.back(),
696+
RangeKind::MultiStatement), Content,
697697
getImmediateContext(), hasSingleEntryPoint(ContainedASTNodes),
698698
hasUnhandledError(ContainedASTNodes),
699699
getOrphanKind(ContainedASTNodes),

‎test/IDE/range_info_basics.swift

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,13 +105,29 @@ func foo8(a : [Int]) {
105105
} while i < 100
106106
}
107107

108-
func foo9(a: Int, b: Int) -> Int {
108+
func foo9(_ a: Int, _ b: Int) -> Int {
109109
if a == b {
110110
return 0
111111
}
112+
switch a {
113+
case 1:
114+
foo9(1, 2)
115+
foo9(1, 2)
116+
return foo9(2, 4)
117+
default:
118+
foo9(1, 2)
119+
foo9(1, 2)
120+
return foo9(2, 4)
121+
}
112122
return 0
113123
}
114124

125+
func testInout(_ a : inout Int) {
126+
var b = a + 1 + 1
127+
b = b + 1
128+
testInout(&b)
129+
}
130+
115131
// RUN: %target-swift-ide-test -range -pos=8:1 -end-pos 8:32 -source-filename %s | %FileCheck %s -check-prefix=CHECK1
116132
// RUN: %target-swift-ide-test -range -pos=9:1 -end-pos 9:26 -source-filename %s | %FileCheck %s -check-prefix=CHECK2
117133
// RUN: %target-swift-ide-test -range -pos=10:1 -end-pos 10:27 -source-filename %s | %FileCheck %s -check-prefix=CHECK3
@@ -139,9 +155,20 @@ func foo9(a: Int, b: Int) -> Int {
139155
// RUN: %target-swift-ide-test -range -pos=87:1 -end-pos=92:6 -source-filename %s | %FileCheck %s -check-prefix=CHECK25
140156
// RUN: %target-swift-ide-test -range -pos=97:1 -end-pos=104:6 -source-filename %s | %FileCheck %s -check-prefix=CHECK26
141157
// RUN: %target-swift-ide-test -range -pos=109:6 -end-pos=111:4 -source-filename %s | %FileCheck %s -check-prefix=CHECK-INVALID
158+
// RUN: %target-swift-ide-test -range -pos=114:1 -end-pos=115:15 -source-filename %s | %FileCheck %s -check-prefix=CHECK27
159+
// RUN: %target-swift-ide-test -range -pos=118:1 -end-pos=119:15 -source-filename %s | %FileCheck %s -check-prefix=CHECK27
160+
// RUN: %target-swift-ide-test -range -pos=126:11 -end-pos=126:12 -source-filename %s | %FileCheck %s -check-prefix=CHECK-INT
161+
// RUN: %target-swift-ide-test -range -pos=126:11 -end-pos=126:20 -source-filename %s | %FileCheck %s -check-prefix=CHECK-INT
162+
// RUN: %target-swift-ide-test -range -pos=127:7 -end-pos=127:8 -source-filename %s | %FileCheck %s -check-prefix=CHECK-INT
163+
// RUN: %target-swift-ide-test -range -pos=127:3 -end-pos=127:4 -source-filename %s | %FileCheck %s -check-prefix=CHECK-INT-LVALUE
164+
// RUN: %target-swift-ide-test -range -pos=128:13 -end-pos=128:15 -source-filename %s | %FileCheck %s -check-prefix=CHECK-INT-INOUT
142165

143166
// CHECK-INVALID: <Kind>Invalid</Kind>
144167

168+
// CHECK-INT: <Type>Int</Type>
169+
// CHECK-INT-LVALUE: <Type>@lvalue Int</Type>
170+
// CHECK-INT-INOUT: <Type>inout Int</Type>
171+
145172
// CHECK1: <Kind>SingleDecl</Kind>
146173
// CHECK1-NEXT: <Content>func foo1() -> Int { return 0 }</Content>
147174
// CHECK1-NEXT: <Context>swift_ide_test.(file)</Context>
@@ -198,7 +225,7 @@ func foo9(a: Int, b: Int) -> Int {
198225
// CHECK7-NEXT: let c = a.byteSwapped
199226
// CHECK7-NEXT: b = b.bigEndian.bigEndian.byteSwapped
200227
// CHECK7-NEXT: print(b + c)</Content>
201-
// CHECK7-NEXT: <Type>()</Type>
228+
// CHECK7-NEXT: <Type>Void</Type>
202229
// CHECK7-NEXT: <Context>swift_ide_test.(file).foo2()</Context>
203230
// CHECK7-NEXT: <Declared>b</Declared><OutscopeReference>false</OutscopeReference>
204231
// CHECK7-NEXT: <Declared>c</Declared><OutscopeReference>false</OutscopeReference>
@@ -238,6 +265,7 @@ func foo9(a: Int, b: Int) -> Int {
238265
// CHECK10: <Kind>MultiStatement</Kind>
239266
// CHECK10-NEXT: <Content>let a = c.c.getC().c.getC().getC().getC()
240267
// CHECK10-NEXT: let b = a.c.c.c.c.getC().getC()</Content>
268+
// CHECK10-NEXT: <Type>Void</Type>
241269
// CHECK10-NEXT: <Context>swift_ide_test.(file).foo5(c:)</Context>
242270
// CHECK10-NEXT: <Declared>a</Declared><OutscopeReference>true</OutscopeReference>
243271
// CHECK10-NEXT: <Declared>b</Declared><OutscopeReference>true</OutscopeReference>
@@ -250,6 +278,7 @@ func foo9(a: Int, b: Int) -> Int {
250278
// CHECK11-NEXT: <Content>let a = c.c.getC().c.getC().getC().getC()
251279
// CHECK11-NEXT: let b = a.c.c.c.c.getC().getC()
252280
// CHECK11-NEXT: let d = a.c.getC().getC().c.c</Content>
281+
// CHECK11-NEXT: <Type>Void</Type>
253282
// CHECK11-NEXT: <Context>swift_ide_test.(file).foo5(c:)</Context>
254283
// CHECK11-NEXT: <Declared>a</Declared><OutscopeReference>true</OutscopeReference>
255284
// CHECK11-NEXT: <Declared>b</Declared><OutscopeReference>true</OutscopeReference>
@@ -282,7 +311,7 @@ func foo9(a: Int, b: Int) -> Int {
282311
// CHECK13-NEXT: let c = a.byteSwapped
283312
// CHECK13-NEXT: b = b.bigEndian.bigEndian.byteSwapped
284313
// CHECK13-NEXT: print(b + c)</Content>
285-
// CHECK13-NEXT: <Type>()</Type>
314+
// CHECK13-NEXT: <Type>Void</Type>
286315
// CHECK13-NEXT: <Context>swift_ide_test.(file).foo6().explicit closure discriminator=0</Context>
287316
// CHECK13-NEXT: <Declared>a</Declared><OutscopeReference>false</OutscopeReference>
288317
// CHECK13-NEXT: <Declared>b</Declared><OutscopeReference>false</OutscopeReference>
@@ -329,7 +358,7 @@ func foo9(a: Int, b: Int) -> Int {
329358
// CHECK15-NEXT: var b = a.bigEndian
330359
// CHECK15-NEXT: let c = a.byteSwapped
331360
// CHECK15-NEXT: b = b.bigEndian.bigEndian.byteSwapped</Content>
332-
// CHECK15-NEXT: <Type>()</Type>
361+
// CHECK15-NEXT: <Type>Void</Type>
333362
// CHECK15-NEXT: <Context>swift_ide_test.(file).foo6().explicit closure discriminator=0.explicit closure discriminator=0</Context>
334363
// CHECK15-NEXT: <Declared>a</Declared><OutscopeReference>false</OutscopeReference>
335364
// CHECK15-NEXT: <Declared>b</Declared><OutscopeReference>true</OutscopeReference>
@@ -402,3 +431,12 @@ func foo9(a: Int, b: Int) -> Int {
402431
// CHECK24: <Orphan>Break</Orphan>
403432
// CHECK25: <Orphan>Break</Orphan>
404433
// CHECK26: <Orphan>Continue</Orphan>
434+
435+
// CHECK27: <Kind>MultiStatement</Kind>
436+
// CHECK27-NEXT: <Content>foo9(1, 2)
437+
// CHECK27-NEXT: foo9(1, 2)</Content>
438+
// CHECK27-NEXT: <Type>Void</Type>
439+
// CHECK27-NEXT: <Context>swift_ide_test.(file).foo9(_:_:)</Context>
440+
// CHECK27-NEXT: <Referenced>foo9</Referenced><Type>(Int, Int) -> Int</Type>
441+
// CHECK27-NEXT: <ASTNodes>2</ASTNodes>
442+
// CHECK27-NEXT: <end>

0 commit comments

Comments
 (0)