Skip to content

Commit 1f563b3

Browse files
rintarorintaro
committed
[incrParse] Fix SyntaxParsingCache::translateToPreEditPosition()
If the position is in the region that is inserted by the edits, 'pre-edit' position shouldn't exist. So we cannot reuse the node at the position. rdar://problem/45259469 https://bugs.swift.org/browse/SR-8995
1 parent 9464060 commit 1f563b3

File tree

4 files changed

+109
-93
lines changed

4 files changed

+109
-93
lines changed

‎include/swift/Parse/SyntaxParsingCache.h

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,11 @@ struct SourceEdit {
3434
/// The length of the string that replaced the range described above.
3535
size_t ReplacementLength;
3636

37+
SourceEdit(size_t Start, size_t End, size_t ReplacementLength)
38+
: Start(Start), End(End), ReplacementLength(ReplacementLength){};
39+
3740
/// The length of the range that has been replaced
38-
size_t originalLength() { return End - Start; }
41+
size_t originalLength() const { return End - Start; }
3942

4043
/// Check if the characters replaced by this edit fall into the given range
4144
/// or are directly adjacent to it
@@ -65,12 +68,16 @@ class SyntaxParsingCache {
6568
: OldSyntaxTree(OldSyntaxTree) {}
6669

6770
/// Add an edit that transformed the source file which created this cache into
68-
/// the source file that is now being parsed incrementally. The order in which
69-
/// the edits are added using this method needs to be the same order in which
70-
/// the edits were applied to the source file.
71-
void addEdit(size_t Start, size_t End, size_t ReplacementLength) {
72-
Edits.push_back({Start, End, ReplacementLength});
73-
}
71+
/// the source file that is now being parsed incrementally. \c Start must be a
72+
/// position from the *original* source file, and it must not overlap any
73+
/// other edits previously added. For instance, given:
74+
/// (aaa, bbb)
75+
/// 0123456789
76+
/// When you want to turn this into:
77+
/// (c, dddd)
78+
/// 0123456789
79+
/// edits should be: { 1, 4, 1 } and { 6, 9, 4 }.
80+
void addEdit(size_t Start, size_t End, size_t ReplacementLength);
7481

7582
/// Check if a syntax node of the given kind at the given position can be
7683
/// reused for a new syntax tree.
@@ -86,11 +93,13 @@ class SyntaxParsingCache {
8693
getReusedRegions(const SourceFileSyntax &SyntaxTree) const;
8794

8895
/// Translates a post-edit position to a pre-edit position by undoing the
89-
/// specified edits.
96+
/// specified edits. Returns \c None if no pre-edit position exists because
97+
/// the post-edit position has been inserted by an edit.
98+
///
9099
/// Should not be invoked externally. Only public for testing purposes.
91-
static size_t
100+
static Optional<size_t>
92101
translateToPreEditPosition(size_t PostEditPosition,
93-
llvm::SmallVector<SourceEdit, 4> Edits);
102+
ArrayRef<SourceEdit> Edits);
94103

95104
private:
96105
llvm::Optional<Syntax> lookUpFrom(const Syntax &Node, size_t NodeStart,

‎lib/Parse/SyntaxParsingCache.cpp

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,13 @@
1616
using namespace swift;
1717
using namespace swift::syntax;
1818

19+
void SyntaxParsingCache::addEdit(size_t Start, size_t End,
20+
size_t ReplacementLength) {
21+
assert((Edits.empty() || Edits.back().End <= Start) &&
22+
"'Start' must be greater than or equal to 'End' of the previous edit");
23+
Edits.emplace_back(Start, End, ReplacementLength);
24+
}
25+
1926
bool SyntaxParsingCache::nodeCanBeReused(const Syntax &Node, size_t NodeStart,
2027
size_t Position,
2128
SyntaxKind Kind) const {
@@ -80,23 +87,31 @@ llvm::Optional<Syntax> SyntaxParsingCache::lookUpFrom(const Syntax &Node,
8087
return llvm::None;
8188
}
8289

83-
size_t SyntaxParsingCache::translateToPreEditPosition(
84-
size_t PostEditPosition, llvm::SmallVector<SourceEdit, 4> Edits) {
90+
Optional<size_t>
91+
SyntaxParsingCache::translateToPreEditPosition(size_t PostEditPosition,
92+
ArrayRef<SourceEdit> Edits) {
8593
size_t Position = PostEditPosition;
86-
for (auto I = Edits.begin(), E = Edits.end(); I != E; ++I) {
87-
auto Edit = *I;
88-
if (Edit.End + Edit.ReplacementLength - Edit.originalLength() <= Position) {
89-
Position = Position - Edit.ReplacementLength + Edit.originalLength();
90-
}
94+
for (auto &Edit : Edits) {
95+
if (Edit.Start > Position)
96+
// Remaining edits doesn't affect the position. (Edits are sorted)
97+
break;
98+
if (Edit.Start + Edit.ReplacementLength > Position)
99+
// This is a position inserted by the edit, and thus doesn't exist in the
100+
// pre-edit version of the file.
101+
return None;
102+
103+
Position = Position - Edit.ReplacementLength + Edit.originalLength();
91104
}
92105
return Position;
93106
}
94107

95108
llvm::Optional<Syntax> SyntaxParsingCache::lookUp(size_t NewPosition,
96109
SyntaxKind Kind) {
97-
size_t OldPosition = translateToPreEditPosition(NewPosition, Edits);
110+
Optional<size_t> OldPosition = translateToPreEditPosition(NewPosition, Edits);
111+
if (!OldPosition.hasValue())
112+
return None;
98113

99-
auto Node = lookUpFrom(OldSyntaxTree, /*NodeStart=*/0, OldPosition, Kind);
114+
auto Node = lookUpFrom(OldSyntaxTree, /*NodeStart=*/0, *OldPosition, Kind);
100115
if (Node.hasValue()) {
101116
ReusedNodeIds.insert(Node->getId());
102117
}

‎test/incrParse/inserted_text_starts_identifier.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %validate-incrparse %s --test-case STRING
3+
4+
// SR-8995 rdar://problem/45259469
5+
6+
self = <<STRING<|||_ _>>>foo(1)[object1, object2] + o bar(1)

‎unittests/Parse/SyntaxParsingCacheTests.cpp

Lines changed: 60 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -1,119 +1,105 @@
11
#include "swift/Parse/SyntaxParsingCache.h"
22
#include "gtest/gtest.h"
33

4+
#include <iostream>
5+
46
using namespace swift;
57
using namespace llvm;
68

9+
10+
namespace llvm {
11+
template <typename T>
12+
void PrintTo(const Optional<T> &optVal, ::std::ostream *os) {
13+
if (optVal.hasValue())
14+
*os << *optVal;
15+
else
16+
*os << "None";
17+
}
18+
} // namespace llvm
19+
20+
void check(ArrayRef<SourceEdit> Edits, ArrayRef<Optional<size_t>> expected) {
21+
for (size_t Pos = 0; Pos != expected.size(); ++Pos) {
22+
Optional<size_t> PrePos =
23+
SyntaxParsingCache::translateToPreEditPosition(Pos, Edits);
24+
EXPECT_EQ(PrePos, expected[Pos]) << "At post-edit position " << Pos;
25+
}
26+
}
27+
728
class TranslateToPreEditPositionTest : public ::testing::Test {};
829

9-
TEST_F(TranslateToPreEditPositionTest, SingleEditBefore) {
30+
TEST_F(TranslateToPreEditPositionTest, SingleEdit1) {
1031
// Old: ab_xy
11-
// New: a1b_xy
12-
//
13-
// Edits:
14-
// (1) 1-2: a -> a1
15-
//
16-
// Lookup for _ at new position 4
32+
// New: c_xy
1733

1834
llvm::SmallVector<SourceEdit, 4> Edits = {
19-
{1, 2, 2}
35+
{0, 2, 1} // ab -> c
2036
};
2137

22-
size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(4, Edits);
23-
EXPECT_EQ(PreEditPos, 3u);
38+
// c _ x y
39+
check(Edits, {None, 2, 3, 4});
2440
}
2541

26-
TEST_F(TranslateToPreEditPositionTest, SingleEditDirectlyBefore) {
42+
TEST_F(TranslateToPreEditPositionTest, SingleEdit) {
2743
// Old: ab_xy
2844
// New: ablah_xy
29-
//
30-
// Edits:
31-
// (1) 2-3: b -> blah
32-
//
33-
// Lookup for _ at new position 6
3445

3546
llvm::SmallVector<SourceEdit, 4> Edits = {
36-
{2, 3, 4}
47+
{1, 2, 4} // b -> blah
3748
};
3849

39-
size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(6, Edits);
40-
EXPECT_EQ(PreEditPos, 3u);
50+
// a b l a h _ x y
51+
check(Edits, {0, None, None, None, None, 2, 3, 4});
4152
}
4253

43-
TEST_F(TranslateToPreEditPositionTest, SingleMultiCharacterEdit) {
54+
TEST_F(TranslateToPreEditPositionTest, SingleInsert) {
4455
// Old: ab_xy
45-
// New: abcdef_xy
46-
//
47-
// Edits:
48-
// (1) 1-3: ab -> abcdef
49-
//
50-
// Lookup for _ at new position 7
56+
// New: 0123ab_xy
5157

5258
llvm::SmallVector<SourceEdit, 4> Edits = {
53-
{1, 3, 6}
59+
{0, 0, 4} // '' -> 0123
5460
};
5561

56-
size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(7, Edits);
57-
EXPECT_EQ(PreEditPos, 3u);
62+
// 0 1 2 3 a b _ x y
63+
check(Edits, { None, None, None, None, 0, 1, 2, 3, 4});
5864
}
5965

60-
TEST_F(TranslateToPreEditPositionTest, EditAfterLookup) {
61-
// Old: ab_xy
62-
// New: ab_xyz
63-
//
64-
// Edits:
65-
// (1) 4-6: xy -> xyz
66-
//
67-
// Lookup for _ at new position 3
66+
TEST_F(TranslateToPreEditPositionTest, SingleDelete) {
67+
// Old: ab_xyz
68+
// New: ab_z
6869

69-
llvm::SmallVector<SourceEdit, 4> Edits = {{4, 6, 4}};
70+
llvm::SmallVector<SourceEdit, 4> Edits = {
71+
{3, 5, 0} // xy -> ''
72+
};
7073

71-
size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(3, Edits);
72-
EXPECT_EQ(PreEditPos, 3u);
74+
// a b _ z
75+
check(Edits, { 0, 1, 2, 5 });
7376
}
7477

7578
TEST_F(TranslateToPreEditPositionTest, SimpleMultiEdit) {
76-
// Old: ab_xy
77-
// New: a1b2_x3y4
78-
//
79-
// Edits:
80-
// (1) 1-2: a -> a1
81-
// (2) 2-3: b -> b2
82-
// (3) 4-5: x -> x3
83-
// (4) 5-6: y -> y4
84-
//
85-
// Lookup for _ at new position 5
79+
// Old: _ab_xy
80+
// New: _a1b2_x3y4
8681

8782
llvm::SmallVector<SourceEdit, 4> Edits = {
88-
{1, 2, 2},
89-
{2, 3, 2},
90-
{4, 5, 2},
91-
{5, 6, 2},
83+
{1, 2, 2}, // a -> a1
84+
{2, 3, 2}, // b -> b2
85+
{4, 5, 2}, // x -> x3
86+
{5, 6, 2}, // y -> y4
9287
};
9388

94-
size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(5, Edits);
95-
EXPECT_EQ(PreEditPos, 3u);
89+
// _ a 1 b 1 _ x 3 y 4
90+
check(Edits, {0, None, None, None, None, 3, None, None, None, None});
9691
}
9792

98-
TEST_F(TranslateToPreEditPositionTest, LongMultiEdit) {
99-
// Old: ab_xy
100-
// New: a11111b2_x3y4
101-
//
102-
// Edits:
103-
// (1) 1-2: a -> a11111
104-
// (2) 2-3: b -> b2
105-
// (3) 4-5: x -> x3
106-
// (4) 5-6: y -> y4
107-
//
108-
// Lookup for _ at new position
93+
TEST_F(TranslateToPreEditPositionTest, ComplexMultiEdit) {
94+
// Old: foo_bar_baz
95+
// New: xx_edits_baz
10996

11097
llvm::SmallVector<SourceEdit, 4> Edits = {
111-
{1, 2, 6},
112-
{2, 3, 2},
113-
{4, 5, 2},
114-
{5, 6, 2},
98+
{0, 3, 2}, // foo -> xx
99+
{4, 7, 0}, // bar -> ''
100+
{7, 7, 5}, // '' -> edits
115101
};
116102

117-
size_t PreEditPos = SyntaxParsingCache::translateToPreEditPosition(9, Edits);
118-
EXPECT_EQ(PreEditPos, 3u);
119-
}
103+
// x x _ e d i t s _ b a z
104+
check(Edits, {None, None, 3, None, None, None, None, None, 7, 8, 9, 10});
105+
}

0 commit comments

Comments
 (0)