[liveedit] Fix patching functions with start position zero

For a script '()=>42', the anonymous arrow function has both start and
end position the same as the script function itself. This causes issues
when sorting the SourcePositionEvents of the function, in two ways:

  * If the start positions are the same, we should order by *furthest*
    end position to ensure the stack is in the right order
  * If both start and end are the same, we need to order by function
    literal id to make sure that start order and end order are inversed.

Also, MapLiterals assumes that start+end position uniquely identifies a
function, which is false in this case, so we process the top-level
script function separately in MapLiterals.

Change-Id: I2b2185dc2825018b7ea44c7d0918238e9b1dd972
Reviewed-on: https://chromium-review.googlesource.com/1141741
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#54525}
This commit is contained in:
Leszek Swirski 2018-07-18 16:36:10 +01:00 committed by Commit Bot
parent 0c54033591
commit e8c5a51c3b
2 changed files with 48 additions and 6 deletions

View File

@ -542,9 +542,25 @@ struct SourcePositionEvent {
if (a.position != b.position) return a.position < b.position;
if (a.type != b.type) return a.type < b.type;
if (a.type == LITERAL_STARTS && b.type == LITERAL_STARTS) {
return a.literal->end_position() < b.literal->end_position();
// If the literals start in the same position, we want the one with the
// furthest (i.e. largest) end position to be first.
if (a.literal->end_position() != b.literal->end_position()) {
return a.literal->end_position() > b.literal->end_position();
}
// If they also end in the same position, we want the first in order of
// literal ids to be first.
return a.literal->function_literal_id() <
b.literal->function_literal_id();
} else if (a.type == LITERAL_ENDS && b.type == LITERAL_ENDS) {
return a.literal->start_position() > b.literal->start_position();
// If the literals end in the same position, we want the one with the
// nearest (i.e. largest) start position to be first.
if (a.literal->start_position() != b.literal->start_position()) {
return a.literal->start_position() > b.literal->start_position();
}
// If they also end in the same position, we want the last in order of
// literal ids to be first.
return a.literal->function_literal_id() >
b.literal->function_literal_id();
} else {
return a.pos_diff < b.pos_diff;
}
@ -658,20 +674,33 @@ using LiteralMap = std::unordered_map<FunctionLiteral*, FunctionLiteral*>;
void MapLiterals(const FunctionLiteralChanges& changes,
const std::vector<FunctionLiteral*>& new_literals,
LiteralMap* unchanged, LiteralMap* changed) {
// Track the top-level script function separately as it can overlap fully with
// another function, e.g. the script "()=>42".
const std::pair<int, int> kTopLevelMarker = std::make_pair(-1, -1);
std::map<std::pair<int, int>, FunctionLiteral*> position_to_new_literal;
for (FunctionLiteral* literal : new_literals) {
DCHECK(literal->start_position() != kNoSourcePosition);
DCHECK(literal->end_position() != kNoSourcePosition);
position_to_new_literal[std::make_pair(literal->start_position(),
literal->end_position())] = literal;
std::pair<int, int> key =
literal->function_literal_id() == FunctionLiteral::kIdTypeTopLevel
? kTopLevelMarker
: std::make_pair(literal->start_position(),
literal->end_position());
// Make sure there are no duplicate keys.
DCHECK_EQ(position_to_new_literal.find(key), position_to_new_literal.end());
position_to_new_literal[key] = literal;
}
LiteralMap mappings;
std::unordered_map<FunctionLiteral*, ChangeState> change_state;
for (const auto& change_pair : changes) {
FunctionLiteral* literal = change_pair.first;
const FunctionLiteralChange& change = change_pair.second;
auto it = position_to_new_literal.find(
std::make_pair(change.new_start_position, change.new_end_position));
std::pair<int, int> key =
literal->function_literal_id() == FunctionLiteral::kIdTypeTopLevel
? kTopLevelMarker
: std::make_pair(change.new_start_position,
change.new_end_position);
auto it = position_to_new_literal.find(key);
if (it == position_to_new_literal.end() ||
HasChangedScope(literal, it->second)) {
change_state[literal] = ChangeState::DAMAGED;

View File

@ -0,0 +1,13 @@
// Copyright 2015 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax
// ()=>42 will have the same start and end position as the top-level script.
var foo = eval("()=>{ return 42 }");
assertEquals(42, foo());
%LiveEditPatchScript(foo, "()=>{ return 13 }");
assertEquals(13, foo());