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

Introduce a DEFAULT visibility to indicate unspecified #8213

Merged
merged 2 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions core/src/main/java/org/jruby/anno/AnnotationBinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ public void processMethodDeclaration(ExecutableElement method) {
String implClass = anno.meta() ? "singletonClass" : "cls";

String baseName = getBaseName(anno.name(), method);
out.println(" javaMethod = new " + annotatedBindingName + "(" + implClass + ", Visibility." + anno.visibility() + ", \"" + baseName + "\");");
out.println(" javaMethod = new " + annotatedBindingName + "(" + implClass + ", Visibility." + anno.visibility().getDefaultVisibilityFor(baseName) + ", \"" + baseName + "\");");
out.println(" populateMethod(javaMethod, " +
join(AnnotationHelper.getArityValue(anno, actualRequired),
quote(method.getSimpleName()),
Expand Down Expand Up @@ -367,7 +367,7 @@ public void processMethodDeclarationMulti(ExecutableElement method) {
String implClass = anno.meta() ? "singletonClass" : "cls";

String baseName = getBaseName(anno.name(), method);
out.println(" javaMethod = new " + annotatedBindingName + "(" + implClass + ", Visibility." + anno.visibility() + ", \"" + baseName + "\");");
out.println(" javaMethod = new " + annotatedBindingName + "(" + implClass + ", Visibility." + anno.visibility().getDefaultVisibilityFor(baseName) + ", \"" + baseName + "\");");
out.println(" populateMethod(javaMethod, " +
join(-1,
quote(method.getSimpleName()),
Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/org/jruby/anno/IndyBinder.java
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,9 @@ public void processMethodDeclarationMulti(List<ExecutableElement> methods) {
mv.dup();

mv.aload(implClass);
mv.getstatic(p(Visibility.class), anno.visibility().name(), ci(Visibility.class));
mv.ldc(AnnotationBinder.getBaseName(anno.name(), methods.get(0)));
String baseName = AnnotationBinder.getBaseName(anno.name(), methods.get(0));
mv.getstatic(p(Visibility.class), anno.visibility().getDefaultVisibilityFor(baseName).name(), ci(Visibility.class));
mv.ldc(baseName);
mv.ldc(encodeSignature(0, 0, 0, 0, 0, true, false));
mv.ldc(true);
mv.ldc(anno.notImplemented());
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/org/jruby/anno/JRubyMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
/**
* The visibility of this method.
*/
Visibility visibility() default Visibility.PUBLIC;
Visibility visibility() default Visibility.DEFAULT;

/**
* What, if anything, method reads from caller's frame
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,13 +357,13 @@ public JavaMethod constructJavaMethod(RubyModule implementationClass, JavaMethod
// Old constructor with no name, use thread-local to pass it.
JavaMethod.NAME_PASSER.set(name);
try {
ic = (JavaMethod) c.getConstructor(RubyModule_and_Visibility).newInstance(implementationClass, desc.anno.visibility());
ic = (JavaMethod) c.getConstructor(RubyModule_and_Visibility).newInstance(implementationClass, desc.anno.visibility().getDefaultVisibilityFor(name));
} finally {
JavaMethod.NAME_PASSER.remove();
}
} else {
// New constructor with name.
ic = (JavaMethod) c.getConstructor(RubyModule_and_Visibility_and_Name).newInstance(implementationClass, desc.anno.visibility(), name);
ic = (JavaMethod) c.getConstructor(RubyModule_and_Visibility_and_Name).newInstance(implementationClass, desc.anno.visibility().getDefaultVisibilityFor(name), name);
}
return ic;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ public DynamicMethod getAnnotatedMethod(final RubyModule implementationClass, fi

return new HandleMethod(
implementationClass,
desc1.anno.visibility(),
desc1.anno.visibility().getDefaultVisibilityFor(desc1.name),
desc1.name,
(min == max) ?
org.jruby.runtime.Signature.from(min, 0, 0, 0, 0, org.jruby.runtime.Signature.Rest.NONE, -1).encode() :
Expand Down
16 changes: 15 additions & 1 deletion core/src/main/java/org/jruby/runtime/Visibility.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
package org.jruby.runtime;

public enum Visibility {
PUBLIC, PROTECTED, PRIVATE, MODULE_FUNCTION, UNDEFINED;
PUBLIC, PROTECTED, PRIVATE, MODULE_FUNCTION, UNDEFINED, DEFAULT;
private static final Visibility[] VALUES = values();

public boolean isPublic() {
Expand All @@ -49,4 +49,18 @@ public boolean isModuleFunction() {
public static Visibility[] getValues() {
return VALUES;
}

public Visibility getDefaultVisibilityFor(String name) {
switch (this) {
case DEFAULT:
switch (name) {
case "initialize":
case "initialize_copy":
return PRIVATE;
}
return PUBLIC;
default:
return this;
}
}
}
36 changes: 36 additions & 0 deletions core/src/test/java/org/jruby/test/TestMethodFactories.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,15 @@

import org.jruby.CompatVersion;
import org.jruby.Ruby;
import org.jruby.RubyClass;
import org.jruby.RubyMethod;
import org.jruby.RubyModule;
import org.jruby.RubyObject;
import org.jruby.anno.JRubyMethod;
import org.jruby.exceptions.ArgumentError;
import org.jruby.internal.runtime.methods.DynamicMethod;
import org.jruby.runtime.ThreadContext;
import org.jruby.runtime.Visibility;
import org.jruby.runtime.builtin.IRubyObject;


Expand Down Expand Up @@ -166,4 +169,37 @@ public void testVersionedMethods() {
assertNotNull(mod.searchMethod("method"));
}

public void testDefaultVisibility() {
RubyClass cls = runtime.defineClass("NoVisibilityInitialize", runtime.getObject(), NoVisibilityInitialize::new);

cls.defineAnnotatedMethods(NoVisibilityInitialize.class);

assertEquals(Visibility.PRIVATE, cls.searchMethod("initialize").getVisibility());
assertEquals(Visibility.PRIVATE, cls.searchMethod("initialize_copy").getVisibility());
assertEquals(Visibility.PUBLIC, cls.searchMethod("some_other_method").getVisibility());
}

public static class NoVisibilityInitialize extends RubyObject {
public NoVisibilityInitialize(Ruby runtime, RubyClass metaClass) {
super(metaClass);
}

// intentionally no visibility
@JRubyMethod
public IRubyObject initialize(ThreadContext context) {
return context.nil;
}

// intentionally no visibility
@JRubyMethod
public IRubyObject initialize_copy(ThreadContext context, IRubyObject copy) {
return context.nil;
}

@JRubyMethod
public IRubyObject some_other_method(ThreadContext context) {
return context.nil;
}
}

}