Project

General

Profile

Actions

Bug #20457

open

Final `return` is eliminated from the AST

Added by tenderlovemaking (Aaron Patterson) about 1 month ago. Updated 29 days ago.

Status:
Open
Assignee:
-
Target version:
-
[ruby-core:117714]

Description

Given the following code:

def foo
  a = 1
  return a
end

If you parse this with RubyVM::AbstractSyntaxTree, the AST will be missing the return node. Of course the return node isn't necessary for compilation, but would be required for building an LSP for example.

Here's a full program to demonstrate:

ast = RubyVM::AbstractSyntaxTree.parse DATA.read
pp ast

# Output is like this:
#
# (SCOPE@1:0-4:3
# tbl: []
# args: nil
# body:
#   (DEFN@1:0-4:3
#    mid: :foo
#    body:
#      (SCOPE@1:0-4:3
#       tbl: [:a]
#       args: (ARGS@1:7-1:7 pre_num: 0 pre_init: nil opt: nil first_post: nil post_num: 0 post_init: nil rest: nil kw: nil kwrest: nil block: nil)
#       body: (BLOCK@2:2-3:10 (LASGN@2:2-2:7 :a (INTEGER@2:6-2:7 1)) (LVAR@3:9-3:10 :a)))))

__END__
def foo
  a = 1
  return a
end

Btw, I'm happy to write failing tests for this type of stuff I'm just not sure where to put it! :)


Related issues 1 (1 open0 closed)

Related to Ruby master - Bug #20464: Redundant returns are unreachable in coverageOpenActions

Updated by nobu (Nobuyoshi Nakada) about 1 month ago ยท Edited

Optimizations in the parser such as reduce_nodes that are not intended for the VM will no longer be necessary.

https://github.com/ruby/ruby/pull/10642

Updated by Eregon (Benoit Daloze) about 1 month ago

I want to add for context that naturally Prism already provides this information (and much more) and has a much better API.
Nobody should use RubyVM::AbstractSyntaxTree, as its documentation pretty much already says.

Updated by tenderlovemaking (Aaron Patterson) 29 days ago

Eregon (Benoit Daloze) wrote in #note-2:

I want to add for context that naturally Prism already provides this information (and much more) and has a much better API.
Nobody should use RubyVM::AbstractSyntaxTree, as its documentation pretty much already says.

From what I understand, lrama intends to eventually provide the same AST that Prism does (or a superset). As you say, people shouldn't use RubyVM::AbstractSyntaxTree, but if lrama (basically parse.y) intends on providing the same AST, then I think cases where Prism provides information that lrama doesn't should be considered a bug (and we should track it).

Actions #4

Updated by mame (Yusuke Endoh) 26 days ago

  • Related to Bug #20464: Redundant returns are unreachable in coverage added
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0