summaryrefslogtreecommitdiff
path: root/test/ruby
diff options
context:
space:
mode:
authorHASUMI Hitoshi <hasumikin@gmail.com>2024-03-29 16:18:14 +0900
committerNobuyoshi Nakada <nobu@ruby-lang.org>2024-04-04 18:29:16 +0900
commitf5e387a30010fe68f8073a6eb5a5b80bb834cd72 (patch)
tree2482ec341e8dabbd2dafa34c92bbad9926f3bb9e /test/ruby
parent27622f3eb92287b3a22256609a05eba39d493923 (diff)
Separate SCRIPT_LINES__ from ast.c
This patch suggests relocating the code dealing with `SCRIPT_LINES__` from ast.c to ruby_parser.c. ## Background - I guess `AbstractSyntaxTree.of` method used to use `SCRIPT_LINES__` internally for some reason before - However, now it appears `SCRIPT_LINES__` is no longer used meaningfully by the method - As evidence of this, (and as my patch shows,) removing the function call of `rb_script_lines_for()` from `ast_s_of()` does not affect the result of `test/ruby/test_ast.rb` Given the above, I think two possibilities can be considered: - (A) `AbstractSyntaxTree.of` has not needed `SCRIPT_LINES__` already (I pick this) - (B) We lack a test case of `AbstractSyntaxTree.of` that needs to use `SCRIPT_LINES__` ## Besides, The current implementation causes strange behavior: ```console ruby -e"SCRIPT_LINES__ = {__FILE__ => []}; puts RubyVM::AbstractSyntaxTree.of(->{ 1 + 2 }, keep_script_lines: true).script_lines" => `-e:1:in '<main>': undefined method 'script_lines' for nil (NoMethodError)` ``` I think this is a bug because `AbstractSyntaxTree.of` is not supposed to return `nil` even in this case. This happens due to the ast.c's dependence on `SCRIPT_LINES__`. And at the end of the `ast_s_of()`, `node_find()` can not find the target child node obviously because it doesn't make sense to look for a corresponding node made from the parameter of `AbstractSyntaxTree.of` in the AST tree made from the value of `{__FILE__ => []}` ## Solution Since I think it's good enough `SCRIPT_LINES__` to be only referred by ruby.c, I chose the possibility "(A)" and wrote this patch which moves `rb_script_lines_for()` from ast.c to ruby_parser.c. So as the result: - `ast_s_of()` function no longer look up `SCRIPT_LINES__` - Even so, this patched code passes the existing tests - The strange behavior above no longer happens (I also added a test for it) Please correct me if I miss something🙏
Diffstat (limited to 'test/ruby')
-rw-r--r--test/ruby/test_ast.rb13
1 files changed, 13 insertions, 0 deletions
diff --git a/test/ruby/test_ast.rb b/test/ruby/test_ast.rb
index d19bda118f..90d19c3d68 100644
--- a/test/ruby/test_ast.rb
+++ b/test/ruby/test_ast.rb
@@ -746,6 +746,19 @@ dummy
assert_equal("def test_keep_script_lines_for_of\n", node_method.source.lines.first)
end
+ def test_keep_script_lines_for_of_with_existing_SCRIPT_LINES__that_has__FILE__as_a_key
+ # This test confirms that the bug that previously occurred because of
+ # `AbstractSyntaxTree.of`s unnecessary dependence on SCRIPT_LINES__ does not reproduce.
+ # The bug occurred only if SCRIPT_LINES__ included __FILE__ as a key.
+ lines = [
+ "SCRIPT_LINES__ = {__FILE__ => []}",
+ "puts RubyVM::AbstractSyntaxTree.of(->{ 1 + 2 }, keep_script_lines: true).script_lines",
+ "p SCRIPT_LINES__"
+ ]
+ test_stdout = lines + ['{"-e"=>[]}']
+ assert_in_out_err(["-e", lines.join("\n")], "", test_stdout, [])
+ end
+
def test_source_with_multibyte_characters
ast = RubyVM::AbstractSyntaxTree.parse(%{a("\u00a7");b("\u00a9")}, keep_script_lines: true)
a_fcall, b_fcall = ast.children[2].children