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

Inconsistency between MRI and JRuby source location. #8079

Open
ioquatix opened this issue Jan 25, 2024 · 5 comments
Open

Inconsistency between MRI and JRuby source location. #8079

ioquatix opened this issue Jan 25, 2024 · 5 comments

Comments

@ioquatix
Copy link
Contributor

samuel@aiko ~/P/i/bake (modernize-gem)> ruby --version
jruby 9.4.5.0 (3.1.4) 2023-11-02 1abae2700f OpenJDK 64-Bit Server VM 21.0.2+13 on 21.0.2+13 +jit [x86_64-linux]

samuel@aiko ~/P/i/bake (modernize-gem)> cat test.rb
class Thing
	attr :name
end

puts Thing.instance_method(:name)

samuel@aiko ~/P/i/bake (modernize-gem)> chruby jruby
samuel@aiko ~/P/i/bake (modernize-gem)> ruby test.rb
#<UnboundMethod: Thing#name(@name)()>
samuel@aiko ~/P/i/bake (modernize-gem)> chruby 3.3
samuel@aiko ~/P/i/bake (modernize-gem)> ruby test.rb
#<UnboundMethod: Thing#name() test.rb:2>

I believe attr is not having the source location set.

@headius
Copy link
Member

headius commented Jan 25, 2024

Probably not, because it would require us to walk the stack to get it. Maybe can do it in 9.5 when we can rely on the lighter weight StackWalker API in JVM.

We could do other tricks too like making a special call site for it, but I dunno if it's worth all that.

@headius headius added this to the JRuby 9.5.0.0 milestone Jan 25, 2024
@ioquatix
Copy link
Contributor Author

I worked around the problem, but I just thought I'd mention the inconsistency.

@rubyFeedback
Copy link

We could do other tricks too like making a special call site for it, but I dunno if it's worth all that.

Ideally 100% compatibility should be attained, but specifically for the "attr", I actually never
used it. I used attr_reader, attr_writer and attr_accessor in the past. However had, I even stopped
doing that some years ago, largely because I always want to have something like:

def foobar?
  @foobar
end; alias foobar foobar?

I like the trailing '?'.

These days, when a class has too many @foo variables, I just store them in a single
hash, which I usually call @internal_hash, and handle via "def reset" (which is the
general method I typically use for resetting towards the initial state).

Evidently many other ruby users may not do something like this, but I just wanted
to add that for me, personally, attr is the least important from among the attr-family,
and even that whole family is not so useful to me personally (though I totally understand
the advantage of being able to quickly and programmatically define accessors, of
course; I am not disputing that part at all).

@headius
Copy link
Member

headius commented Apr 25, 2024

Ideally 100% compatibility should be attained

@rubyFeedback The only missing compatibility here is that methods defined by attr and friends do not know the filename and line number where they were defined, so the source location does not reflect that information. In order for us to support it, all attr functions would need to be able to look up the stack to their call site and use that as the location of the method definition. It's a lot of work for functionality that's not frequently used (and most people don't even know about it).

@headius
Copy link
Member

headius commented Apr 25, 2024

I just pushed #8215 as an example using stack-walking to get the filename and line number, and as expected it's pretty slow. A specialized call site would make this more efficient (basically free) and we could limit this slow mechanism to when attr and friends are called via metaprogramming methods (e.g. "send :attr_reader, ...").

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants