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.

Sign up for GitHub

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

[wip] introduce option to consider scala companion objects stable #637

Closed
wants to merge 2 commits into from
Closed

[wip] introduce option to consider scala companion objects stable #637

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2785197
introduce option to consider scala companion objects stable
Aug 24, 2018
15ef894
wip
Aug 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion ...piler.core.common/src/org/graalvm/compiler/core/common/spi/JavaConstantFieldProvider.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
*/
package org.graalvm.compiler.core.common.spi;

import java.lang.annotation.Annotation;

import org.graalvm.compiler.debug.GraalError;
import org.graalvm.compiler.options.Option;
import org.graalvm.compiler.options.OptionKey;
Expand All @@ -42,6 +44,9 @@ public abstract class JavaConstantFieldProvider implements ConstantFieldProvider
static class Options {
@Option(help = "Determines whether to treat final fields with default values as constant.")//
public static final OptionKey<Boolean> TrustFinalDefaultFields = new OptionKey<>(true);

@Option(help = "Determines whether to treat scala companion objects as stable values.")//
public static final OptionKey<Boolean> ConsiderScalaCompanionObjectsStable = new OptionKey<>(false);
}

protected JavaConstantFieldProvider(MetaAccessProvider metaAccess) {
Expand Down Expand Up @@ -98,7 +103,6 @@ protected boolean isFinalFieldValueConstant(ResolvedJavaField field, JavaConstan
return !value.isDefaultForKind() || Options.TrustFinalDefaultFields.getValue(tool.getOptions());
}

@SuppressWarnings("unused")
protected boolean isStableField(ResolvedJavaField field, ConstantFieldTool<?> tool) {
if (isSyntheticEnumSwitchMap(field)) {
return true;
Expand All @@ -109,9 +113,37 @@ protected boolean isStableField(ResolvedJavaField field, ConstantFieldTool<?> to
if (field.equals(stringHashField)) {
return true;
}
if (Options.ConsiderScalaCompanionObjectsStable.getValue(tool.getOptions()) &&
isScalaCompanionObjectField(field)) {
return true;
}
return false;
}

private static boolean isScalaCompanionObjectField(ResolvedJavaField field) {
ResolvedJavaType declaringClass = field.getDeclaringClass();
String declaringClassName = declaringClass.toClassName();
if (declaringClass.isInitialized() && field.getName().equals("MODULE$") && declaringClassName.endsWith("$")) {
try {
String companionInstanceClassName = declaringClassName.substring(0, declaringClassName.length() - 1);
Class<?> companionInstanceClass = ClassLoader.getSystemClassLoader().loadClass(companionInstanceClassName);
Copy link
Member

@dougxc dougxc Aug 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this code is correct. You really want the class loader of declaringClass. This class loader is not exposed by JVMCI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How can I get a hold of the correct class loader? Or is it not possible? I used the system class loader because it seems that's how other parts of the code base load classes

Copy link
Member

@dougxc dougxc Aug 27, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Class loaders are intentionally not part of JVMCI as JVMCI must also operate in environments where class loaders may not be supported (e.g. SVM and libgraal). What you want is the API introduced recently to resolve a String to a JavaType. Note however that this API is HotSpot specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dougxc according to @retronym, we could avoid loading the class here if we read the Scala attribute from declaringClass. Is there a way to access class atributes? I couldn't find anything looking at ResolvedJavaType

return isScalaClass(companionInstanceClass);
} catch (ClassNotFoundException e) {
return false;
}
}
return false;
}

private static boolean isScalaClass(Class<?> cls) {
for (Annotation annotation : cls.getAnnotations()) {
if (annotation.annotationType().getName().equals("scala.reflect.ScalaSignature")) {
return true;
}
}
return (cls.getEnclosingClass() != null) ? isScalaClass(cls.getEnclosingClass()) : false;
}

protected boolean isDefaultStableField(ResolvedJavaField field, ConstantFieldTool<?> tool) {
assert isStableField(field, tool);
if (isSyntheticEnumSwitchMap(field)) {
Expand Down