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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warnings emitted by requiring 'ostruct' #8200

Closed
PragTob opened this issue Apr 15, 2024 · 4 comments 路 Fixed by #8206
Closed

Warnings emitted by requiring 'ostruct' #8200

PragTob opened this issue Apr 15, 2024 · 4 comments 路 Fixed by #8206
Milestone

Comments

@PragTob
Copy link

PragTob commented Apr 15, 2024

馃憢 Hello my friends! Long time no see, some might say the not seeing level is over 9000 ;)

Environment

tobi@qiqi:~/github/simplecov(ci-madness)$ jruby -v
jruby 9.4.6.0 (3.1.4) 2024-02-20 576fab2c51 OpenJDK 64-Bit Server VM 11.0.22+7-post-Ubuntu-0ubuntu222.04.1 on 11.0.22+7-post-Ubuntu-0ubuntu222.04.1 +jit [x86_64-linux]
tobi@qiqi:~/github/simplecov(ci-madness)$ uname -a
Linux qiqi 6.5.0-27-generic #28~22.04.1-Ubuntu SMP PREEMPT_DYNAMIC Fri Mar 15 10:51:06 UTC 2 x86_64 x86_64 x86_64 GNU/Linux

Behaviour

Just requiring open struct yields the following warnings with warnings turned on

tobi@qiqi:~/github/simplecov(ci-madness)$ cat wtwas.rb 
require 'ostruct'
tobi@qiqi:~/github/simplecov(ci-madness)$ jruby -w wtwas.rb 
/home/tobi/.asdf/installs/ruby/jruby-9.4.6.0/lib/ruby/stdlib/ostruct.rb:466: warning: OpenStruct#public_send accesses caller method's state and should not be aliased
/home/tobi/.asdf/installs/ruby/jruby-9.4.6.0/lib/ruby/stdlib/ostruct.rb:466: warning: OpenStruct#method accesses caller method's state and should not be aliased

Expected Behavior

No warnings :)

More context

FWIW the simplecov CI is failing because of this, I'll work around it for now somehow. We have a test that just makes sure we don't emit warnings.

Cheers, and thanks for all your work 馃挌

PragTob added a commit to simplecov-ruby/simplecov that referenced this issue Apr 15, 2024
PragTob added a commit to simplecov-ruby/simplecov that referenced this issue Apr 15, 2024
Deactivate warnings test on JRuby due to jruby/jruby#8200
@headius
Copy link
Member

headius commented Apr 22, 2024

This is related to #7524 which added additional whitelisted methods for which we won't warn.

The problem here is that JRuby wants to warn users when they alias "special" methods that access the caller's frame, since those aliases are often wrapped with another method that breaks that frame access. For example, you can't alias and wrap eval because the eval won't actually execute in the method that calls the wrapper.

These two methods access the caller's frame in order to get the scope for refinement lookups. If called via a wrapper, they won't work properly, because they'll use the wrapper method's scope.

The fix I proposed in #7524 whitelists these names so they won't be aliased, since we know that openstruct does this. In other cases we have worked with @marcandre to reduce the number of methods it aliases in this way, but neither solution is really very good.

@headius
Copy link
Member

headius commented Apr 22, 2024

Further clarification of why we warn: In order to optimize method bodies by eliminating extra frame and scope information, we gather a list of all method names known to access the caller's frame or scope. When one of these methods gets aliased, that becomes a method name that also needs caller access, and any code already compiled to call that new name may not have the right context set up. So we warn to indicate these methods may not behave like the user expects.

headius added a commit to headius/jruby that referenced this issue Apr 22, 2024
The ostruct library aliases all methods from Object/Kernel with
a bang suffix ("!") to allow them to be callable even though the
OpenStruct should support them as field names. Because we use
record these names in a table of known frame-aware methods, we
have typically warned when aliasing them. This rarely comes up,
since aliasing usually is accompanied by wrapping, which breaks
their behavior on all implementations, but this aliasing in
ostruct is unusual and pervasive, leading to issues like jruby#8200.

This patch assumes we're going to have issues with ostruct forever
and eagerly adds the "!" names alongside the regular names for all
method names that match /[a-z_]/, so taht existing methods and
future methods will behavior properly and not warn when aliased.

It adds a small amount to startup, since these method names must
be added twice, but there are not many such names in the system.

Fixes jruby#8200

Replaces jruby#7524
headius added a commit to headius/jruby that referenced this issue Apr 22, 2024
The ostruct library aliases all methods from Object/Kernel with
a bang suffix ("!") to allow them to be callable even though the
OpenStruct should support them as field names. Because we use
record these names in a table of known frame-aware methods, we
have typically warned when aliasing them. This rarely comes up,
since aliasing usually is accompanied by wrapping, which breaks
their behavior on all implementations, but this aliasing in
ostruct is unusual and pervasive, leading to issues like jruby#8200.

This patch assumes we're going to have issues with ostruct forever
and eagerly adds the "!" names alongside the regular names for all
method names that match /[a-z_]/, so taht existing methods and
future methods will behavior properly and not warn when aliased.

It adds a small amount to startup, since these method names must
be added twice, but there are not many such names in the system.

Fixes jruby#8200

Replaces jruby#7524
@headius
Copy link
Member

headius commented Apr 22, 2024

I pushed #8206 with a new sort of fix: eagerly add all frame-aware names with an additional bang-suffixed version in expectation that this will be an ongoing problem with ostruct.

This could probably be reduced to just Kernel, Object, and BasicObject names but that is a messier fix.

@headius headius added this to the JRuby 9.4.7.0 milestone Apr 22, 2024
@PragTob
Copy link
Author

PragTob commented Apr 23, 2024

Thanks for all the work as always Charlie! 馃挌

IMG_20220705_184557

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

Successfully merging a pull request may close this issue.

2 participants