Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix anonymous rest argument tracing #209

Merged
merged 3 commits into from
Aug 3, 2021

Conversation

st0012
Copy link
Member

@st0012 st0012 commented Jul 30, 2021

Methods with ... parameter doesn't read any argument, so the tracer should just ignore it.

Currently the tracer doesn't handle this case:

❯ ruby -Ilib -r debug target.rb
DEBUGGER: Session start (pid: 54807)
[1, 7] in target.rb
=>    1| binding.b(do: "trace pass 1")
      2|
      3| def foo(...)
      4|   block.call
      5| end
      6|
      7| foo(1)
=>#0    <main> at target.rb:1
(rdbg:binding.break) trace pass 1
Enable PassTracer for 1 (enabled)
Traceback (most recent call last):
        5: from target.rb:7:in `<main>'
        4: from target.rb:4:in `foo'
        3: from /Users/st0012/projects/debug/lib/debug/tracer.rb:142:in `block in setup'
        2: from /Users/st0012/projects/debug/lib/debug/tracer.rb:142:in `each'
        1: from /Users/st0012/projects/debug/lib/debug/tracer.rb:153:in `block (2 levels) in setup'
/Users/st0012/projects/debug/lib/debug/tracer.rb:153:in `local_variable_get': wrong local variable name `*' for #<Binding:0x00007ffb7da840f0> (NameError)

This makes tracer unusable in Rails apps because it uses ... in method delegation

https://github.com/rails/rails/blob/7e8946c4da8b69e3b33a3f16cdfd8923c8756000/activesupport/lib/active_support/core_ext/module/delegation.rb#L198-L199

test/support/utils.rb Outdated Show resolved Hide resolved
test/support/utils.rb Outdated Show resolved Hide resolved
@st0012 st0012 force-pushed the fix-anoymous-rest-argument-tracing branch from e700e71 to 7170eab Compare July 31, 2021 02:14
@ko1
Copy link
Collaborator

ko1 commented Aug 2, 2021

could you rebase it?

@st0012 st0012 force-pushed the fix-anoymous-rest-argument-tracing branch from 7170eab to 05df57a Compare August 2, 2021 14:32
@st0012
Copy link
Member Author

st0012 commented Aug 2, 2021

rebased.

@ko1 ko1 merged commit 730a40d into ruby:master Aug 3, 2021
@st0012 st0012 deleted the fix-anoymous-rest-argument-tracing branch August 3, 2021 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants