Project

General

Profile

Actions

Bug #1797

closed

trace instruction not generated by compiler

Added by mark-moseley (Mark Moseley) over 14 years ago. Updated almost 13 years ago.

Status:
Closed
Assignee:
-
ruby -v:
1.9.1
[ruby-core:24463]

Description

=begin
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.
=end

Actions #1

Updated by mark-moseley (Mark Moseley) over 14 years ago

=begin
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.

=end

Actions #2

Updated by mark-moseley (Mark Moseley) over 14 years ago

=begin
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.
=end

Actions #3

Updated by mame (Yusuke Endoh) over 14 years ago

=begin
Hi,

2009/7/21 Mark Moseley :

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

=end

Actions #4

Updated by mark-moseley (Mark Moseley) over 14 years ago

=begin
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.
=end

Actions #5

Updated by matz (Yukihiro Matsumoto) over 14 years ago

=begin
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 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.

=end

Actions #6

Updated by mark-moseley (Mark Moseley) over 14 years ago

=begin
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.
=end

Actions #7

Updated by yugui (Yuki Sonoda) over 14 years ago

  • Status changed from Open to Closed
  • % Done changed from 0 to 100

=begin
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.

=end

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0