From 9037639eb12c0ace8b0693bab7ea72060027e586 Mon Sep 17 00:00:00 2001 From: Jakob Gruber Date: Fri, 17 Nov 2017 16:23:55 +0000 Subject: [PATCH] Revert "[coverage] add coverage for binary expressions" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 4d3bc552b5b359c208c79c7566ac87846819c179. Reason for revert: https://crbug.com/785778 Original change's description: > [coverage] add coverage for binary expressions > > Adds block-level coverage tracking for binary && and || > expressions. Introduces a BinaryOperation source-range > for tracking the operations themselves and an Expression > source-range, used for tracking NaryLogical expressions. > > This builds on work by jgruber@chromium.org in > the issue. > > TBR=marja@chromium.org > R=​jgruber@chromium.org, rmcilroy@chromium.org > > Bug: v8:6660 > Change-Id: I83a81f13a3514a734c06948b2d3e91138fb00e18 > Reviewed-on: https://chromium-review.googlesource.com/754564 > Commit-Queue: Jakob Gruber > Reviewed-by: Ross McIlroy > Reviewed-by: Jakob Gruber > Cr-Commit-Position: refs/heads/master@{#49304} TBR=rmcilroy@chromium.org,marja@chromium.org,jgruber@chromium.org,ben@npmjs.com # Not skipping CQ checks because original CL landed > 1 day ago. Bug: v8:6660 Change-Id: Ie017c528604b2e01400f527511413eaea5786198 Reviewed-on: https://chromium-review.googlesource.com/776768 Reviewed-by: Jakob Gruber Commit-Queue: Jakob Gruber Cr-Commit-Position: refs/heads/master@{#49454} --- AUTHORS | 1 - src/ast/ast-source-ranges.h | 15 --- src/interpreter/bytecode-generator.cc | 107 ++++---------------- src/interpreter/bytecode-generator.h | 19 ++-- src/parsing/parser-base.h | 8 -- src/parsing/parser.h | 7 -- test/mjsunit/code-coverage-block.js | 138 -------------------------- 7 files changed, 28 insertions(+), 267 deletions(-) diff --git a/AUTHORS b/AUTHORS index 4d3b3ed5f7..6f82d1207e 100644 --- a/AUTHORS +++ b/AUTHORS @@ -46,7 +46,6 @@ Andrew Paprocki Andrei Kashcha Anna Henningsen Bangfu Tao -Ben Coe Ben Noordhuis Benjamin Tan Bert Belder diff --git a/src/ast/ast-source-ranges.h b/src/ast/ast-source-ranges.h index 62f57d6d4c..b6192e6972 100644 --- a/src/ast/ast-source-ranges.h +++ b/src/ast/ast-source-ranges.h @@ -33,7 +33,6 @@ struct SourceRange { V(Block) \ V(CaseClause) \ V(Conditional) \ - V(Expression) \ V(IfStatement) \ V(IterationStatement) \ V(JumpStatement) \ @@ -114,20 +113,6 @@ class ConditionalSourceRanges final : public AstNodeSourceRanges { SourceRange else_range_; }; -class ExpressionSourceRanges final : public AstNodeSourceRanges { - public: - explicit ExpressionSourceRanges(const SourceRange& body_range) - : body_range_(body_range) {} - - SourceRange GetRange(SourceRangeKind kind) { - DCHECK_EQ(kind, SourceRangeKind::kBody); - return body_range_; - } - - private: - SourceRange body_range_; -}; - class IfStatementSourceRanges final : public AstNodeSourceRanges { public: explicit IfStatementSourceRanges(const SourceRange& then_range, diff --git a/src/interpreter/bytecode-generator.cc b/src/interpreter/bytecode-generator.cc index cbbad9f9d0..c1b6e4f6ae 100644 --- a/src/interpreter/bytecode-generator.cc +++ b/src/interpreter/bytecode-generator.cc @@ -494,32 +494,6 @@ class BytecodeGenerator::ControlScopeForTryFinally final DeferredCommands* commands_; }; -// Allocate and fetch the coverage indices tracking NaryLogical Expressions. -class BytecodeGenerator::NArayCodeCoverageSlots { - public: - explicit NArayCodeCoverageSlots(BytecodeGenerator* generator, - NaryOperation* expr) - : generator_(generator) { - if (generator_->block_coverage_builder_ == nullptr) return; - for (size_t i = 0; i < expr->subsequent_length(); i++) { - coverage_slots_.push_back(generator_->AllocateBlockCoverageSlotIfEnabled( - expr->subsequent(i), SourceRangeKind::kBody)); - } - } - - int GetSlotFor(size_t subsequent_expr_index) const { - if (generator_->block_coverage_builder_ == nullptr) { - return BlockCoverageBuilder::kNoCoverageArraySlot; - } - DCHECK(coverage_slots_.size() > subsequent_expr_index); - return coverage_slots_[subsequent_expr_index]; - } - - private: - BytecodeGenerator* generator_; - std::vector coverage_slots_; -}; - void BytecodeGenerator::ControlScope::PerformCommand(Command command, Statement* statement, int source_position) { @@ -4123,7 +4097,7 @@ void BytecodeGenerator::VisitNaryCommaExpression(NaryOperation* expr) { void BytecodeGenerator::VisitLogicalTestSubExpression( Token::Value token, Expression* expr, BytecodeLabels* then_labels, - BytecodeLabels* else_labels, int coverage_slot) { + BytecodeLabels* else_labels) { DCHECK(token == Token::OR || token == Token::AND); BytecodeLabels test_next(zone()); @@ -4134,28 +4108,23 @@ void BytecodeGenerator::VisitLogicalTestSubExpression( VisitForTest(expr, &test_next, else_labels, TestFallthrough::kThen); } test_next.Bind(builder()); - - BuildIncrementBlockCoverageCounterIfEnabled(coverage_slot); } void BytecodeGenerator::VisitLogicalTest(Token::Value token, Expression* left, - Expression* right, - int right_coverage_slot) { + Expression* right) { DCHECK(token == Token::OR || token == Token::AND); TestResultScope* test_result = execution_result()->AsTest(); BytecodeLabels* then_labels = test_result->then_labels(); BytecodeLabels* else_labels = test_result->else_labels(); TestFallthrough fallthrough = test_result->fallthrough(); - VisitLogicalTestSubExpression(token, left, then_labels, else_labels, - right_coverage_slot); + VisitLogicalTestSubExpression(token, left, then_labels, else_labels); // The last test has the same then, else and fallthrough as the parent test. VisitForTest(right, then_labels, else_labels, fallthrough); } -void BytecodeGenerator::VisitNaryLogicalTest( - Token::Value token, NaryOperation* expr, - const NArayCodeCoverageSlots* coverage_slots) { +void BytecodeGenerator::VisitNaryLogicalTest(Token::Value token, + NaryOperation* expr) { DCHECK(token == Token::OR || token == Token::AND); DCHECK_GT(expr->subsequent_length(), 0); @@ -4164,21 +4133,18 @@ void BytecodeGenerator::VisitNaryLogicalTest( BytecodeLabels* else_labels = test_result->else_labels(); TestFallthrough fallthrough = test_result->fallthrough(); - VisitLogicalTestSubExpression(token, expr->first(), then_labels, else_labels, - coverage_slots->GetSlotFor(0)); + VisitLogicalTestSubExpression(token, expr->first(), then_labels, else_labels); for (size_t i = 0; i < expr->subsequent_length() - 1; ++i) { VisitLogicalTestSubExpression(token, expr->subsequent(i), then_labels, - else_labels, - coverage_slots->GetSlotFor(i + 1)); + else_labels); } // The last test has the same then, else and fallthrough as the parent test. VisitForTest(expr->subsequent(expr->subsequent_length() - 1), then_labels, else_labels, fallthrough); } -bool BytecodeGenerator::VisitLogicalOrSubExpression(Expression* expr, - BytecodeLabels* end_labels, - int coverage_slot) { +bool BytecodeGenerator::VisitLogicalOrSubExpression( + Expression* expr, BytecodeLabels* end_labels) { if (expr->ToBooleanIsTrue()) { VisitForAccumulatorValue(expr); end_labels->Bind(builder()); @@ -4188,15 +4154,11 @@ bool BytecodeGenerator::VisitLogicalOrSubExpression(Expression* expr, builder()->JumpIfTrue(ToBooleanModeFromTypeHint(type_hint), end_labels->New()); } - - BuildIncrementBlockCoverageCounterIfEnabled(coverage_slot); - return false; } -bool BytecodeGenerator::VisitLogicalAndSubExpression(Expression* expr, - BytecodeLabels* end_labels, - int coverage_slot) { +bool BytecodeGenerator::VisitLogicalAndSubExpression( + Expression* expr, BytecodeLabels* end_labels) { if (expr->ToBooleanIsFalse()) { VisitForAccumulatorValue(expr); end_labels->Bind(builder()); @@ -4206,9 +4168,6 @@ bool BytecodeGenerator::VisitLogicalAndSubExpression(Expression* expr, builder()->JumpIfFalse(ToBooleanModeFromTypeHint(type_hint), end_labels->New()); } - - BuildIncrementBlockCoverageCounterIfEnabled(coverage_slot); - return false; } @@ -4216,25 +4175,19 @@ void BytecodeGenerator::VisitLogicalOrExpression(BinaryOperation* binop) { Expression* left = binop->left(); Expression* right = binop->right(); - int right_coverage_slot = - AllocateBlockCoverageSlotIfEnabled(right, SourceRangeKind::kBody); - if (execution_result()->IsTest()) { TestResultScope* test_result = execution_result()->AsTest(); if (left->ToBooleanIsTrue()) { builder()->Jump(test_result->NewThenLabel()); } else if (left->ToBooleanIsFalse() && right->ToBooleanIsFalse()) { - BuildIncrementBlockCoverageCounterIfEnabled(right_coverage_slot); builder()->Jump(test_result->NewElseLabel()); } else { - VisitLogicalTest(Token::OR, left, right, right_coverage_slot); + VisitLogicalTest(Token::OR, left, right); } test_result->SetResultConsumedByTest(); } else { BytecodeLabels end_labels(zone()); - if (VisitLogicalOrSubExpression(left, &end_labels, right_coverage_slot)) { - return; - } + if (VisitLogicalOrSubExpression(left, &end_labels)) return; VisitForAccumulatorValue(right); end_labels.Bind(builder()); } @@ -4244,25 +4197,19 @@ void BytecodeGenerator::VisitNaryLogicalOrExpression(NaryOperation* expr) { Expression* first = expr->first(); DCHECK_GT(expr->subsequent_length(), 0); - NArayCodeCoverageSlots coverage_slots(this, expr); - if (execution_result()->IsTest()) { TestResultScope* test_result = execution_result()->AsTest(); if (first->ToBooleanIsTrue()) { builder()->Jump(test_result->NewThenLabel()); } else { - VisitNaryLogicalTest(Token::OR, expr, &coverage_slots); + VisitNaryLogicalTest(Token::OR, expr); } test_result->SetResultConsumedByTest(); } else { BytecodeLabels end_labels(zone()); - if (VisitLogicalOrSubExpression(first, &end_labels, - coverage_slots.GetSlotFor(0))) { - return; - } + if (VisitLogicalOrSubExpression(first, &end_labels)) return; for (size_t i = 0; i < expr->subsequent_length() - 1; ++i) { - if (VisitLogicalOrSubExpression(expr->subsequent(i), &end_labels, - coverage_slots.GetSlotFor(i + 1))) { + if (VisitLogicalOrSubExpression(expr->subsequent(i), &end_labels)) { return; } } @@ -4277,25 +4224,19 @@ void BytecodeGenerator::VisitLogicalAndExpression(BinaryOperation* binop) { Expression* left = binop->left(); Expression* right = binop->right(); - int right_coverage_slot = - AllocateBlockCoverageSlotIfEnabled(right, SourceRangeKind::kBody); - if (execution_result()->IsTest()) { TestResultScope* test_result = execution_result()->AsTest(); if (left->ToBooleanIsFalse()) { builder()->Jump(test_result->NewElseLabel()); } else if (left->ToBooleanIsTrue() && right->ToBooleanIsTrue()) { - BuildIncrementBlockCoverageCounterIfEnabled(right_coverage_slot); builder()->Jump(test_result->NewThenLabel()); } else { - VisitLogicalTest(Token::AND, left, right, right_coverage_slot); + VisitLogicalTest(Token::AND, left, right); } test_result->SetResultConsumedByTest(); } else { BytecodeLabels end_labels(zone()); - if (VisitLogicalAndSubExpression(left, &end_labels, right_coverage_slot)) { - return; - } + if (VisitLogicalAndSubExpression(left, &end_labels)) return; VisitForAccumulatorValue(right); end_labels.Bind(builder()); } @@ -4305,25 +4246,19 @@ void BytecodeGenerator::VisitNaryLogicalAndExpression(NaryOperation* expr) { Expression* first = expr->first(); DCHECK_GT(expr->subsequent_length(), 0); - NArayCodeCoverageSlots coverage_slots(this, expr); - if (execution_result()->IsTest()) { TestResultScope* test_result = execution_result()->AsTest(); if (first->ToBooleanIsFalse()) { builder()->Jump(test_result->NewElseLabel()); } else { - VisitNaryLogicalTest(Token::AND, expr, &coverage_slots); + VisitNaryLogicalTest(Token::AND, expr); } test_result->SetResultConsumedByTest(); } else { BytecodeLabels end_labels(zone()); - if (VisitLogicalAndSubExpression(first, &end_labels, - coverage_slots.GetSlotFor(0))) { - return; - } + if (VisitLogicalAndSubExpression(first, &end_labels)) return; for (size_t i = 0; i < expr->subsequent_length() - 1; ++i) { - if (VisitLogicalAndSubExpression(expr->subsequent(i), &end_labels, - coverage_slots.GetSlotFor(i + 1))) { + if (VisitLogicalAndSubExpression(expr->subsequent(i), &end_labels)) { return; } } diff --git a/src/interpreter/bytecode-generator.h b/src/interpreter/bytecode-generator.h index 5a9cd14fb9..54b2cdc853 100644 --- a/src/interpreter/bytecode-generator.h +++ b/src/interpreter/bytecode-generator.h @@ -56,7 +56,6 @@ class BytecodeGenerator final : public AstVisitor { class EffectResultScope; class FeedbackSlotCache; class GlobalDeclarationsBuilder; - class NArayCodeCoverageSlots; class RegisterAllocationScope; class TestResultScope; class ValueResultScope; @@ -175,23 +174,20 @@ class BytecodeGenerator final : public AstVisitor { // Visit a logical OR/AND within a test context, rewiring the jumps based // on the expression values. - void VisitLogicalTest(Token::Value token, Expression* left, Expression* right, - int right_coverage_slot); - void VisitNaryLogicalTest(Token::Value token, NaryOperation* expr, - const NArayCodeCoverageSlots* coverage_slots); + void VisitLogicalTest(Token::Value token, Expression* left, + Expression* right); + void VisitNaryLogicalTest(Token::Value token, NaryOperation* expr); // Visit a (non-RHS) test for a logical op, which falls through if the test // fails or jumps to the appropriate labels if it succeeds. void VisitLogicalTestSubExpression(Token::Value token, Expression* expr, BytecodeLabels* then_labels, - BytecodeLabels* else_labels, - int coverage_slot); + BytecodeLabels* else_labels); // Helpers for binary and nary logical op value expressions. - bool VisitLogicalOrSubExpression(Expression* expr, BytecodeLabels* end_labels, - int coverage_slot); + bool VisitLogicalOrSubExpression(Expression* expr, + BytecodeLabels* end_labels); bool VisitLogicalAndSubExpression(Expression* expr, - BytecodeLabels* end_labels, - int coverage_slot); + BytecodeLabels* end_labels); // Visit the header/body of a loop iteration. void VisitIterationHeader(IterationStatement* stmt, @@ -208,7 +204,6 @@ class BytecodeGenerator final : public AstVisitor { void BuildLoadPropertyKey(LiteralProperty* property, Register out_reg); int AllocateBlockCoverageSlotIfEnabled(AstNode* node, SourceRangeKind kind); - void BuildIncrementBlockCoverageCounterIfEnabled(AstNode* node, SourceRangeKind kind); void BuildIncrementBlockCoverageCounterIfEnabled(int coverage_array_slot); diff --git a/src/parsing/parser-base.h b/src/parsing/parser-base.h index ca4f70fe09..91062d4b16 100644 --- a/src/parsing/parser-base.h +++ b/src/parsing/parser-base.h @@ -3081,7 +3081,6 @@ template typename ParserBase::ExpressionT ParserBase::ParseBinaryExpression( int prec, bool accept_IN, bool* ok) { DCHECK_GE(prec, 4); - SourceRange right_range; ExpressionT x = ParseUnaryExpression(CHECK_OK); for (int prec1 = Precedence(peek(), accept_IN); prec1 >= prec; prec1--) { // prec1 >= 4 @@ -3089,25 +3088,18 @@ typename ParserBase::ExpressionT ParserBase::ParseBinaryExpression( impl()->RewriteNonPattern(CHECK_OK); BindingPatternUnexpectedToken(); ArrowFormalParametersUnexpectedToken(); - - SourceRangeScope right_range_scope(scanner(), &right_range); Token::Value op = Next(); int pos = position(); const bool is_right_associative = op == Token::EXP; const int next_prec = is_right_associative ? prec1 : prec1 + 1; ExpressionT y = ParseBinaryExpression(next_prec, accept_IN, CHECK_OK); - right_range_scope.Finalize(); impl()->RewriteNonPattern(CHECK_OK); if (impl()->ShortcutNumericLiteralBinaryExpression(&x, y, op, pos)) { continue; } - if (op == Token::OR || op == Token::AND) { - impl()->RecordExpressionSourceRange(y, right_range); - } - // For now we distinguish between comparisons and other binary // operations. (We could combine the two and get rid of this // code and AST node eventually.) diff --git a/src/parsing/parser.h b/src/parsing/parser.h index c4ecf1416d..44f48ae285 100644 --- a/src/parsing/parser.h +++ b/src/parsing/parser.h @@ -1034,13 +1034,6 @@ class V8_EXPORT_PRIVATE Parser : public NON_EXPORTED_BASE(ParserBase) { new (zone()) ConditionalSourceRanges(then_range, else_range)); } - V8_INLINE void RecordExpressionSourceRange(Expression* node, - const SourceRange& body_range) { - if (source_range_map_ == nullptr) return; - source_range_map_->Insert(node, - new (zone()) ExpressionSourceRanges(body_range)); - } - V8_INLINE void RecordJumpStatementSourceRange(Statement* node, int32_t continuation_position) { if (source_range_map_ == nullptr) return; diff --git a/test/mjsunit/code-coverage-block.js b/test/mjsunit/code-coverage-block.js index 40ca2d81ee..3730770eb7 100644 --- a/test/mjsunit/code-coverage-block.js +++ b/test/mjsunit/code-coverage-block.js @@ -666,142 +666,4 @@ f(); // 0200 {"start":61,"end":150,"count":1}] ); -TestCoverage( -"LogicalOrExpression assignment", -` -const a = true || 99 // 0000 -function b () { // 0050 - const b = a || 2 // 0100 -} // 0150 -b() // 0200 -b() // 0250 -`, -[{"start":0,"end":299,"count":1}, - {"start":15,"end":20,"count":0}, - {"start":50,"end":151,"count":2}, - {"start":114,"end":118,"count":0}]); - -TestCoverage( -"LogicalOrExpression IsTest()", -` -true || false // 0000 -const a = 99 // 0050 -a || 50 // 0100 -const b = false // 0150 -if (b || true) {} // 0200 -`, -[{"start":0,"end":249,"count":1}, - {"start":5,"end":13,"count":0}, - {"start":102,"end":107,"count":0}]); - -TestCoverage( -"LogicalAndExpression assignment", -` -const a = false && 99 // 0000 -function b () { // 0050 - const b = a && 2 // 0100 -} // 0150 -b() // 0200 -b() // 0250 -const c = true && 50 // 0300 -`, -[{"start":0,"end":349,"count":1}, - {"start":16,"end":21,"count":0}, - {"start":50,"end":151,"count":2}, - {"start":114,"end":118,"count":0}]); - -TestCoverage( -"LogicalAndExpression IsTest()", -` -false && true // 0000 -const a = 0 // 0050 -a && 50 // 0100 -const b = true // 0150 -if (b && true) {} // 0200 -true && true // 0250 -`, -[{"start":0,"end":299,"count":1}, - {"start":6,"end":13,"count":0}, - {"start":102,"end":107,"count":0}]); - -TestCoverage( -"NaryLogicalOr assignment", -` -const a = true // 0000 -const b = false // 0050 -const c = false || false || 99 // 0100 -const d = false || true || 99 // 0150 -const e = true || true || 99 // 0200 -const f = b || b || 99 // 0250 -const g = b || a || 99 // 0300 -const h = a || a || 99 // 0350 -const i = a || (b || c) || d // 0400 -`, -[{"start":0,"end":449,"count":1}, - {"start":174,"end":179,"count":0}, - {"start":215,"end":222,"count":0}, - {"start":223,"end":228,"count":0}, - {"start":317,"end":322,"count":0}, - {"start":362,"end":366,"count":0}, - {"start":367,"end":372,"count":0}, - {"start":412,"end":423,"count":0}, - {"start":424,"end":428,"count":0}]); - -TestCoverage( -"NaryLogicalOr IsTest()", -` -const a = true // 0000 -const b = false // 0050 -false || false || 99 // 0100 -false || true || 99 // 0150 -true || true || 99 // 0200 -b || b || 99 // 0250 -b || a || 99 // 0300 -a || a || 99 // 0350 -`, -[{"start":0,"end":399,"count":1}, - {"start":164,"end":169,"count":0}, - {"start":205,"end":212,"count":0}, - {"start":213,"end":218,"count":0}, - {"start":307,"end":312,"count":0}, - {"start":352,"end":356,"count":0}, - {"start":357,"end":362,"count":0}]); - -TestCoverage( -"NaryLogicalAnd assignment", -` -const a = true // 0000 -const b = false // 0050 -const c = false && false && 99 // 0100 -const d = false && true && 99 // 0150 -const e = true && true && 99 // 0200 -const f = true && false || true // 0250 -const g = true || false && true // 0300 -`, -[{"start":0,"end":349,"count":1}, - {"start":116,"end":124,"count":0}, - {"start":125,"end":130,"count":0}, - {"start":166,"end":173,"count":0}, - {"start":174,"end":179,"count":0}, - {"start":315,"end":331,"count":0} -]); - -TestCoverage( -"NaryLogicalAnd IsTest()", -` -const a = true // 0000 -const b = false // 0050 -false && false && 99 // 0100 -false && true && 99 // 0150 -true && true && 99 // 0200 -true && false || true // 0250 -true || false && true // 0300 -`, -[{"start":0,"end":349,"count":1}, - {"start":106,"end":114,"count":0}, - {"start":115,"end":120,"count":0}, - {"start":156,"end":163,"count":0}, - {"start":164,"end":169,"count":0}, - {"start":305,"end":321,"count":0}]); - %DebugToggleBlockCoverage(false);