From: ko1@... Date: 2020-05-07T08:31:40+00:00 Subject: [ruby-core:98178] [Ruby master Bug#16819] Line reporting off by one when reporting line of a hash? Issue #16819 has been updated by ko1 (Koichi Sasada). Assignee set to ko1 (Koichi Sasada) Status changed from Open to Assigned Yes, it is intentional behavior because the first instruction of the `A = { ... }` expression is line 9 (`a: 1`). Each line number attribute is attached to the instruction and there is no instruction at the line 8 for the performance, from Ruby 2.5 [Feature #14104]. I agree it is not good result. Before Ruby 2.5, we inserted `trace` instruction and this instruction has "line 8" attribute just before line 9. However, the `trace` instructions are almost nop and we removed it. I wonder there is a good way to solve it. ---------------------------------------- Bug #16819: Line reporting off by one when reporting line of a hash? https://bugs.ruby-lang.org/issues/16819#change-85418 * Author: enebo (Thomas Enebo) * Status: Assigned * Priority: Normal * Assignee: ko1 (Koichi Sasada) * Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN ---------------------------------------- If I run this program: ``` TracePoint.new(:line) { |t| p t.lineno}.enable def foo(a, b) # 2 a + b # 3 end # 4 # 5 foo 1, 2 # 6 # 7 A = { # 8 a: 1, # 9 b: 2 # 10 } # 11 ``` I see: ``` system ~/work/jruby no_sourceposition * 2388% mri26 ../snippets/ast1.rb 2 6 3 9 ``` I believe this 9 should be an 8 (it is what we currently emit for JRuby). I tried to figure out why this is the case and I patched RubyVM::AbstractSyntaxTree with the ability to note newline flag: ``` diff --git a/ast.c b/ast.c index f0e8dd2eaf..df58006a96 100644 --- a/ast.c +++ b/ast.c @@ -7,6 +7,8 @@ #include "vm_core.h" #include "iseq.h" +#define RBOOL(v) ((v) ? Qtrue : Qfalse) + static VALUE rb_mAST; static VALUE rb_cNode; @@ -731,6 +733,16 @@ rb_ast_node_inspect(VALUE self) return str; } +static VALUE +rb_ast_node_newline(VALUE self) +{ + struct ASTNodeData *data; + TypedData_Get_Struct(self, struct ASTNodeData, &rb_node_type, data); + + return RBOOL(data->node->flags & NODE_FL_NEWLINE); +} + + void Init_ast(void) { @@ -756,5 +768,6 @@ Init_ast(void) rb_define_method(rb_cNode, "last_lineno", rb_ast_node_last_lineno, 0); rb_define_method(rb_cNode, "last_column", rb_ast_node_last_column, 0); rb_define_method(rb_cNode, "children", rb_ast_node_children, 0); + rb_define_method(rb_cNode, "newline?", rb_ast_node_newline, 0); rb_define_method(rb_cNode, "inspect", rb_ast_node_inspect, 0); } ``` I also made a simple script: ``` source = File.read ARGV.shift root = RubyVM::AbstractSyntaxTree.parse source def print_node(node, indent = "") if node.respond_to? :first_lineno eol = node.newline? ? " <-- newline" : "" $stdout.write "#{indent}(#{node.type}@#{node.first_lineno-1}-#{node.last_lineno-1})" case node.type when :LIT, :STR puts " = #{node.children[0].inspect}#{eol}" when :ARRAY puts eol node.children[0..-2].each do |child| print_node(child, indent + " ") end when :FCALL puts " = #{node.children[0]}#{eol}" node.children[1..-1].each do |child| print_node(child, indent + " ") end else puts eol node.children.each do |child| print_node(child, indent + " ") end end elsif node.nil? puts "#{indent}nil" else puts "#{indent}#{node.inspect}" end end print_node root puts RubyVM::InstructionSequence.compile(source).disasm ``` If I run this I see MRI has line 8 marked as the newline (which JRuby also matches) but if we look at the disasm it would appear compile.c decided to put the line in a different location: ``` ../ruby/ruby --disable-gems ../snippets/ast_mri.rb ../snippets/ast1.rb (SCOPE@0-10) [] nil (BLOCK@0-10) (CALL@0-0) <-- newline (ITER@0-0) (CALL@0-0) (CONST@0-0) :TracePoint :new (ARRAY@0-0) (LIT@0-0) = :line (SCOPE@0-0) [:t] (ARGS@0-0) 1 nil nil nil 0 nil nil nil nil nil (FCALL@0-0) = p <-- newline (ARRAY@0-0) (CALL@0-0) (DVAR@0-0) :t :lineno nil :enable nil (DEFN@1-3) <-- newline :foo (SCOPE@1-3) [:a, :b] (ARGS@1-1) 2 nil nil nil 0 nil nil nil nil nil (OPCALL@2-2) <-- newline (LVAR@2-2) :a :+ (ARRAY@2-2) (LVAR@2-2) :b (FCALL@5-5) = foo <-- newline (ARRAY@5-5) (LIT@5-5) = 1 (LIT@5-5) = 2 (CDECL@7-10) <-- newline :A (HASH@7-10) (ARRAY@8-9) (LIT@8-8) = :a (LIT@8-8) = 1 (LIT@9-9) = :b (LIT@9-9) = 2 == disasm: #@:1 (1,0)-(11,18)> (catch: FALSE) == catch table | catch type: break st: 0000 ed: 0013 sp: 0000 cont: 0013 | == disasm: #@:1 (1,22)-(1,39)> (catch: FALSE) | == catch table | | catch type: redo st: 0001 ed: 0010 sp: 0000 cont: 0001 | | catch type: next st: 0001 ed: 0010 sp: 0000 cont: 0010 | |------------------------------------------------------------------------ | local table (size: 1, argc: 1 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1]) | [ 1] t@0 | 0000 nop ( 1)[Bc] | 0001 putself [Li] | 0002 getlocal_WC_0 t@0 | 0004 opt_send_without_block , | 0007 opt_send_without_block , | 0010 nop | 0011 leave ( 1)[Br] |------------------------------------------------------------------------ 0000 opt_getinlinecache 7, ( 1)[Li] 0003 getconstant :TracePoint 0005 opt_setinlinecache 0007 putobject :line 0009 send , , block in 0013 nop 0014 opt_send_without_block , ( 1) 0017 pop 0018 putspecialobject 1 ( 2)[Li] 0020 putobject :foo 0022 putiseq foo 0024 opt_send_without_block , 0027 pop 0028 putself ( 6)[Li] 0029 putobject_INT2FIX_1_ 0030 putobject 2 0032 opt_send_without_block , 0035 pop 0036 duphash {:a=>1, :b=>2} ( 9)[Li] 0038 dup ( 8) 0039 putspecialobject 3 0041 setconstant :A 0043 leave == disasm: #:2 (2,0)-(4,3)> (catch: FALSE) local table (size: 2, argc: 2 [opts: 0, rest: -1, post: 0, block: -1, kw: -1@-1, kwrest: -1]) [ 2] a@0 [ 1] b@1 0000 getlocal_WC_0 a@0 ( 3)[LiCa] 0002 getlocal_WC_0 b@1 0004 opt_plus , 0007 leave ( 4)[Re] ``` -- https://bugs.ruby-lang.org/ Unsubscribe: