[incrParse] Fix SyntaxParsingCache::translateToPreEditPosition() #20321
Conversation
|
@swift-ci Please smoke test |
|
|
||
| // SR-8995 rdar://problem/45259469 | ||
|
|
||
| self = <<STRING<|||_ _>>>foo(1)[object1, object2] + o bar(1) |
There was a problem hiding this comment.
In this test case, the second _ inserted used to be translated to the "pre-edit" position of bar(1). So the parser tried to reuse bar(1) when parsing _foo(1)...
| static size_t | ||
| translateToPreEditPosition(size_t PostEditPosition, | ||
| llvm::SmallVector<SourceEdit, 4> Edits); | ||
| static bool translateToPreEditPosition(size_t &Position, |
There was a problem hiding this comment.
I think returning an llvm::Optional<size_t> would be cleaner.
| /// specified edits. | ||
| /// specified edits. Returns \c true if pre-edit position cannot be | ||
| /// determined (for example, the position is in a region which is inserted by | ||
| /// a edit). |
There was a problem hiding this comment.
While looking through the PR, it somehow took me fairly long to understand when false is returned. I think it might help, if you add something along the lines of returns \c true if no pre-edit position exists because the post-edit position has been inserted by an en edit. It's up to you to decide though. I understand it now.
lib/Parse/SyntaxParsingCache.cpp
Outdated
| // Remaining edits doesn't affect the position. (Edits are sorted) | ||
| break; | ||
| if (Edit.Start + Edit.ReplacementLength > Position) | ||
| // This is a position inserted by the edit. |
There was a problem hiding this comment.
For more clarity, I'd add and thus doesn't exist in the pre-edit version of the file.
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
rintaro
force-pushed
the
incrparse-rdar45259469
branch
from
f2162b4 to
1f563b3
Compare
|
@ahoppen Thanks! updated the PR. |
|
@swift-ci Please smoke test |
1 similar comment
|
@swift-ci Please smoke test |
|
Awesome! @rintaro , could we cherry-pick it to swift-5.0-branch as well? |
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
CC: @ahoppen