-
Notifications
You must be signed in to change notification settings - Fork 914
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
Requiring visibility=PRIVATE for native initialize is error-prone #8210
Milestone
Comments
headius
added a commit
to headius/jruby
that referenced
this issue
Apr 25, 2024
In jruby#8146 we saw that DelegateClass can be broken by JRuby extensions that do not explicitly set their initialize methods to Visibility.PRIVATE. This is an error-prone requirement as detailed in jruby#8210, since there's no indication that initialize is special and should always be private. This patch introduces a "DEFAULT" visibility to be the default for the annotation. When binding methods using annotation processing, the DEFAULT visibility will translate itself to PRIVATE for the initialize method and PUBLIC for other cases. This ensures that initialize is always set to PRIVATE if no other visibility is specified, avoiding the bug. If initialize is set to some other visibility, it will keep that visibility. Fixes jruby#8210
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In #8146 we see a side effect of not properly setting up a native
#initialize
method as private:DelegateClass
will attempt to also wrap#initialize
, breaking the delegator's constructor path and leading toArgumentError: not delegated
errors.The issue in that case is that a third-party JRuby extension defines
#initialize
without any change to visibility, which effectively results in it being public. The immediate fix would be to addvisibility = Visibility.PRIVATE
to the@JRubyMethod
annotation, but this is obviously going to be missed by people unfamiliar with the requirement that#initialize
be private. I am opening this bug to consider ways to prevent that from happening in the future.A few ideas:
#initialize
is defined as public. It would be noise on startup, but immediately apparent to any extension writers what they need to do.#initialize
as private, regardless of the annotation.DelegateClass
to always ignore initialize, since delegating it will almost always be wrong and broken. This would make it possible to fix immediately (with a gem release) and dodge the issue of how to handle users that make public#initialize
methods.The workaround is to manually update the visibility of any affected
#initialize
back to private, as shown in #8146.The text was updated successfully, but these errors were encountered: