oracle / graal

GraalVM compiles Java applications into native executables that start instantly, scale fast, and use fewer compute resources 🚀
https://www.graalvm.org
Other
20.4k stars 1.64k forks source link

Would expect that access to host classes static fields can be inherited from interfaces and base classes #10025

Open in-fke opened 1 week ago

in-fke commented 1 week ago

Describe GraalVM and your environment :

Have you verified this issue still happens when using the latest snapshot? No, but the source code still looks the same.

Describe the issue Access to host classes static fields is not inherited from interfaces or base classes. In the given example, in Java and Rhino, "Implementation.INTERFACE_CONSTANT" would yield "X", in Graal, it yields "undefined".

Code snippet or code repository that reproduces the issue

    /**
     * The Interface Interface.
     */
    public interface Interface {

        /** The constant. */
        public static final String INTERFACE_CONSTANT = "X";
    }

    /**
     * The Class BaseImplementation.
     */
    public static class BaseImplementation {

        /** The Constant BASE_CONSTANT. */
        public static final String BASE_CONSTANT = "Y";
    }    

    /**
     * The Class Implementation.
     */
    public static class Implementation extends BaseImplementation implements Interface {

    }

    /**
     * Test.
     */
    @Test
    public void test() {
        try (Context context = Context.newBuilder("js")
                .allowHostAccess(org.graalvm.polyglot.HostAccess.ALL)
                .allowHostClassLookup(className -> true) // Allow access to all classes
                .build()) {

            Value result;

            // yields null, yet in Java, "Implementation.INTERFACE_CONSTANT" yields "X"
            result = context.eval("js", "(Java.type('" + Implementation.class.getName() + "')).INTERFACE_CONSTANT");
            // assertEquals(Implementation.INTERFACE_CONSTANT, result.asString());
            assertNull(result.asString());

            // yields null, yet in Java, "Implementation.BASE_CONSTANT" yields "Y"
            result = context.eval("js", "(Java.type('" + Implementation.class.getName() + "')).BASE_CONSTANT");
            // assertEquals(Implementation.BASE_CONSTANT, result.asString());
            assertNull(result.asString());            

            // yields "X", since we are accessing the interface directly
            result = context.eval("js", "(Java.type('" + Interface.class.getName() + "')).INTERFACE_CONSTANT");
            assertEquals("X", result.asString());            

            // yields "Y", since we are accessing the base class directly
            result = context.eval("js", "(Java.type('" + BaseImplementation.class.getName() + "')).BASE_CONSTANT");
            assertEquals("Y", result.asString());            

        }        
    }

Steps to reproduce the issue See example. The code to "blame" seems to be here: https://github.com/oracle/graal/blob/master/truffle/src/com.oracle.truffle.host/src/com/oracle/truffle/host/HostClassDesc.java#L354

While collectPublicMethods makes use of allowsAccessInheritance and inherits methods, the method collectPublicFields does not. See

                        // do not inherit static fields
                        if (f.getDeclaringClass() == type && hostAccess.allowsAccess(f)) {
                            staticFieldMap.put(f.getName(), HostFieldDesc.unreflect(lookup, f));
                        }

Expected behavior Along the lines of Java and Rhino, one would expect that public static fields are also inherited from Interfaces and Superclasses. If behaviour would be broken by simply changing the code, mabye introducing allowsAccessFieldInheritance would help to introduce this functionality while staying backwards compatible.

in-fke commented 1 week ago

Or rather allowsFieldAccessInheritance.