Bug #1797 [ruby-core:24463]

trace instruction not generated by compiler

Added by Mark Moseley 373 days ago. Updated 274 days ago.

Status :Closed Start :07/21/2009
Priority :Normal Due date :
Assigned to :- % Done :

100%

Category :-
Target version :-
ruby -v :

1.9.1


Description

With the following code:

1: def foo
2:   a = 5
3:   return a
4: end

No trace instruction is generated for line 3.

If line 3 is just "a", without the return, then it functions as expected.

Associated revisions

Revision 25537
Added by yugui 274 days ago

merges r24579 and r24581 from trunk into ruby_1_9_1. --

  • compile.c (NODE_RETURN): fire return event at explicit return. [ruby-dev:38701]

--

--

  • parse.y (reduce_nodes_gen): preserve NODE_FL_NEWLINE flag during node reducing. [ruby-core:24463]

History

07/27/2009 11:55 AM - Mark Moseley

Here's another one:

1: i = 0
2: 1.upto(2) {
3: i += 1
4: }

No trace instruction is generated for line 3. Add more statements in the {}, and it works fine.

07/30/2009 08:44 AM - Mark Moseley

Re: Jacky Cheung

> in the ruby  can doesn't has line 3
> your code can wirite like this:
> def foo
>   a = 5
> end
>
> that's ok!

Jacky, I understand that functionally it is the same. Look at this as an issue of release vs. debug builds. If Ruby is going to generate trace messages, then I would think that those messages would correspond to all lines of valid code, and not omit the ones that were optimized away.

And I also don't understand why the trace message is generated when line 3 is just "a", instead of "return a". Clearly, something is wrong; there should be no difference between the two.

08/19/2009 02:42 AM - Yusuke Endoh

Hi,

2009/7/21 Mark Moseley <redmine@ruby-lang.org>:
> With the following code:
>
> 1: def foo
> 2:   a = 5
> 3:   return a
> 4: end
>
> No trace instruction is generated for line 3.
>
> If line 3 is just "a", without the return, then it functions as expected.


I think it is a parser's fault, not compiler's.
reduce_nodes (in parse.y) may incorrectly eliminate some nodes marked as
NODE_FL_NEWLINE.
The following patch preserves and propagates the mark.


Index: parse.y
===================================================================
--- parse.y	(revision 24580)
+++ parse.y	(working copy)
@@ -8406,6 +8406,7 @@
      (reduce_nodes(&node->n1), body = &node->n2, 1))

     while (node) {
+	int newline = node->flags & NODE_FL_NEWLINE;
 	switch (nd_type(node)) {
 	  end:
 	  case NODE_NIL:
@@ -8413,9 +8414,11 @@
 	    return;
 	  case NODE_RETURN:
 	    *body = node = node->nd_stts;
+	    if (newline && node) node->flags |= NODE_FL_NEWLINE;
 	    continue;
 	  case NODE_BEGIN:
 	    *body = node = node->nd_body;
+	    if (newline && node) node->flags |= NODE_FL_NEWLINE;
 	    continue;
 	  case NODE_BLOCK:
 	    body = &node->nd_end->nd_head;
@@ -8439,6 +8442,7 @@
 	    return;
 	}
 	node = *body;
+	if (newline && node) node->flags |= NODE_FL_NEWLINE;
     }

 #undef subnodes


I did not completely understand your patch (http://gist.github.com/166330).
`last_line_traced' seems to be a workaround against the above bug, then
to be no longer needed.
But I couldn't understand why change in `compile_massign_lhs' was needed.

Is my above patch enough for you?  If not, could you elaborate the
situation you are facing?

-- 
Yusuke ENDOH <mame@tsg.ne.jp>

08/19/2009 07:49 AM - Mark Moseley

I agree that it would be better to stay within the current NODE_FL_NEWLINE solution and use this patch. I was trying to come up with a solution that also fixed bug #1676, but that appears to be closed now. I'll apply the patches for both and verify the ruby-debug test cases.

08/20/2009 01:15 AM - Yukihiro Matsumoto

Hi,

In message "Re: [ruby-core:24969] Re: [Bug #1797] trace instruction not generated by 	compiler"
    on Wed, 19 Aug 2009 02:41:49 +0900, Yusuke ENDOH <mame@tsg.ne.jp> writes:

|I think it is a parser's fault, not compiler's.
|reduce_nodes (in parse.y) may incorrectly eliminate some nodes marked as
|NODE_FL_NEWLINE.
|The following patch preserves and propagates the mark.

Could you check in?

							matz.

08/21/2009 05:18 AM - Mark Moseley

I've verified that both of my test cases listed above now pass.

However, #1676 still fails. My patch fixes it, although I understand if you want to take a different approach.

And to answer the question about the change that my patch makes to compile_massign_lhs(), that's due to this kind of code:

x = [ 1, 2, 3, 4, 5, 6 ]
a, b, c,
d, e, f = x

Currently compile_massign_lhs() removes a "putnil" inserted prematurely due to the statement spread across two lines. I had to modify it to also remove a "trace" put there as well.

10/28/2009 11:17 PM - Yuki Sonoda

  • Status changed from Open to Closed
  • % Done changed from 0 to 100
This issue was solved with changeset r25537.
Mark, thank you for reporting this issue.
Your contribution to Ruby is greatly appreciated.
May Ruby be with you.

Also available in: Atom PDF