diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/AtomInteropTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/AtomInteropTest.java index 8da2fe340d3c..925fb83fe4c7 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/AtomInteropTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/AtomInteropTest.java @@ -2,14 +2,19 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; +import com.oracle.truffle.api.interop.InteropLibrary; +import com.oracle.truffle.api.interop.InvalidArrayIndexException; +import com.oracle.truffle.api.interop.UnsupportedMessageException; import org.enso.test.utils.ContextUtils; import org.graalvm.polyglot.Context; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; /** @@ -90,7 +95,6 @@ public void typeHasAnyAsSuperType() { assertThat(anyType.getMetaSimpleName(), is("Any")); } - @Ignore("https://github.com/enso-org/enso/issues/10675") @Test public void atomMembersAreConstructorFields_ManyConstructors() { var myTypeAtom = @@ -110,6 +114,242 @@ public void atomMembersAreConstructorFields_ManyConstructors() { containsInAnyOrder("g1", "g2", "g3")); } + @Test + public void methodIsAtomMember() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons a b + method self = 42 + + main = My_Type.Cons "a" "b" + """); + assertThat("Method is a member of the atom", myTypeAtom.getMemberKeys(), hasItem("method")); + assertThat("method is an invokable member", myTypeAtom.canInvokeMember("method"), is(true)); + } + + @Test + public void methodIsAtomMember_InteropLibrary() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons a b + method self = 42 + + main = My_Type.Cons "a" "b" + """); + var atom = ContextUtils.unwrapValue(ctx, myTypeAtom); + var interop = InteropLibrary.getUncached(); + assertThat("Atom has members", interop.hasMembers(atom), is(true)); + assertThat("Method is readable", interop.isMemberReadable(atom, "method"), is(true)); + assertThat("Method is invocable", interop.isMemberInvocable(atom, "method"), is(true)); + assertThat("Field is readable", interop.isMemberReadable(atom, "a"), is(true)); + } + + @Test + public void fieldsFromPrivateConstructorAreInternalMembers() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + private Cons a + + main = My_Type.Cons "a" + """); + var atom = ContextUtils.unwrapValue(ctx, myTypeAtom); + var interop = InteropLibrary.getUncached(); + assertThat("field a is internal", interop.isMemberInternal(atom, "a"), is(true)); + } + + @Test + public void fieldFromPrivateConstructorIsReadable() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + private Cons a + + main = My_Type.Cons "a" + """); + ContextUtils.executeInContext( + ctx, + () -> { + var atom = ContextUtils.unwrapValue(ctx, myTypeAtom); + var interop = InteropLibrary.getUncached(); + assertThat( + "Field from private constructor is readable", + interop.isMemberReadable(atom, "a"), + is(true)); + assertThat( + "Field from private constructor is invocable", + interop.isMemberInvocable(atom, "a"), + is(true)); + assertThat( + "Field from private constructor can be read", + interop.asString(interop.readMember(atom, "a")), + is("a")); + assertThat( + "Field from private constructor can be invoked", + interop.asString(interop.invokeMember(atom, "a")), + is("a")); + return null; + }); + } + + @Test + public void allMethodsAreInternalMembers() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons a + pub_method self = 42 + private priv_method self = 42 + + main = My_Type.Cons "a" + """); + var atom = ContextUtils.unwrapValue(ctx, myTypeAtom); + var interop = InteropLibrary.getUncached(); + assertThat( + "public method is internal member", interop.isMemberInternal(atom, "pub_method"), is(true)); + assertThat( + "private method is internal member", + interop.isMemberInternal(atom, "priv_method"), + is(true)); + } + + @Test + public void allMembersAreReadableAndInvocable() + throws UnsupportedMessageException, InvalidArrayIndexException { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons a + pub_method self = 42 + private priv_method self = 42 + + main = My_Type.Cons "a" + """); + var atom = ContextUtils.unwrapValue(ctx, myTypeAtom); + var interop = InteropLibrary.getUncached(); + var members = interop.getMembers(atom, true); + for (long i = 0; i < interop.getArraySize(members); i++) { + var memberName = interop.asString(interop.readArrayElement(members, i)); + assertThat( + "Member " + memberName + " should be readable", + interop.isMemberReadable(atom, memberName), + is(true)); + assertThat( + "Member " + memberName + " should be invocable", + interop.isMemberInvocable(atom, memberName), + is(true)); + } + } + + @Test + public void constructorIsNotAtomMember() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons a b + method self = 42 + + main = My_Type.Cons "a" "b" + """); + assertThat("Cons is not atom member", myTypeAtom.getMemberKeys(), not(hasItem("Cons"))); + } + + @Test + public void fieldIsInvocable() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons a b + + main = My_Type.Cons 1 2 + """); + ContextUtils.executeInContext( + ctx, + () -> { + var atom = ContextUtils.unwrapValue(ctx, myTypeAtom); + var interop = InteropLibrary.getUncached(); + assertThat("Field a is invocable", interop.isMemberInvocable(atom, "a"), is(true)); + var aField = interop.invokeMember(atom, "a"); + assertThat("Field is a number", interop.asInt(aField), is(1)); + assertThat("Field b is invocable", interop.isMemberInvocable(atom, "b"), is(true)); + return null; + }); + } + + @Test + public void fieldIsReadable() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons a + + main = My_Type.Cons 1 + """); + ContextUtils.executeInContext( + ctx, + () -> { + var atom = ContextUtils.unwrapValue(ctx, myTypeAtom); + var interop = InteropLibrary.getUncached(); + assertThat("Field a is readable", interop.isMemberReadable(atom, "a"), is(true)); + return null; + }); + } + + @Test + public void staticMethodIsNotAtomMember() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons + static_method = 42 + + main = My_Type.Cons + """); + assertThat( + "Static method is not atom member", + myTypeAtom.getMemberKeys(), + not(hasItem(containsString("static_method")))); + } + + @Test + public void constructorIsNotAtomMember_InteropLibrary() { + var myTypeAtom = + ContextUtils.evalModule( + ctx, + """ + type My_Type + Cons a b + method self = 42 + + main = My_Type.Cons "a" "b" + """); + var atom = ContextUtils.unwrapValue(ctx, myTypeAtom); + var interop = InteropLibrary.getUncached(); + assertThat("Cons is not atom member", interop.isMemberExisting(atom, "Cons"), is(false)); + } + @Test public void typeMembersAreConstructors() { var myType = diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java index f3b9a757aab8..c36cc56519ed 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/DebuggingEnsoTest.java @@ -42,7 +42,6 @@ import org.junit.After; import org.junit.Assert; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; public class DebuggingEnsoTest { @@ -483,7 +482,6 @@ public void testAtomFieldsAreReadable() { } } - @Ignore("https://github.com/enso-org/enso/issues/10675") @Test public void testAtomFieldAreReadable_MultipleConstructors() { var fooFunc = diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TypeMembersTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TypeMembersTest.java index 8bac52d718b0..d27720b333b6 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TypeMembersTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/TypeMembersTest.java @@ -4,13 +4,11 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; import java.net.URI; import java.util.Set; import org.enso.test.utils.ContextUtils; import org.graalvm.polyglot.Context; -import org.graalvm.polyglot.PolyglotException; import org.graalvm.polyglot.Source; import org.graalvm.polyglot.Value; import org.junit.After; @@ -78,10 +76,9 @@ public void checkAtomMembers() throws Exception { assertEquals("seven check", 7, seven.asInt()); assertEquals("three check", 3, three.asInt()); - assertMembers("Keys in list1", false, headAtom, "head", "tail", "is_empty"); - assertMembers("Keys in list2", false, endAtom, "head", "tail", "is_empty"); - assertMembers("Keys in list1", false, headAtom, "h", "t"); - assertMembers("Keys in list2", true, endAtom, "h", "t"); + assertMembers("Keys in list1", headAtom, "head", "tail", "is_empty"); + assertMembers("Keys in list2", endAtom, "head", "tail", "is_empty"); + assertMembers("Keys in list1", headAtom, "h", "t"); } @Test @@ -105,24 +102,15 @@ public void ensureNonBuiltinMembersArePresent() throws Exception { var module = ctx.eval(src); var compileError = module.invokeMember("eval_expression", "v"); - assertEquals("all members", compileError.getMemberKeys(), Set.of("to_display_text", "message")); + assertEquals("all members", Set.of("to_display_text", "message"), compileError.getMemberKeys()); } - private static void assertMembers(String msg, boolean invokeFails, Value v, String... keys) { + private static void assertMembers(String msg, Value v, String... keys) { var realKeys = v.getMemberKeys(); for (var k : keys) { assertTrue(msg + " - found " + k + " in " + realKeys, realKeys.contains(k)); assertTrue(msg + " - has member " + k, v.hasMember(k)); - if (invokeFails) { - try { - v.invokeMember(k); - fail("Invoking " + k + " on " + v + " shall fail"); - } catch (PolyglotException ex) { - assertEquals("Field `" + k + "` of IntList could not be found.", ex.getMessage()); - } - } else { - assertNotNull(msg + " - can be invoked", v.invokeMember(k)); - } + assertNotNull(msg + " - can be invoked", v.invokeMember(k)); } } } diff --git a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/privateaccess/PrivateConstructorAccessTest.java b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/privateaccess/PrivateConstructorAccessTest.java index 43bb97e23f58..b4dc714d873d 100644 --- a/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/privateaccess/PrivateConstructorAccessTest.java +++ b/engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/privateaccess/PrivateConstructorAccessTest.java @@ -3,15 +3,12 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.nullValue; import static org.hamcrest.core.AllOf.allOf; import static org.junit.Assert.fail; import java.io.ByteArrayOutputStream; import java.io.IOException; -import java.util.List; import org.enso.common.RuntimeOptions; -import org.enso.interpreter.util.ScalaConversions; import org.enso.polyglot.PolyglotContext; import org.enso.test.utils.ContextUtils; import org.enso.test.utils.ProjectUtils; @@ -40,25 +37,6 @@ public void privateConstructorCanBeCalledInUnknownProject() { } } - @Test - public void privateFieldIsNotExposedToPolyglot() throws IOException { - var mainSrc = - """ - type My_Type - private Cons data - main = My_Type.Cons 42 - """; - var projDir = tempFolder.newFolder().toPath(); - ProjectUtils.createProject("My_Project", mainSrc, projDir); - ProjectUtils.testProjectRun( - projDir, - res -> { - assertThat(res.hasMember("data"), is(false)); - assertThat(res.canInvokeMember("data"), is(false)); - assertThat(res.getMember("data"), is(nullValue())); - }); - } - @Test public void privateConstructorIsNotExposedToPolyglot() throws IOException { var mainSrc = """ @@ -79,39 +57,6 @@ public void privateConstructorIsNotExposedToPolyglot() throws IOException { } } - @Test - public void typeWithPrivateConstructorExposesPublicMethodsToPolyglot() throws IOException { - var mainSrc = - """ - type My_Type - private Cons data - get_data self = self.data - main = - My_Type.Cons 42 - """; - var projDir = tempFolder.newFolder().toPath(); - ProjectUtils.createProject("My_Project", mainSrc, projDir); - var mainSrcPath = projDir.resolve("src").resolve("Main.enso"); - try (var ctx = - ContextUtils.defaultContextBuilder() - .option(RuntimeOptions.PROJECT_ROOT, projDir.toAbsolutePath().toString()) - .build()) { - var polyCtx = new PolyglotContext(ctx); - var mainMod = polyCtx.evalModule(mainSrcPath.toFile()); - var myType = mainMod.getType("My_Type"); - var getDataMethod = mainMod.getMethod(myType, "get_data").get(); - var assocType = mainMod.getAssociatedType(); - var mainMethod = mainMod.getMethod(assocType, "main").get(); - var res = mainMethod.execute(); - assertThat("Atoms should generally have members", res.hasMembers(), is(true)); - assertThat("data is a private field", res.hasMember("data"), is(false)); - assertThat("get_data is a public method", res.hasMember("get_data"), is(true)); - var data = getDataMethod.execute(ScalaConversions.seq(List.of(res))); - assertThat("public accessor method can be called from polyglot", data.isNumber(), is(true)); - assertThat("public accessor method can be called from polyglot", data.asInt(), is(42)); - } - } - @Test public void canPatternMatchOnPrivateConstructorFromSameProject() throws IOException { var mainSrc = diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/MethodRootNode.java b/engine/runtime/src/main/java/org/enso/interpreter/node/MethodRootNode.java index 90674085e6c7..fa1517967415 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/MethodRootNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/MethodRootNode.java @@ -42,12 +42,13 @@ private MethodRootNode( shortName(type.getName(), methodName), null, false); + this.type = type; this.methodName = methodName; } - private static String shortName(String atomName, String methodName) { - return atomName + "." + methodName; + private static String shortName(String typeName, String methodName) { + return typeName + "." + methodName; } /** diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/Atom.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/Atom.java index 4d5fd5a54442..3ace353dc5b6 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/Atom.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/Atom.java @@ -3,7 +3,6 @@ import com.oracle.truffle.api.CompilerDirectives; import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary; import com.oracle.truffle.api.dsl.Cached; -import com.oracle.truffle.api.dsl.Idempotent; import com.oracle.truffle.api.dsl.Specialization; import com.oracle.truffle.api.exception.AbstractTruffleException; import com.oracle.truffle.api.interop.ArityException; @@ -16,10 +15,13 @@ import com.oracle.truffle.api.library.ExportMessage; import com.oracle.truffle.api.nodes.ExplodeLoop; import com.oracle.truffle.api.profiles.BranchProfile; +import java.util.Arrays; import java.util.HashSet; import java.util.Set; +import java.util.stream.Collectors; import org.enso.interpreter.EnsoLanguage; import org.enso.interpreter.runtime.callable.UnresolvedSymbol; +import org.enso.interpreter.runtime.callable.argument.ArgumentDefinition; import org.enso.interpreter.runtime.callable.function.Function; import org.enso.interpreter.runtime.data.EnsoObject; import org.enso.interpreter.runtime.data.Type; @@ -30,7 +32,23 @@ import org.enso.interpreter.runtime.type.TypesGen; import org.enso.interpreter.runtime.warning.WarningsLibrary; -/** A runtime representation of an Atom in Enso. */ +/** + * A runtime representation of an Atom in Enso. + * + *

{@link InteropLibrary Interop} protocol

+ * + * {@link InteropLibrary#getMembers(Object) Members} are fields and methods. Only fields of the atom + * constructor used to construct this atom are considered members. If the constructor is + * project-private, the fields are considered internal members. All methods (public and + * project-private) are considered internal members. + * + *

Since all the members are methods, they are both {@link + * InteropLibrary#isMemberReadable(Object, String) readable} and {@link + * InteropLibrary#isMemberInvocable(Object, String) invocable}. + * + *

Trying to {@link InteropLibrary#invokeMember(Object, String, Object...) invoke} + * project-private method results in {@link UnsupportedMessageException}. + */ @ExportLibrary(InteropLibrary.class) @ExportLibrary(TypesLibrary.class) public abstract class Atom implements EnsoObject { @@ -137,96 +155,179 @@ boolean hasMembers() { } /** - * Returns a polyglot list of all the public members of Atom. A member is any method on the atom. - * A member is public if it is not project-private. + * Returns list of fields of the Atom. If {@code includeInternal} is true, all methods, including + * project-private, are included. Fields are returned as filed getters, i.e., methods. Only fields + * for the constructor that was used to construct this atom are returned. */ @ExportMessage @CompilerDirectives.TruffleBoundary EnsoObject getMembers(boolean includeInternal) { - Set members = - constructor.getDefinitionScope().getMethodsForType(constructor.getType()); - Set allFuncMembers = new HashSet<>(); - if (members != null) { - allFuncMembers.addAll(members); - } - members = constructor.getType().getDefinitionScope().getMethodsForType(constructor.getType()); - if (members != null) { - allFuncMembers.addAll(members); + Set allMembers = new HashSet<>(); + allMembers.addAll(getInstanceMethods()); + + if (includeInternal) { + allMembers.addAll(getFieldGetters()); + } else { + if (!hasProjectPrivateConstructor()) { + allMembers.addAll(getFieldGetters()); + } } - String[] publicMembers = - allFuncMembers.stream() - .filter(method -> !method.getSchema().isProjectPrivate()) - .map(Function::getName) + + String[] filteredMembers = + allMembers.stream() + .filter( + method -> { + if (includeInternal) { + return true; + } else { + return !method.getSchema().isProjectPrivate(); + } + }) + .map( + func -> { + var funcNameItems = func.getName().split("\\."); + return funcNameItems[funcNameItems.length - 1]; + }) .distinct() .toArray(String[]::new); - return ArrayLikeHelpers.wrapStrings(publicMembers); + return ArrayLikeHelpers.wrapStrings(filteredMembers); } - protected boolean isMethodProjectPrivate(Type type, String methodName) { - Function method = constructor.getDefinitionScope().getMethodForType(type, methodName); - if (method != null) { - return method.getSchema().isProjectPrivate(); + /** Get all instance methods for this atom's type. */ + private Set getInstanceMethods() { + var methodsFromCtorScope = + constructor.getDefinitionScope().getMethodsForType(constructor.getType()); + var methodsFromTypeScope = + constructor.getType().getDefinitionScope().getMethodsForType(constructor.getType()); + var allMethods = new HashSet(); + if (methodsFromCtorScope != null) { + allMethods.addAll(methodsFromCtorScope); + } + if (methodsFromTypeScope != null) { + allMethods.addAll(methodsFromTypeScope); } - method = constructor.getType().getDefinitionScope().getMethodForType(type, methodName); - return method != null && method.getSchema().isProjectPrivate(); + return allMethods.stream() + .filter(method -> !isFieldGetter(method)) + .collect(Collectors.toUnmodifiableSet()); } - /** A member is invocable if it is a method on the Atom and it is public. */ - @ExportMessage - @CompilerDirectives.TruffleBoundary - final boolean isMemberInvocable(String member) { - var type = constructor.getType(); - Set members = constructor.getDefinitionScope().getMethodNamesForType(type); - if (members != null && members.contains(member)) { - return !isMethodProjectPrivate(type, member); + /** Get field getters for this atom's constructor. */ + private Set getFieldGetters() { + var allMethods = constructor.getDefinitionScope().getMethodsForType(constructor.getType()); + if (allMethods != null) { + return allMethods.stream() + .filter(method -> isFieldGetter(method) && isGetterForOwnField(method)) + .collect(Collectors.toUnmodifiableSet()); } - members = type.getDefinitionScope().getMethodNamesForType(type); - if (members != null && members.contains(member)) { - return !isMethodProjectPrivate(type, member); + return Set.of(); + } + + /** + * Returns true if the given {@code function} is a getter for a field inside this atom + * constructor. + * + * @param function the function to check. + * @return true if the function is a getter for a field inside this atom constructor. + */ + private boolean isGetterForOwnField(Function function) { + if (function.getCallTarget() != null + && function.getCallTarget().getRootNode() instanceof GetFieldBaseNode getFieldNode) { + var fieldName = getFieldNode.getName(); + var thisConsFieldNames = + Arrays.stream(constructor.getFields()).map(ArgumentDefinition::getName).toList(); + return thisConsFieldNames.contains(fieldName); } return false; } - /** A member is readable if it is an atom constructor and if the atom constructor is public. */ + private boolean isFieldGetter(Function function) { + return function.getCallTarget() != null + && function.getCallTarget().getRootNode() instanceof GetFieldBaseNode; + } + + /** A member is invocable if it is readable, i.e., if it is a field or a method. */ + @ExportMessage + @CompilerDirectives.TruffleBoundary + final boolean isMemberInvocable(String member) { + return isMemberReadable(member); + } + + /** Readable members are fields of non-project-private constructors and public methods. */ @ExportMessage @ExplodeLoop final boolean isMemberReadable(String member) { + for (int i = 0; i < constructor.getArity(); i++) { + if (member.equals(constructor.getFields()[i].getName())) { + return true; + } + } + var method = findMethod(member); + return method != null; + } + + /** + * All methods are internal, including public methods. Fields of project-private constructor are + * internal as well. + */ + @ExportMessage + final boolean isMemberInternal(String member) { if (hasProjectPrivateConstructor()) { - return false; + return true; } for (int i = 0; i < constructor.getArity(); i++) { if (member.equals(constructor.getFields()[i].getName())) { - return true; + // Fields of public constructor are not internal. + return false; } } - return false; + // All methods are internal. + return true; } /** - * Reads a field of the atom. + * Reads a field or a method. * - * @param member An identifier of a field. - * @return Value of the member. - * @throws UnknownIdentifierException If an unknown field is requested. - * @throws UnsupportedMessageException If the atom constructor is project-private, and thus all - * the fields are project-private. + * @param member An identifier of a field or method. + * @return Value of the field or function. + * @throws UnknownIdentifierException If an unknown field/method is requested. + * @throws UnsupportedMessageException If the requested member is not readable. */ @ExportMessage @ExplodeLoop final Object readMember(String member, @CachedLibrary(limit = "3") StructsLibrary structs) throws UnknownIdentifierException, UnsupportedMessageException { - if (hasProjectPrivateConstructor()) { - throw UnsupportedMessageException.create(); + if (!isMemberReadable(member)) { + throw UnknownIdentifierException.create(member); } for (int i = 0; i < constructor.getArity(); i++) { if (member.equals(constructor.getFields()[i].getName())) { return structs.getField(this, i); } } + var method = findMethod(member); + if (method != null) { + return method; + } throw UnknownIdentifierException.create(member); } - /** Only public (non project-private) methods can be invoked. */ + @TruffleBoundary + private Function findMethod(String methodName) { + var matchedMethod = + getInstanceMethods().stream() + .filter( + method -> { + var nameItems = method.getName().split("\\."); + return nameItems[nameItems.length - 1].equals(methodName); + }) + .findFirst(); + return matchedMethod.orElse(null); + } + + /** + * All members - fields (field getters) and methods can be invoked. Including project-private + * methods. + */ @ExportMessage static class InvokeMember { @@ -238,7 +339,6 @@ static UnresolvedSymbol buildSym(AtomConstructor cons, String name) { guards = { "receiver.getConstructor() == cachedConstructor", "member.equals(cachedMember)", - "!isProjectPrivate(cachedConstructor, cachedMember)" }, limit = "3") static Object doCached( @@ -253,7 +353,6 @@ static Object doCached( ArityException, UnsupportedTypeException, UnknownIdentifierException { - assert !isProjectPrivate(cachedConstructor, cachedMember); Object[] args = new Object[arguments.length + 1]; args[0] = receiver; System.arraycopy(arguments, 0, args, 1, arguments.length); @@ -278,24 +377,10 @@ static Object doUncached( ArityException, UnsupportedTypeException, UnknownIdentifierException { - if (isProjectPrivate(receiver.getConstructor(), member)) { - throw UnsupportedMessageException.create(); - } UnresolvedSymbol symbol = buildSym(receiver.getConstructor(), member); return doCached( receiver, member, arguments, receiver.getConstructor(), member, symbol, symbols); } - - @Idempotent - @TruffleBoundary - protected static boolean isProjectPrivate(AtomConstructor cons, String member) { - Function method = cons.getDefinitionScope().getMethodForType(cons.getType(), member); - if (method != null) { - return method.getSchema().isProjectPrivate(); - } - method = cons.getType().getDefinitionScope().getMethodForType(cons.getType(), member); - return method != null && method.getSchema().isProjectPrivate(); - } } @ExportMessage diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/AtomConstructor.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/AtomConstructor.java index bedd86003cef..9a94a4880ce7 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/AtomConstructor.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/AtomConstructor.java @@ -317,8 +317,11 @@ public static AtomConstructor accessorFor(Function fn) { public static Map collectFieldAccessors(EnsoLanguage language, Type type) { var constructors = type.getConstructors().values(); var roots = new TreeMap(); - if (constructors.size() != 1) { + if (constructors.size() > 1) { var names = new TreeMap>(); + // We assume that all the constructors have the same definition scope. So we + // take just the first one. + var moduleScope = constructors.iterator().next().getDefinitionScope(); for (var cons : constructors) { for (var field : cons.getFields()) { var items = names.computeIfAbsent(field.getName(), (k) -> new ArrayList<>()); @@ -334,9 +337,10 @@ public static Map collectFieldAccessors(EnsoLanguage language, language, name, Type.noType(), + moduleScope, fields.toArray(new GetFieldWithMatchNode.GetterPair[0]))); } - } else { + } else if (constructors.size() == 1) { var cons = constructors.toArray(AtomConstructor[]::new)[0]; for (var field : cons.getFields()) { var node = diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldBaseNode.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldBaseNode.java new file mode 100644 index 000000000000..f5f039f18451 --- /dev/null +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldBaseNode.java @@ -0,0 +1,45 @@ +package org.enso.interpreter.runtime.data.atom; + +import org.enso.compiler.context.LocalScope; +import org.enso.interpreter.EnsoLanguage; +import org.enso.interpreter.node.EnsoRootNode; +import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.runtime.data.Type; +import org.enso.interpreter.runtime.data.text.Text; +import org.enso.interpreter.runtime.error.PanicException; +import org.enso.interpreter.runtime.scope.ModuleScope; + +/** Abstract base class for all nodes that access field(s) of an Atom. */ +abstract class GetFieldBaseNode extends EnsoRootNode { + protected final Type type; + protected final String fieldName; + protected final ModuleScope moduleScope; + + GetFieldBaseNode(EnsoLanguage language, Type type, String fieldName, ModuleScope moduleScope) { + super(language, LocalScope.empty(), moduleScope, fieldName, null); + this.type = type; + this.fieldName = fieldName; + this.moduleScope = moduleScope; + } + + @Override + public String getName() { + return fieldName; + } + + @Override + public String getQualifiedName() { + return type.getQualifiedName().createChild(fieldName).toString(); + } + + protected PanicException noSuchFieldPanic(Atom atom) { + var nameText = Text.create(fieldName); + return new PanicException( + EnsoContext.get(this) + .getBuiltins() + .error() + .getNoSuchFieldError() + .newInstance(atom, nameText), + this); + } +} diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldNode.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldNode.java index dd0696356c7b..e80ba6a86dad 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldNode.java @@ -2,18 +2,16 @@ import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.NodeInfo; -import org.enso.compiler.context.LocalScope; import org.enso.interpreter.EnsoLanguage; -import org.enso.interpreter.node.EnsoRootNode; import org.enso.interpreter.runtime.callable.function.Function; import org.enso.interpreter.runtime.data.Type; import org.enso.interpreter.runtime.scope.ModuleScope; -@NodeInfo(shortName = "get_field", description = "A base for auto-generated Atom getters.") -final class GetFieldNode extends EnsoRootNode { +@NodeInfo( + shortName = "get_field", + description = "Returns a single field from an Atom based on the given index.") +final class GetFieldNode extends GetFieldBaseNode { private final int index; - private final String name; - private final Type type; private @Child StructsLibrary structs = StructsLibrary.getFactory().createDispatched(10); @@ -24,10 +22,8 @@ final class GetFieldNode extends EnsoRootNode { * @param index the index this node should use for field lookup. */ GetFieldNode(EnsoLanguage language, int index, Type type, String name, ModuleScope moduleScope) { - super(language, LocalScope.empty(), moduleScope, name, null); + super(language, type, name, moduleScope); this.index = index; - this.type = type; - this.name = name; } /** @@ -43,16 +39,6 @@ public Object execute(VirtualFrame frame) { return structs.getField(atom, index); } - @Override - public String getQualifiedName() { - return type.getQualifiedName().createChild(name).toString(); - } - - @Override - public String getName() { - return name; - } - @Override public boolean isCloningAllowed() { return true; @@ -65,6 +51,7 @@ protected boolean isCloneUninitializedSupported() { @Override protected GetFieldNode cloneUninitialized() { - return new GetFieldNode(getLanguage(EnsoLanguage.class), index, type, name, getModuleScope()); + return new GetFieldNode( + getLanguage(EnsoLanguage.class), index, type, fieldName, getModuleScope()); } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldWithMatchNode.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldWithMatchNode.java index 7d12eb45beaf..1fad5bd73d18 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldWithMatchNode.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/data/atom/GetFieldWithMatchNode.java @@ -1,55 +1,31 @@ package org.enso.interpreter.runtime.data.atom; import com.oracle.truffle.api.CompilerDirectives; -import com.oracle.truffle.api.TruffleLanguage; import com.oracle.truffle.api.frame.VirtualFrame; import com.oracle.truffle.api.nodes.ExplodeLoop; import com.oracle.truffle.api.nodes.NodeInfo; -import com.oracle.truffle.api.nodes.RootNode; -import org.enso.interpreter.runtime.EnsoContext; +import org.enso.interpreter.EnsoLanguage; import org.enso.interpreter.runtime.callable.function.Function; import org.enso.interpreter.runtime.data.Type; -import org.enso.interpreter.runtime.data.text.Text; -import org.enso.interpreter.runtime.error.PanicException; +import org.enso.interpreter.runtime.scope.ModuleScope; -@NodeInfo(shortName = "get_field", description = "A base for auto-generated Atom getters.") -final class GetFieldWithMatchNode extends RootNode { - static class GetterPair { - private final AtomConstructor target; - private final int index; +@NodeInfo( + shortName = "get_field_with_match", + description = "Finds an atom field in multiple constructors") +final class GetFieldWithMatchNode extends GetFieldBaseNode { - public GetterPair(AtomConstructor target, int index) { - this.target = target; - this.index = index; - } - - public AtomConstructor getTarget() { - return target; - } - - public int getIndex() { - return index; - } - } + record GetterPair(AtomConstructor target, int index) {} - private final String name; - private final Text nameText; - private final Type type; private final @CompilerDirectives.CompilationFinal(dimensions = 1) GetterPair[] getterPairs; private @Children StructsLibrary[] structsLibraries; - /** - * Creates a new instance of this node. - * - * @param language the current language instance. - * @param index the index this node should use for field lookup. - */ public GetFieldWithMatchNode( - TruffleLanguage language, String name, Type type, GetterPair[] getterPairs) { - super(language); - this.name = name; - this.type = type; - this.nameText = Text.create(name); + EnsoLanguage language, + String name, + Type type, + ModuleScope moduleScope, + GetterPair[] getterPairs) { + super(language, type, name, moduleScope); this.getterPairs = getterPairs; this.structsLibraries = new StructsLibrary[getterPairs.length]; for (int i = 0; i < getterPairs.length; i++) { @@ -68,22 +44,6 @@ public Object execute(VirtualFrame frame) { return structsLibraries[i].getField(atom, getter.index); } } - throw new PanicException( - EnsoContext.get(this) - .getBuiltins() - .error() - .getNoSuchFieldError() - .newInstance(atom, nameText), - this); - } - - @Override - public String getQualifiedName() { - return type.getQualifiedName().createChild(name).toString(); - } - - @Override - public String getName() { - return type.getName() + "." + name; + throw noSuchFieldPanic(atom); } } diff --git a/test/Base_Tests/src/Semantic/Private_Spec.enso b/test/Base_Tests/src/Semantic/Private_Spec.enso index bbe3d0f19f3e..0d141ad4b7cd 100644 --- a/test/Base_Tests/src/Semantic/Private_Spec.enso +++ b/test/Base_Tests/src/Semantic/Private_Spec.enso @@ -41,12 +41,10 @@ add_specs spec_builder = obj = Priv_Type.create 42 obj.get_data . should_equal 42 - group_builder.specify "cannot get private field from JS" pending=pending_js_missing <| + group_builder.specify "can get private field from JS" pending=pending_js_missing <| obj = Priv_Type.create 42 - # When JS tries to access a private field, it does not throw a panic, as it does not - # see the field. It returns undefined. ret = js_access_field obj - ret.is_nothing . should_be_true + ret.should_equal 42 group_builder.specify "can call private constructor via public factory method from JS" pending=pending_js_missing <| create_fn x = Priv_Type.create x