[ruby-core:125396] [Ruby Bug#22020] Inner call node without all arguments returned by RubyVM::AbstractSyntaxTree.of for call with a block
Issue #22020 has been reported by Eregon (Benoit Daloze). ---------------------------------------- Bug #22020: Inner call node without all arguments returned by RubyVM::AbstractSyntaxTree.of for call with a block https://bugs.ruby-lang.org/issues/22020 * Author: Eregon (Benoit Daloze) * Status: Open * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- ```ruby begin foo(1, 2, kw: :arg) { 42 } rescue => e pp RubyVM::AbstractSyntaxTree.of e.backtrace_locations[0] end ``` ``` $ ruby --parser=parse.y rubyvm_ast_node_id_loc.rb (FCALL@2:2-2:21 :foo (LIST@2:6-2:20 (INTEGER@2:6-2:7 1) (INTEGER@2:9-2:10 2) (HASH@2:12-2:20 (LIST@2:12-2:20 (SYM@2:12-2:15 :kw) (SYM@2:16-2:20 :arg) nil)) nil)) ``` That covers `foo(1, 2, kw: :arg)` so all arguments except the block argument, which looks inconsistent. I believe it should be: ``` $ ruby --parser=parse.y rubyvm_ast_node_id_loc.rb (ITER@2:2-2:28 (FCALL@2:2-2:21 :foo (LIST@2:6-2:20 (INTEGER@2:6-2:7 1) (INTEGER@2:9-2:10 2) (HASH@2:12-2:20 (LIST@2:12-2:20 (SYM@2:12-2:15 :kw) (SYM@2:16-2:20 :arg) nil)) nil)) (SCOPE@2:22-2:28 tbl: [] args: nil body: (INTEGER@2:24-2:26 42))) ``` PR: https://github.com/ruby/ruby/pull/16836 -- https://bugs.ruby-lang.org/
Issue #22020 has been updated by Eregon (Benoit Daloze). The current locations are like this: ``` foo(1, 2, kw: :arg) { 123 } |---------ITER------------| |------FCALL------| |SCOPE| ``` When `foo` does not exist, it's a `NoMethodError`, and `RubyVM::AbstractSyntaxTree.of e.backtrace_locations[0]` is currently returning the `FCALL` node. This is including all arguments but not the block, which seems inconsistent. https://github.com/ruby/ruby/pull/16836 returns the `ITER` node instead for that case. Another error case is: ``` lambda { }.call(1) |--ITER--| |----| |-| FCALL SCOPE ``` This already returns the `ITER` node currently. ErrorHighlight needs to differentiate both. It's able to do so using `point_type` based on the exception class & message in https://github.com/ruby/ruby/pull/16836, but that might not be ideal. Also `RubyVM::AbstractSyntaxTree.of(lambda { })` returns the `ITER` node currently. Alternatives solutions are: 2) Like the PR, return `ITER` for `foo(1, 2, kw: :arg) { 123 }`, but return `SCOPE` instead of `ITER` for `lambda { }.call(1)` and for `RubyVM::AbstractSyntaxTree.of(lambda { })` This seems clean, the block gets its own separate node from the call. One thought is methods also have `SCOPE` (see [here](https://github.com/ruby/ruby/pull/16836#issuecomment-4367216816)), but I think those nodes are never returned from `RubyVM::AbstractSyntaxTree.of` so there should be no ambiguity. 3) Don't change which nodes are returned, instead extend the location of the FCALL to cover the block as well, same as the location of ITER: ``` foo(1, 2, kw: :arg) { 123 } |---------ITER------------| |---------FCALL-----------| |SCOPE| ``` --- For information, this is what it looks like with Prism: ``` foo(1, 2, kw: :arg) { 123 } |-------CallNode----------| |-----| BlockNode ``` ---------------------------------------- Bug #22020: Inner call node without all arguments returned by RubyVM::AbstractSyntaxTree.of for call with a block https://bugs.ruby-lang.org/issues/22020#change-117171 * Author: Eregon (Benoit Daloze) * Status: Open * ruby -v: ruby 4.1.0dev * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- ```ruby begin foo(1, 2, kw: :arg) { 42 } rescue => e pp RubyVM::AbstractSyntaxTree.of e.backtrace_locations[0] end ``` ``` $ ruby --parser=parse.y rubyvm_ast_node_id_loc.rb (FCALL@2:2-2:21 :foo (LIST@2:6-2:20 (INTEGER@2:6-2:7 1) (INTEGER@2:9-2:10 2) (HASH@2:12-2:20 (LIST@2:12-2:20 (SYM@2:12-2:15 :kw) (SYM@2:16-2:20 :arg) nil)) nil)) ``` That covers `foo(1, 2, kw: :arg)` so all arguments except the block argument, which looks inconsistent. I believe it should be: ``` $ ruby --parser=parse.y rubyvm_ast_node_id_loc.rb (ITER@2:2-2:28 (FCALL@2:2-2:21 :foo (LIST@2:6-2:20 (INTEGER@2:6-2:7 1) (INTEGER@2:9-2:10 2) (HASH@2:12-2:20 (LIST@2:12-2:20 (SYM@2:12-2:15 :kw) (SYM@2:16-2:20 :arg) nil)) nil)) (SCOPE@2:22-2:28 tbl: [] args: nil body: (INTEGER@2:24-2:26 42))) ``` which covers `foo(1, 2, kw: :arg) { 42 }`. PR: https://github.com/ruby/ruby/pull/16836 -- https://bugs.ruby-lang.org/
Issue #22020 has been updated by Eregon (Benoit Daloze). My motivation here is I would like to implement `Thread::Bactrace::Location#source_range`. For this to work in `--parser=parse.y` mode it needs to return the same start/end line/column as Prism in all cases of `RubyVM::AbstractSyntaxTree.of(a Thread::Bactrace::Location)`. Some adjustments could maybe be done specifically in the `Thread::Bactrace::Location#source_range` method if not ambiguous, but it seems cleaner to align the location in `RubyVM::AbstractSyntaxTree`, I think at least for this case. Concretely for `no_such_method(1, 2, kw: :arg) { 123 }` if we want to highlight the entire expression raising the NoMethodError we should highlight `no_such_method(1, 2, kw: :arg) { 123 }`, and not `no_such_method(1, 2, kw: :arg)` which is what `RubyVM::AbstractSyntaxTree` currently gives as the location. ---------------------------------------- Bug #22020: Inner call node without all arguments returned by RubyVM::AbstractSyntaxTree.of for call with a block https://bugs.ruby-lang.org/issues/22020#change-117172 * Author: Eregon (Benoit Daloze) * Status: Open * ruby -v: ruby 4.1.0dev * Backport: 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN ---------------------------------------- ```ruby begin foo(1, 2, kw: :arg) { 42 } rescue => e pp RubyVM::AbstractSyntaxTree.of e.backtrace_locations[0] end ``` ``` $ ruby --parser=parse.y rubyvm_ast_node_id_loc.rb (FCALL@2:2-2:21 :foo (LIST@2:6-2:20 (INTEGER@2:6-2:7 1) (INTEGER@2:9-2:10 2) (HASH@2:12-2:20 (LIST@2:12-2:20 (SYM@2:12-2:15 :kw) (SYM@2:16-2:20 :arg) nil)) nil)) ``` That covers `foo(1, 2, kw: :arg)` so all arguments except the block argument, which looks inconsistent. I believe it should be: ``` $ ruby --parser=parse.y rubyvm_ast_node_id_loc.rb (ITER@2:2-2:28 (FCALL@2:2-2:21 :foo (LIST@2:6-2:20 (INTEGER@2:6-2:7 1) (INTEGER@2:9-2:10 2) (HASH@2:12-2:20 (LIST@2:12-2:20 (SYM@2:12-2:15 :kw) (SYM@2:16-2:20 :arg) nil)) nil)) (SCOPE@2:22-2:28 tbl: [] args: nil body: (INTEGER@2:24-2:26 42))) ``` which covers `foo(1, 2, kw: :arg) { 42 }`. PR: https://github.com/ruby/ruby/pull/16836 -- https://bugs.ruby-lang.org/
participants (1)
-
Eregon (Benoit Daloze)