Bug #1797 [ruby-core:24463]
trace instruction not generated by compiler
| 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
merges r24579 and r24581 from trunk into ruby_1_9_1. --
- compile.c (NODE_RETURN): fire return event at explicit return.
[ruby-dev:38701]
--
- test/ruby/test_settracefunc.rb (test_return, test_return2): add two
tests for
[ruby-dev:38701]and[ruby-core:24463].
--
- 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,#1676still 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.