From 680e06002666883537c05f796c31c6eacd6b4858 Mon Sep 17 00:00:00 2001 From: Matt Valentine-House Date: Wed, 20 Nov 2024 16:20:38 +0000 Subject: [PATCH] [prism/compiler] end_cursor should never be NULL This fixes a failed assertion reported to SimpleCov https://github.com/simplecov-ruby/simplecov/issues/1113 This can be repro'd as follows: 1. Create a file `test.rb` containing the following code ``` @foo&.(@bar) ``` 2. require it with branch coverage enabled ``` ruby -rcoverage -e "Coverage.start(branches: true); require_relative 'test.rb'" ``` The assertion is failing because the Prism compiler is incorrectly detecting the start and end cursor position of the call site for the implicit call .() This patch replicates the parse.y behaviour of setting the default end_cursor to be the final closing location of the call node. This behaviour can be verified against `parse.y` by modifying the test command as follows: ``` ruby --parser=parse.y -rcoverage -e "Coverage.start(branches: true); require_relative 'test.rb'" ``` [Bug #20866] --- prism_compile.c | 1 + test/coverage/test_coverage.rb | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/prism_compile.c b/prism_compile.c index fe5fde8d63..84f206ab8f 100644 --- a/prism_compile.c +++ b/prism_compile.c @@ -3630,6 +3630,7 @@ pm_compile_call(rb_iseq_t *iseq, const pm_call_node_t *call_node, LINK_ANCHOR *c const uint8_t *end_cursor = cursors[0]; end_cursor = (end_cursor == NULL || cursors[1] == NULL) ? cursors[1] : (end_cursor > cursors[1] ? end_cursor : cursors[1]); end_cursor = (end_cursor == NULL || cursors[2] == NULL) ? cursors[2] : (end_cursor > cursors[2] ? end_cursor : cursors[2]); + if (!end_cursor) end_cursor = call_node->closing_loc.end; const pm_line_column_t start_location = PM_NODE_START_LINE_COLUMN(scope_node->parser, call_node); const pm_line_column_t end_location = pm_newline_list_line_column(&scope_node->parser->newline_list, end_cursor, scope_node->parser->start_line); diff --git a/test/coverage/test_coverage.rb b/test/coverage/test_coverage.rb index 814b7e088b..9db1f8f253 100644 --- a/test/coverage/test_coverage.rb +++ b/test/coverage/test_coverage.rb @@ -463,6 +463,8 @@ class TestCoverage < Test::Unit::TestCase [:"&.", 3, 7, 0, 7, 6] => {[:then, 4, 7, 0, 7, 6]=>0, [:else, 5, 7, 0, 7, 6]=>1}, [:"&.", 6, 8, 0, 8, 10] => {[:then, 7, 8, 0, 8, 10]=>1, [:else, 8, 8, 0, 8, 10]=>0}, [:"&.", 9, 9, 0, 9, 10] => {[:then, 10, 9, 0, 9, 10]=>0, [:else, 11, 9, 0, 9, 10]=>1}, + [:"&.", 12, 10, 0, 10, 6] => {[:then, 13, 10, 0, 10, 6] => 0, [:else, 14, 10, 0, 10, 6] => 1}, + [:"&.", 15, 11, 0, 11, 5] => {[:then, 16, 11, 0, 11, 5] => 0, [:else, 17, 11, 0, 11, 5] => 1}, } } assert_coverage(<<~"end;", { branches: true }, result) @@ -475,6 +477,8 @@ class TestCoverage < Test::Unit::TestCase b&.foo c&.foo = 1 d&.foo = 1 + d&.(b) + d&.() end; end