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

Bind inaccessible methods with informational error output #5823

Open
headius opened this issue Aug 9, 2019 · 4 comments
Open

Bind inaccessible methods with informational error output #5823

headius opened this issue Aug 9, 2019 · 4 comments

Comments

@headius
Copy link
Member

headius commented Aug 9, 2019

For #5821 I discovered that we are not properly checking accessibility for methods when setting up their Ruby proxy classes. I have fixed that issue, by providing a JRuby class as the "caller" module, but I believe we need to do a better job helping people open up modules they need for JRuby apps to work.

I propose the following:

  • When a method in a class is not visible and can't be set accessible, bind a dummy method in its place.
  • The dummy method will provide information on how to open up the module/package in question.
  • This will allow existing code to immediately see what "add-opens" flags they need to add.

I believe the current situation will never scale, since people will see NoMethodError instead of accessibility errors or helpful tips. We have eliminated the JVM's warning but not replaced it with anything of our own. Adding dummy methods would solve this.

@cshupp1
Copy link

cshupp1 commented Aug 9, 2019

Thumbs up on this idea!

@enebo
Copy link
Member

enebo commented Feb 11, 2020

I think this is a great idea but detargetting since we have way more targetted issues than we will solve for 9.3.0.0 already.

@byteit101
Copy link
Member

There are libraries that allow us to add opens. We could detect this situation, warn, and then fix it with something like this code: https://github.com/stefan-zobel/wip/blob/master/src/main/java/misc/AddOpens.java or this library: https://github.com/burningwave/core/blob/fb23c1b1e313fd4c6a2d02bf19657c25db785db1/src/main/java/org/burningwave/core/classes/Modules.java#L176

@headius
Copy link
Member Author

headius commented Nov 16, 2022

Those two examples of adding opens at runtime both require deep reflective access to java.lang classes. In other words, we'd have to open up java.lang in order to add other opens at runtime, and I'm not sure that's something we should be doing, or at least not without deeply considering the implications of having java.lang classes be reflectively mutable by JRuby.

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

4 participants