Skip to content

Java class reopened to add a super call should dispatch normally#7007

Merged
headius merged 2 commits intojruby:jruby-9.3from
headius:reopened_java_super
Jan 17, 2022
Merged

Java class reopened to add a super call should dispatch normally#7007
headius merged 2 commits intojruby:jruby-9.3from
headius:reopened_java_super

Conversation

@headius
Copy link
Member

@headius headius commented Jan 17, 2022

The logic here was falling into the reified Ruby < Java subclass logic when trying to super out of a reopened normal Java class. When in such a situation, we should detect that we are not in a Ruby subclass of a Java class and not try to use that logic, which depends on additional structures not present on normal proxied Java classes

Fixes #6968.

This logic always blindly proceeded to Ruby < Java subclass super
logic for all JavaProxy, but many JavaProxy are just normal Java
classes wrapped with our Ruby proxy logic. In this latter case, we
need to detect that it is not a Ruby < Java subclass (by the lack
of a JavaProxyClass) and dispatch through normal Java proxy
mechanisms.

Fixes jruby#6968
@headius headius added this to the JRuby 9.3.3.0 milestone Jan 17, 2022
@headius headius merged commit c453613 into jruby:jruby-9.3 Jan 17, 2022
@headius headius deleted the reopened_java_super branch January 17, 2022 21:05
public class SubclassOfClassWithSimpleMethod extends ClassWithSimpleMethod {
public String foo(String s) {
return "foo" + s;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this actually a valid test for the issue? My original test case for #6968 does not override foo() in the subclass.

Copy link
Member Author

Choose a reason for hiding this comment

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

Aha, great catch! The fix does pass with this override removed, but I copy/pasted the wrong code into the body of the subclass. The subclass is fixed in c5bb622.

headius added a commit that referenced this pull request Jan 18, 2022
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 this pull request may close these issues.

2 participants