Skip to content

Commit fe191e8

Browse files
bcoeCommit Bot
bcoe
authored and
Commit Bot
committed
[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 <[email protected]> Reviewed-by: Toon Verwaest <[email protected]> Reviewed-by: Gus Caplan <[email protected]> Reviewed-by: Sigurd Schneider <[email protected]> Commit-Queue: Benjamin Coe <[email protected]> Cr-Commit-Position: refs/heads/master@{#72075}
1 parent 9cb7e57 commit fe191e8

File tree

5 files changed

+89
-12
lines changed

5 files changed

+89
-12
lines changed

‎src/ast/ast-source-ranges.h‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ struct SourceRange {
4747
V(Block) \
4848
V(CaseClause) \
4949
V(Conditional) \
50+
V(Expression) \
5051
V(FunctionLiteral) \
5152
V(IfStatement) \
5253
V(IterationStatement) \
@@ -281,6 +282,24 @@ class NaryOperationSourceRanges final : public AstNodeSourceRanges {
281282
ZoneVector<SourceRange> ranges_;
282283
};
283284

285+
class ExpressionSourceRanges final : public AstNodeSourceRanges {
286+
public:
287+
explicit ExpressionSourceRanges(const SourceRange& right_range)
288+
: right_range_(right_range) {}
289+
290+
SourceRange GetRange(SourceRangeKind kind) override {
291+
DCHECK(HasRange(kind));
292+
return right_range_;
293+
}
294+
295+
bool HasRange(SourceRangeKind kind) override {
296+
return kind == SourceRangeKind::kRight;
297+
}
298+
299+
private:
300+
SourceRange right_range_;
301+
};
302+
284303
class SuspendSourceRanges final : public ContinuationSourceRanges {
285304
public:
286305
explicit SuspendSourceRanges(int32_t continuation_position)

‎src/interpreter/bytecode-generator.cc‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4599,8 +4599,11 @@ void BytecodeGenerator::VisitThrow(Throw* expr) {
45994599
void BytecodeGenerator::VisitPropertyLoad(Register obj, Property* property) {
46004600
if (property->is_optional_chain_link()) {
46014601
DCHECK_NOT_NULL(optional_chaining_null_labels_);
4602+
int right_range =
4603+
AllocateBlockCoverageSlotIfEnabled(property, SourceRangeKind::kRight);
46024604
builder()->LoadAccumulatorWithRegister(obj).JumpIfUndefinedOrNull(
46034605
optional_chaining_null_labels_->New());
4606+
BuildIncrementBlockCoverageCounterIfEnabled(right_range);
46044607
}
46054608

46064609
AssignType property_kind = Property::GetAssignType(property);
@@ -4940,8 +4943,11 @@ void BytecodeGenerator::VisitCall(Call* expr) {
49404943

49414944
if (expr->is_optional_chain_link()) {
49424945
DCHECK_NOT_NULL(optional_chaining_null_labels_);
4946+
int right_range =
4947+
AllocateBlockCoverageSlotIfEnabled(expr, SourceRangeKind::kRight);
49434948
builder()->LoadAccumulatorWithRegister(callee).JumpIfUndefinedOrNull(
49444949
optional_chaining_null_labels_->New());
4950+
BuildIncrementBlockCoverageCounterIfEnabled(right_range);
49454951
}
49464952

49474953
// Evaluate all arguments to the function call and store in sequential args
@@ -5220,7 +5226,10 @@ void BytecodeGenerator::VisitDelete(UnaryOperation* unary) {
52205226
OptionalChainNullLabelScope label_scope(this);
52215227
VisitForAccumulatorValue(property->obj());
52225228
if (property->is_optional_chain_link()) {
5229+
int right_range = AllocateBlockCoverageSlotIfEnabled(
5230+
property, SourceRangeKind::kRight);
52235231
builder()->JumpIfUndefinedOrNull(label_scope.labels()->New());
5232+
BuildIncrementBlockCoverageCounterIfEnabled(right_range);
52245233
}
52255234
Register object = register_allocator()->NewRegister();
52265235
builder()->StoreAccumulatorInRegister(object);

‎src/parsing/parser-base.h‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3297,17 +3297,24 @@ ParserBase<Impl>::ParseLeftHandSideContinuation(ExpressionT result) {
32973297

32983298
bool optional_chaining = false;
32993299
bool is_optional = false;
3300+
int optional_link_begin;
33003301
do {
33013302
switch (peek()) {
33023303
case Token::QUESTION_PERIOD: {
33033304
if (is_optional) {
33043305
ReportUnexpectedToken(peek());
33053306
return impl()->FailureExpression();
33063307
}
3308+
// Include the ?. in the source range position.
3309+
optional_link_begin = scanner()->peek_location().beg_pos;
33073310
Consume(Token::QUESTION_PERIOD);
33083311
is_optional = true;
33093312
optional_chaining = true;
3310-
continue;
3313+
if (Token::IsPropertyOrCall(peek())) continue;
3314+
int pos = position();
3315+
ExpressionT key = ParsePropertyOrPrivatePropertyName();
3316+
result = factory()->NewProperty(result, key, pos, is_optional);
3317+
break;
33113318
}
33123319

33133320
/* Property */
@@ -3387,14 +3394,7 @@ ParserBase<Impl>::ParseLeftHandSideContinuation(ExpressionT result) {
33873394
}
33883395

33893396
default:
3390-
/* Optional Property */
3391-
if (is_optional) {
3392-
DCHECK_EQ(scanner()->current_token(), Token::QUESTION_PERIOD);
3393-
int pos = position();
3394-
ExpressionT key = ParsePropertyOrPrivatePropertyName();
3395-
result = factory()->NewProperty(result, key, pos, is_optional);
3396-
break;
3397-
}
3397+
// Template literals in/after an Optional Chain not supported:
33983398
if (optional_chaining) {
33993399
impl()->ReportMessageAt(scanner()->peek_location(),
34003400
MessageTemplate::kOptionalChainingNoTemplate);
@@ -3405,8 +3405,12 @@ ParserBase<Impl>::ParseLeftHandSideContinuation(ExpressionT result) {
34053405
result = ParseTemplateLiteral(result, position(), true);
34063406
break;
34073407
}
3408-
is_optional = false;
3409-
} while (is_optional || Token::IsPropertyOrCall(peek()));
3408+
if (is_optional) {
3409+
SourceRange chain_link_range(optional_link_begin, end_position());
3410+
impl()->RecordExpressionSourceRange(result, chain_link_range);
3411+
is_optional = false;
3412+
}
3413+
} while (Token::IsPropertyOrCall(peek()));
34103414
if (optional_chaining) return factory()->NewOptionalChain(result);
34113415
return result;
34123416
}

‎src/parsing/parser.h‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,6 +1002,14 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase<Parser>) {
10021002
node, zone()->New<IterationStatementSourceRanges>(body_range));
10031003
}
10041004

1005+
// Used to record source ranges of expressions associated with optional chain:
1006+
V8_INLINE void RecordExpressionSourceRange(Expression* node,
1007+
const SourceRange& right_range) {
1008+
if (source_range_map_ == nullptr) return;
1009+
source_range_map_->Insert(node,
1010+
zone()->New<ExpressionSourceRanges>(right_range));
1011+
}
1012+
10051013
V8_INLINE void RecordSuspendSourceRange(Expression* node,
10061014
int32_t continuation_position) {
10071015
if (source_range_map_ == nullptr) return;

‎test/mjsunit/code-coverage-block.js‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1177,7 +1177,7 @@ a(true); // 0500
11771177
{"start":0,"end":401,"count":2},
11781178
{"start":154,"end":254,"count":0}]);
11791179

1180-
TestCoverage(
1180+
TestCoverage(
11811181
"https://crbug.com/v8/11231 - nullish coalescing",
11821182
`
11831183
const a = true // 0000
@@ -1195,4 +1195,41 @@ const i = c ?? b ?? 'hello' // 0400
11951195
{"start":262,"end":274,"count":0},
11961196
{"start":417,"end":427,"count":0}]);
11971197

1198+
TestCoverage(
1199+
"Optional Chaining",
1200+
`
1201+
const a = undefined || null // 0000
1202+
const b = a?.b // 0050
1203+
const c = a?.['b'] // 0100
1204+
const d = { // 0150
1205+
e: {f: 99, g: () => {return undefined}} // 0200
1206+
} // 0250
1207+
const e = d?.e?.f // 0300
1208+
const f = d?.e?.['f'] // 0350
1209+
const g = d?.e?.f?.g // 0400
1210+
const h = d?.e?.f?.g?.h // 0450
1211+
const i = d?.['d']?.['e']?.['h'] // 0500
1212+
const k = a?.('b') // 0550
1213+
const l = d?.e?.g?.() // 0600
1214+
const m = d?.e?.g?.()?.a?.b // 0650
1215+
delete a?.b // 0700
1216+
const n = d?.[d?.x?.f] // 0750
1217+
if (a?.[d?.x?.f]) { const p = 99 } else {}// 0800
1218+
const p = d?.[d?.x?.f]?.x // 0850
1219+
`,
1220+
[{"start":0,"end":899,"count":1},
1221+
{"start":61,"end":64,"count":0},
1222+
{"start":111,"end":118,"count":0},
1223+
{"start":470,"end":473,"count":0},
1224+
{"start":518,"end":532,"count":0},
1225+
{"start":561,"end":568,"count":0},
1226+
{"start":671,"end":677,"count":0},
1227+
{"start":708,"end":711,"count":0},
1228+
{"start":768,"end":771,"count":0},
1229+
{"start":805,"end":816,"count":0},
1230+
{"start":818,"end":834,"count":0},
1231+
{"start":868,"end":871,"count":0},
1232+
{"start":872,"end":875,"count":0},
1233+
{"start":216,"end":240,"count":2}]);
1234+
11981235
%DebugToggleBlockCoverage(false);

0 commit comments

Comments
 (0)