From 80bc3c809a89c4e84049198712ecd0be17d91bf1 Mon Sep 17 00:00:00 2001 From: Benjamin Coe Date: Fri, 15 Jan 2021 17:26:28 -0800 Subject: [PATCH] deps: V8: cherry-pick fe191e8d05cc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Original commit message: [coverage] optional chaining coverage Implement coverage tracking for optional chains. Bug: v8:10060 Change-Id: I4f29eda64b6d859939f5f58f4fabead649905795 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2573013 Reviewed-by: Leszek Swirski Reviewed-by: Toon Verwaest Reviewed-by: Gus Caplan Reviewed-by: Sigurd Schneider Commit-Queue: Benjamin Coe Cr-Commit-Position: refs/heads/master@{#72075} Refs: https://github.com/v8/v8/commit/fe191e8d05ccfa740432f56d34b1ff2d32265a8d PR-URL: https://github.com/nodejs/node/pull/36956 Reviewed-By: Rich Trott Reviewed-By: Michaël Zasso --- common.gypi | 2 +- deps/v8/src/ast/ast-source-ranges.h | 19 +++++++++ deps/v8/src/interpreter/bytecode-generator.cc | 9 +++++ deps/v8/src/parsing/parser-base.h | 26 +++++++------ deps/v8/src/parsing/parser.h | 8 ++++ deps/v8/test/mjsunit/code-coverage-block.js | 39 ++++++++++++++++++- 6 files changed, 90 insertions(+), 13 deletions(-) diff --git a/common.gypi b/common.gypi index be7c06d4c81882..e5f01195e63850 100644 --- a/common.gypi +++ b/common.gypi @@ -36,7 +36,7 @@ # Reset this number to 0 on major V8 upgrades. # Increment by one for each non-official patch applied to deps/v8. - 'v8_embedder_string': '-node.11', + 'v8_embedder_string': '-node.12', ##### V8 defaults for Node.js ##### diff --git a/deps/v8/src/ast/ast-source-ranges.h b/deps/v8/src/ast/ast-source-ranges.h index 1b42a055dd642d..1e96ec4c27c58b 100644 --- a/deps/v8/src/ast/ast-source-ranges.h +++ b/deps/v8/src/ast/ast-source-ranges.h @@ -47,6 +47,7 @@ struct SourceRange { V(Block) \ V(CaseClause) \ V(Conditional) \ + V(Expression) \ V(FunctionLiteral) \ V(IfStatement) \ V(IterationStatement) \ @@ -281,6 +282,24 @@ class NaryOperationSourceRanges final : public AstNodeSourceRanges { ZoneVector ranges_; }; +class ExpressionSourceRanges final : public AstNodeSourceRanges { + public: + explicit ExpressionSourceRanges(const SourceRange& right_range) + : right_range_(right_range) {} + + SourceRange GetRange(SourceRangeKind kind) override { + DCHECK(HasRange(kind)); + return right_range_; + } + + bool HasRange(SourceRangeKind kind) override { + return kind == SourceRangeKind::kRight; + } + + private: + SourceRange right_range_; +}; + class SuspendSourceRanges final : public ContinuationSourceRanges { public: explicit SuspendSourceRanges(int32_t continuation_position) diff --git a/deps/v8/src/interpreter/bytecode-generator.cc b/deps/v8/src/interpreter/bytecode-generator.cc index f8d79e7764f11e..f26d431057c042 100644 --- a/deps/v8/src/interpreter/bytecode-generator.cc +++ b/deps/v8/src/interpreter/bytecode-generator.cc @@ -4599,8 +4599,11 @@ void BytecodeGenerator::VisitThrow(Throw* expr) { void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* property) { if (property->is_optional_chain_link()) { DCHECK_NOT_NULL(optional_chaining_null_labels_); + int right_range = + AllocateBlockCoverageSlotIfEnabled(property, SourceRangeKind::kRight); builder()->LoadAccumulatorWithRegister(obj).JumpIfUndefinedOrNull( optional_chaining_null_labels_->New()); + BuildIncrementBlockCoverageCounterIfEnabled(right_range); } AssignType property_kind = Property::GetAssignType(property); @@ -4941,8 +4944,11 @@ void BytecodeGenerator::VisitCall(Call* expr) { if (expr->is_optional_chain_link()) { DCHECK_NOT_NULL(optional_chaining_null_labels_); + int right_range = + AllocateBlockCoverageSlotIfEnabled(expr, SourceRangeKind::kRight); builder()->LoadAccumulatorWithRegister(callee).JumpIfUndefinedOrNull( optional_chaining_null_labels_->New()); + BuildIncrementBlockCoverageCounterIfEnabled(right_range); } // Evaluate all arguments to the function call and store in sequential args @@ -5221,7 +5227,10 @@ void BytecodeGenerator::VisitDelete(UnaryOperation* unary) { OptionalChainNullLabelScope label_scope(this); VisitForAccumulatorValue(property->obj()); if (property->is_optional_chain_link()) { + int right_range = AllocateBlockCoverageSlotIfEnabled( + property, SourceRangeKind::kRight); builder()->JumpIfUndefinedOrNull(label_scope.labels()->New()); + BuildIncrementBlockCoverageCounterIfEnabled(right_range); } Register object = register_allocator()->NewRegister(); builder()->StoreAccumulatorInRegister(object); diff --git a/deps/v8/src/parsing/parser-base.h b/deps/v8/src/parsing/parser-base.h index ff5f0af39012eb..fd0dcb79c1fac6 100644 --- a/deps/v8/src/parsing/parser-base.h +++ b/deps/v8/src/parsing/parser-base.h @@ -3297,6 +3297,7 @@ ParserBase::ParseLeftHandSideContinuation(ExpressionT result) { bool optional_chaining = false; bool is_optional = false; + int optional_link_begin; do { switch (peek()) { case Token::QUESTION_PERIOD: { @@ -3304,10 +3305,16 @@ ParserBase::ParseLeftHandSideContinuation(ExpressionT result) { ReportUnexpectedToken(peek()); return impl()->FailureExpression(); } + // Include the ?. in the source range position. + optional_link_begin = scanner()->peek_location().beg_pos; Consume(Token::QUESTION_PERIOD); is_optional = true; optional_chaining = true; - continue; + if (Token::IsPropertyOrCall(peek())) continue; + int pos = position(); + ExpressionT key = ParsePropertyOrPrivatePropertyName(); + result = factory()->NewProperty(result, key, pos, is_optional); + break; } /* Property */ @@ -3387,14 +3394,7 @@ ParserBase::ParseLeftHandSideContinuation(ExpressionT result) { } default: - /* Optional Property */ - if (is_optional) { - DCHECK_EQ(scanner()->current_token(), Token::QUESTION_PERIOD); - int pos = position(); - ExpressionT key = ParsePropertyOrPrivatePropertyName(); - result = factory()->NewProperty(result, key, pos, is_optional); - break; - } + // Template literals in/after an Optional Chain not supported: if (optional_chaining) { impl()->ReportMessageAt(scanner()->peek_location(), MessageTemplate::kOptionalChainingNoTemplate); @@ -3405,8 +3405,12 @@ ParserBase::ParseLeftHandSideContinuation(ExpressionT result) { result = ParseTemplateLiteral(result, position(), true); break; } - is_optional = false; - } while (is_optional || Token::IsPropertyOrCall(peek())); + if (is_optional) { + SourceRange chain_link_range(optional_link_begin, end_position()); + impl()->RecordExpressionSourceRange(result, chain_link_range); + is_optional = false; + } + } while (Token::IsPropertyOrCall(peek())); if (optional_chaining) return factory()->NewOptionalChain(result); return result; } diff --git a/deps/v8/src/parsing/parser.h b/deps/v8/src/parsing/parser.h index d5521acaa1eb2b..3bc5bb0fe53637 100644 --- a/deps/v8/src/parsing/parser.h +++ b/deps/v8/src/parsing/parser.h @@ -1002,6 +1002,14 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase) { node, zone()->New(body_range)); } + // Used to record source ranges of expressions associated with optional chain: + V8_INLINE void RecordExpressionSourceRange(Expression* node, + const SourceRange& right_range) { + if (source_range_map_ == nullptr) return; + source_range_map_->Insert(node, + zone()->New(right_range)); + } + V8_INLINE void RecordSuspendSourceRange(Expression* node, int32_t continuation_position) { if (source_range_map_ == nullptr) return; diff --git a/deps/v8/test/mjsunit/code-coverage-block.js b/deps/v8/test/mjsunit/code-coverage-block.js index 4584f3134a90db..e9d38d71466a9b 100644 --- a/deps/v8/test/mjsunit/code-coverage-block.js +++ b/deps/v8/test/mjsunit/code-coverage-block.js @@ -1177,7 +1177,7 @@ a(true); // 0500 {"start":0,"end":401,"count":2}, {"start":154,"end":254,"count":0}]); - TestCoverage( +TestCoverage( "https://crbug.com/v8/11231 - nullish coalescing", ` const a = true // 0000 @@ -1195,4 +1195,41 @@ const i = c ?? b ?? 'hello' // 0400 {"start":262,"end":274,"count":0}, {"start":417,"end":427,"count":0}]); +TestCoverage( +"Optional Chaining", +` +const a = undefined || null // 0000 +const b = a?.b // 0050 +const c = a?.['b'] // 0100 +const d = { // 0150 + e: {f: 99, g: () => {return undefined}} // 0200 +} // 0250 +const e = d?.e?.f // 0300 +const f = d?.e?.['f'] // 0350 +const g = d?.e?.f?.g // 0400 +const h = d?.e?.f?.g?.h // 0450 +const i = d?.['d']?.['e']?.['h'] // 0500 +const k = a?.('b') // 0550 +const l = d?.e?.g?.() // 0600 +const m = d?.e?.g?.()?.a?.b // 0650 +delete a?.b // 0700 +const n = d?.[d?.x?.f] // 0750 +if (a?.[d?.x?.f]) { const p = 99 } else {}// 0800 +const p = d?.[d?.x?.f]?.x // 0850 +`, +[{"start":0,"end":899,"count":1}, + {"start":61,"end":64,"count":0}, + {"start":111,"end":118,"count":0}, + {"start":470,"end":473,"count":0}, + {"start":518,"end":532,"count":0}, + {"start":561,"end":568,"count":0}, + {"start":671,"end":677,"count":0}, + {"start":708,"end":711,"count":0}, + {"start":768,"end":771,"count":0}, + {"start":805,"end":816,"count":0}, + {"start":818,"end":834,"count":0}, + {"start":868,"end":871,"count":0}, + {"start":872,"end":875,"count":0}, + {"start":216,"end":240,"count":2}]); + %DebugToggleBlockCoverage(false);