Skip to content

Commit 7c783ca

Browse files
Simplify MethodResolver configuration (#14030)
Rather than trying to lookup module, type and method by name. Pass the module and type instance into the `MethodResolver` and use them directly.
1 parent 72a7ada commit 7c783ca

File tree

4 files changed

+66
-17
lines changed

4 files changed

+66
-17
lines changed

distribution/lib/Standard/Table/0.0.0-dev/src/Expression.enso

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,18 @@ from Standard.Base import all
22
import Standard.Base.Errors.Common.Type_Error
33
from Standard.Base.Metadata import Widget
44

5+
import project.Internal.Expression_Statics as Expression_Statics_Module
6+
import project.Internal.Expression_Statics.Expression_Statics
7+
import project.Column as Column_Module
58
import project.Column.Column
9+
import project.Refined_Types.Numeric_Column as Numeric_Column_Module
610
import project.Refined_Types.Numeric_Column.Numeric_Column
711
import project.Table.Table
812

913
polyglot java import java.lang.IllegalArgumentException
1014
polyglot java import java.lang.UnsupportedOperationException
1115
polyglot java import org.enso.table.expressions.ExpressionVisitorImpl
16+
polyglot java import org.enso.table.expressions.ExpressionVisitorImpl.MethodResolver
1217
polyglot java import org.enso.table.expressions.ExpressionVisitorImpl.SyntaxErrorException
1318
polyglot java import org.enso.table.expressions.ExpressionVisitorImpl.TypeErrorException
1419

@@ -50,9 +55,9 @@ type Expression
5055
## Helper for the expression to tell it which functions needs a Vector
5156
var_args_functions = ['is_in', 'coalesce', 'min', 'max']
5257
## Define which enso types we will look in to resolve methods in the expression
53-
mr1 = ExpressionVisitorImpl.newMethodResolver "Standard.Table.Internal.Expression_Statics" "Expression_Statics" isStaticMethod=True makeTypedColumn=Nothing
54-
mr2 = ExpressionVisitorImpl.newMethodResolver "Standard.Table.Column" "Column" isStaticMethod=False makeTypedColumn=c->c
55-
mr3 = ExpressionVisitorImpl.newMethodResolver "Standard.Table.Refined_Types.Numeric_Column" "Numeric_Column" isStaticMethod=False makeTypedColumn=c->(c : Numeric_Column)
58+
mr1 = ExpressionVisitorImpl.newMethodResolver Expression_Statics_Module Expression_Statics isStaticMethod=True makeTypedColumn=Nothing
59+
mr2 = ExpressionVisitorImpl.newMethodResolver Column_Module Column isStaticMethod=False makeTypedColumn=c->c
60+
mr3 = ExpressionVisitorImpl.newMethodResolver Numeric_Column_Module Numeric_Column isStaticMethod=False makeTypedColumn=c->(c : Numeric_Column)
5661
method_resolvers = [mr1, mr2, mr3]
5762
handle_parse_error = Panic.catch SyntaxErrorException handler=(cause-> Error.throw (Expression_Error.Syntax_Error cause.payload.getMessage cause.payload.getLine cause.payload.getColumn))
5863
handle_unsupported = handle_java_error UnsupportedOperationException Expression_Error.Unsupported_Operation

engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/interop/AtomInteropTest.java

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import static org.hamcrest.Matchers.not;
1111
import static org.hamcrest.Matchers.notNullValue;
1212
import static org.junit.Assert.assertEquals;
13+
import static org.junit.Assert.assertNotNull;
1314
import static org.junit.Assert.assertSame;
1415
import static org.junit.Assert.assertTrue;
1516

@@ -173,6 +174,46 @@ public void fieldsFromPrivateConstructorAreInternalMembers() {
173174
assertThat("field a is internal", interop.isMemberInternal(atom, "a"), is(true));
174175
}
175176

177+
@Test
178+
public void methodsAreVisibleEvenWithPrivateConstructors() throws Exception {
179+
var myTypeAtom =
180+
ctxRule.evalModule(
181+
"""
182+
type My_Type
183+
private Cons a
184+
185+
read self = self.a
186+
187+
main = My_Type.Cons "a"
188+
""");
189+
var atom = ctxRule.unwrapValue(myTypeAtom);
190+
var interop = InteropLibrary.getUncached();
191+
var read = interop.readMember(atom, "read");
192+
assertNotNull("Found read member", read);
193+
assertEquals("a", read.toString());
194+
}
195+
196+
@Test
197+
public void methodsAreVisibleOnTypeEvenWithPrivateConstructors() throws Exception {
198+
var atom =
199+
ctxRule.evalModule(
200+
"""
201+
type My_Type
202+
private Cons a
203+
204+
read self = self.a
205+
206+
main = My_Type.Cons "a"
207+
""");
208+
var type = ctxRule.unwrapValue(atom.getMetaObject());
209+
var rawAtom = ctxRule.unwrapValue(atom);
210+
var interop = InteropLibrary.getUncached();
211+
var read = interop.readMember(type, "read");
212+
assertNotNull("Found read member", read);
213+
assertTrue("Can read", interop.isExecutable(read));
214+
assertEquals("a", interop.execute(read, rawAtom).toString());
215+
}
216+
176217
@Test
177218
public void fieldFromPrivateConstructorIsReadable()
178219
throws UnsupportedMessageException,

engine/runtime/src/main/java/org/enso/interpreter/runtime/data/Type.java

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -403,10 +403,13 @@ EnsoObject getMembers(boolean includeInternal) {
403403
@ExportMessage
404404
@CompilerDirectives.TruffleBoundary
405405
boolean isMemberReadable(String member) {
406+
if (methods().containsKey(member)) {
407+
return true;
408+
}
406409
if (hasAllConstructorsPrivate) {
407410
return false;
408411
} else {
409-
return constructors.containsKey(member) || methods().containsKey(member);
412+
return constructors.containsKey(member);
410413
}
411414
}
412415

@@ -493,12 +496,11 @@ static InvokeCallableNode buildInvokeCallableNode(Function func) {
493496
@ExportMessage
494497
@CompilerDirectives.TruffleBoundary
495498
Object readMember(String member) throws UnknownIdentifierException {
496-
if (hasAllConstructorsPrivate) {
497-
throw UnknownIdentifierException.create(member);
498-
}
499-
var cons = constructors.get(member);
500-
if (cons != null) {
501-
return cons;
499+
if (!hasAllConstructorsPrivate) {
500+
var cons = constructors.get(member);
501+
if (cons != null) {
502+
return cons;
503+
}
502504
}
503505
var method = methods().get(member);
504506
if (method != null) {

std-bits/table/src/main/java/org/enso/table/expressions/ExpressionVisitorImpl.java

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,14 @@ public interface MethodInterface {
6767

6868
public record MethodResolver(
6969
Value module, Value type, boolean isStaticMethod, Function<Object, Value> makeTypedColumn) {
70-
public MethodResolver(
70+
private MethodResolver(
71+
Context ctx,
7172
String moduleName,
7273
String typeName,
7374
boolean isStaticMethod,
7475
Function<Object, Value> makeTypedColumn) {
7576
this(
76-
Context.getCurrent().getBindings("enso").invokeMember("get_module", moduleName),
77+
ctx.getBindings("enso").invokeMember("get_module", moduleName),
7778
typeName,
7879
isStaticMethod,
7980
makeTypedColumn);
@@ -97,11 +98,11 @@ public Value resolve(String methodName) {
9798
}
9899

99100
public static MethodResolver newMethodResolver(
100-
String moduleName,
101-
String typeName,
102-
boolean isStaticMethod,
103-
Function<Object, Value> makeTypedColumn) {
104-
return new MethodResolver(moduleName, typeName, isStaticMethod, makeTypedColumn);
101+
Value module, Value type, boolean isStaticMethod, Function<Object, Value> makeTypedColumn) {
102+
var moduleName = module.getMetaQualifiedName();
103+
var typeName = type.getMetaSimpleName();
104+
return new MethodResolver(
105+
module.getContext(), moduleName, typeName, isStaticMethod, makeTypedColumn);
105106
}
106107

107108
public static class Method implements MethodInterface {

0 commit comments

Comments
 (0)