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

Improve constant lookup in SourceFinder #871

Merged
merged 1 commit into from
Feb 14, 2024

Conversation

tompng
Copy link
Member

@tompng tompng commented Feb 13, 2024

Improve show_source of constant to find const owner more precise.

Constant lookup does not depend on self, but on Module.nesting

X = 1
class A
  Y = 1
  class B
    Z = 1
    [X, Y, Z] # these are visible here. Module.nesting #=> [A::B, A]
  end
end

class A::B
  # [X, Z] # Y is not visible here. Module.nesting #=> [A::B]
end

In this test code I added, X, Y, Z, Array are all visible with value == 1

X = 1
module M
  Y = 1
  Z = 2
end
class A
  Z = 1
  Array = 1
  class B
    include M
    Object.new.instance_eval { binding.irb }
  end
end

Module.nesting is [A::B, A] and lookup order seems A::B → A → A::B.ancestors → A.ancestors → TOPLEVEL(Object)

@tompng tompng marked this pull request as draft February 13, 2024 15:08
@tompng tompng force-pushed the const_search_path branch 2 times, most recently from 6f71459 to 6c10788 Compare February 13, 2024 16:27
@tompng tompng marked this pull request as ready for review February 13, 2024 16:36
elsif parts == [''] # ::ConstName
Object
else # ConstPath::ConstName
eval_receiver_or_owner(parts.join('::'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, the result of this should be the same as eval_receiver_or_owner(signature)? If that's the case, maybe we can store that and use it here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If signature == 'A::B::C', parts.join('::') will be 'A::B'
We need to evaluate A::B in current binding and get base = FooBar::A::B (FooBar part depends on binding's Module.nesting)
Now we can find source by calling base.const_source_location('C')

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, thanks for explaining 👍

base = @irb_context.workspace.binding.receiver.yield_self { |r| r.is_a?(Module) ? r : Object }
file, line = base.const_source_location(signature)
*parts, name = signature.split('::', -1)
base = (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think having these parentheses introduces a new style that didn't exist in this repo before 🤔 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated to

a =
  if cond
  else
  end

style

@st0012 st0012 added the bug Something isn't working label Feb 13, 2024
@tompng tompng merged commit 87c279c into ruby:master Feb 14, 2024
29 checks passed
@tompng tompng deleted the const_search_path branch February 14, 2024 13:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

2 participants