Bug #1797
trace instruction not generated by compiler
| Status: | Closed | Start date: | 07/21/2009 | |
|---|---|---|---|---|
| Priority: | Normal | Due date: | ||
| Assignee: | - | % 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
Updated by mark-moseley (Mark Moseley) almost 3 years ago
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.
Updated by mark-moseley (Mark Moseley) almost 3 years ago
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.
Updated by mame (Yusuke Endoh) almost 3 years ago
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>
Updated by mark-moseley (Mark Moseley) almost 3 years ago
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.
Updated by matz (Yukihiro Matsumoto) almost 3 years ago
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.
Updated by mark-moseley (Mark Moseley) almost 3 years ago
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.
Updated by yugui (Yuki Sonoda) over 2 years ago
- 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.