Skip to content

Commit

Permalink
Create static wrappers for builtin types
Browse files Browse the repository at this point in the history
#3764 introduced static wrappers
for instance methods. Except it had a limitation to only be allowed for
types with at least a single constructor.
That excluded builtin types as well which, by default, don't have them.
This limitation is problematic for Array/Vector consolidation and makes
builtin types somehow second-citizens.

This change lifts the limitation for builtin types only. Note that we do
not want to share the implementatin of the generated builtin methods.
At the same time due to the additional argument we have to adjust the
starting index of the arguments.
This change avoids messing with the existing dispatch logic, to avoid
unnecessary complexity.

As a result it is now possible to call builtin types' instance methods,
statically:
```
  arr = Array.new_1 42
  Array.length arr
```
That would previously lead to missing method exception in runtime.
The only exception is `Nothing`. Primarily because it requires `Nothing`
to have a proper eigentype (`Nothing.type`) which would messed up a lot
of existing logic.
  • Loading branch information
hubertp committed Jan 26, 2023
1 parent 1097c41 commit 5ed8df8
Show file tree
Hide file tree
Showing 10 changed files with 177 additions and 63 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ public Class<? extends Builtin> getSuperType() {

@Override
protected boolean containsValues() {
return false;
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -175,8 +175,13 @@ private void registerBuiltinMethods(ModuleScope scope, EnsoLanguage language) {
methods.entrySet().stream().forEach(entry -> {
Type tpe = entry.getValue().isAutoRegister ? (!entry.getValue().isStatic() ? type : type.getEigentype()) : null;
if (tpe != null) {
Optional<BuiltinFunction> fun = entry.getValue().toFunction(language);
LoadedBuiltinMethod value = entry.getValue();
Optional<BuiltinFunction> fun = value.toFunction(language, false);
fun.ifPresent(f -> scope.registerMethod(tpe, entry.getKey(), f.getFunction()));
if (!value.isStatic()) {
Optional<BuiltinFunction> fun2 = value.toFunction(language, true);
fun2.ifPresent(f -> scope.registerMethod(tpe, entry.getKey() + "_static", f.getFunction()));
}
}
});
}
Expand Down Expand Up @@ -360,7 +365,7 @@ private static Map<String, LoadedBuiltinMethod> readBuiltinMethodsMethods() {
@SuppressWarnings("unchecked")
Class<BuiltinRootNode> clazz =
(Class<BuiltinRootNode>) Class.forName(builtinMeta[1]);
Method meth = clazz.getMethod("makeFunction", EnsoLanguage.class);
Method meth = clazz.getMethod("makeFunction", EnsoLanguage.class, boolean.class);
LoadedBuiltinMethod meta = new LoadedBuiltinMethod(meth, isStatic, isAutoRegister);
return new AbstractMap.SimpleEntry<String, LoadedBuiltinMethod>(builtinMeta[0], meta);
} catch (ClassNotFoundException | NoSuchMethodException e) {
Expand All @@ -380,17 +385,17 @@ private static Map<String, LoadedBuiltinMethod> readBuiltinMethodsMethods() {
* @return A non-empty function under the given name, if it exists. An empty value if no such
* builtin method was ever registerd
*/
public Optional<BuiltinFunction> getBuiltinFunction(String type, String methodName, EnsoLanguage language) {
public Optional<BuiltinFunction> getBuiltinFunction(String type, String methodName, EnsoLanguage language, boolean isStaticInstance) {
// TODO: move away from String mapping once Builtins is gone
Map<String, LoadedBuiltinMethod> atomNodes = builtinMethodNodes.get(type);
if (atomNodes == null) return Optional.empty();
LoadedBuiltinMethod builtin = atomNodes.get(methodName);
if (builtin == null) return Optional.empty();
return builtin.toFunction(language);
return builtin.toFunction(language, isStaticInstance);
}

public Optional<BuiltinFunction> getBuiltinFunction(Type type, String methodName, EnsoLanguage language) {
return getBuiltinFunction(type.getName(), methodName, language);
return getBuiltinFunction(type.getName(), methodName, language, false);
}

public <T extends Builtin> T getBuiltinType(Class<T> clazz) {
Expand Down Expand Up @@ -618,9 +623,9 @@ public Type fromTypeSystem(String typeName) {
}

private record LoadedBuiltinMethod(Method meth, boolean isStatic, boolean isAutoRegister) {
Optional<BuiltinFunction> toFunction(EnsoLanguage language) {
Optional<BuiltinFunction> toFunction(EnsoLanguage language, boolean isStaticInstance) {
try {
return Optional.ofNullable((Function) meth.invoke(null, language)).map(f-> new BuiltinFunction(f, isAutoRegister));
return Optional.ofNullable((Function) meth.invoke(null, language, isStaticInstance)).map(f-> new BuiltinFunction(f, isAutoRegister));
} catch (Exception e) {
e.printStackTrace();
return Optional.empty();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,14 +378,19 @@ class IrToTruffle(
)

val function = methodDef.body match {
case fn: IR.Function if isBuiltinMethod(fn.body) =>
case fn: IR.Function
if isBuiltinMethod(
fn.body
) =>
// For builtin types that own the builtin method we only check that
// the method has been registered during the initialization of builtins
// and not attempt to register it in the scope (can't redefined methods).
// For non-builtin types (or modules) that own the builtin method
// we have to look up the function and register it in the scope.
val x = methodDef.body.asInstanceOf[IR.Function.Lambda].body
val fullMethodName = x.asInstanceOf[IR.Literal.Text]
val fullMethodName = methodDef.body
.asInstanceOf[IR.Function.Lambda]
.body
.asInstanceOf[IR.Literal.Text]

val builtinNameElements = fullMethodName.text.split('.')
if (builtinNameElements.length != 2) {
Expand All @@ -396,8 +401,15 @@ class IrToTruffle(
val methodName = builtinNameElements(1)
val methodOwnerName = builtinNameElements(0)

val staticWrapper = methodDef.isStaticWrapperForInstanceMethod

val builtinFunction = context.getBuiltins
.getBuiltinFunction(methodOwnerName, methodName, language)
.getBuiltinFunction(
methodOwnerName,
methodName,
language,
staticWrapper
)
builtinFunction.toScala
.map(Some(_))
.toRight(
Expand All @@ -406,18 +418,41 @@ class IrToTruffle(
)
)
.left
.flatMap(l =>
.flatMap { l =>
// Builtin Types Number and Integer have methods only for documentation purposes
if (
cons == context.getBuiltins.number().getNumber ||
cons == context.getBuiltins.number().getInteger
) Right(None)
val number = context.getBuiltins.number()
val ok =
staticWrapper && (cons == number.getNumber.getEigentype || cons == number.getInteger.getEigentype) ||
!staticWrapper && (cons == number.getNumber || cons == number.getInteger)
if (ok) Right(None)
else Left(l)
)
}
.map(fOpt =>
// Register builtin iff it has not been automatically registered at an early stage
// of builtins initialization.
fOpt.filter(m => !m.isAutoRegister).map(m => m.getFunction)
// of builtins initialization or if it is a static wrapper.
fOpt
.filter(m => !m.isAutoRegister() || staticWrapper)
.map { m =>
if (staticWrapper) {
// Static wrappers differ in the number of arguments by 1.
// Therefore we cannot simply get the registered function.
// BuiltinRootNode.execute will infer the right order of arguments.
val bodyBuilder =
new expressionProcessor.BuildFunctionBody(
fn.arguments,
fn.body,
effectContext,
true
)
new RuntimeFunction(
m.getFunction.getCallTarget,
null,
new FunctionSchema(new Array[RuntimeAnnotation](0), bodyBuilder.args(): _*)
)
} else {
m.getFunction
}
}
)
case fn: IR.Function =>
val bodyBuilder =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import org.enso.compiler.context.{InlineContext, ModuleContext}
import org.enso.compiler.core.IR
import org.enso.compiler.core.ir.MetadataStorage.ToPair
import org.enso.compiler.data.BindingsMap
import org.enso.compiler.data.BindingsMap.{Resolution, ResolvedType}
import org.enso.compiler.data.BindingsMap.{Resolution, ResolvedType, Type}
import org.enso.compiler.exception.CompilerError
import org.enso.compiler.pass.IRPass
import org.enso.compiler.pass.analyse.BindingAnalysis
Expand All @@ -16,7 +16,7 @@ import org.enso.compiler.pass.desugar.{

import scala.annotation.unused

/** Resolves the correct `this` argument type for method definitions and stores
/** Resolves the correct `self` argument type for method definitions and stores
* the resolution in the method's metadata.
*/
case object MethodDefinitions extends IRPass {
Expand Down Expand Up @@ -91,7 +91,8 @@ case object MethodDefinitions extends IRPass {
method.methodReference.typePointer.flatMap(
_.getMetadata(this)
) match {
case Some(Resolution(ResolvedType(_, tp))) if tp.members.nonEmpty =>
case Some(Resolution(ResolvedType(_, tp)))
if canGenerateStaticWrappers(tp) =>
val dup = method.duplicate()
val static = dup.copy(body =
IR.Function.Lambda(
Expand All @@ -110,7 +111,8 @@ case object MethodDefinitions extends IRPass {
)
)
List(method, static)
case _ => List(method)
case _ =>
List(method)
}

case other => List(other)
Expand All @@ -119,6 +121,14 @@ case object MethodDefinitions extends IRPass {
ir.copy(bindings = withStaticAliases)
}

// Generate static wrappers for
// 1. Types having at least one type constructor
// 2. All builtin types except for Nothing. Nothing's eigentype is Nothing and not Nothing.type,
// would lead to overriding conflicts.
// TODO: Remvoe the hardcoded type once Enso's annotations can define parameters.
private def canGenerateStaticWrappers(tp: Type): Boolean =
tp.members.nonEmpty || (tp.builtinType && (tp.name != "Nothing"))

private def resolveType(
typePointer: IR.Name,
availableSymbolsMap: BindingsMap
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,24 @@ class MethodsTest extends InterpreterTest {
)
}

"be callable on builtin types when non-static, with additional self arg" in {
val code =
"""import Standard.Base.IO
|import Standard.Base.Data.Array.Array
|import Standard.Base.Data.Text.Text
|
|main =
| a = Array.new_1 1
| t_1 = "foo"
| t_2 = "bar"
|
| IO.println (Array.length a)
| IO.println (Text.+ t_1 t_2)
|""".stripMargin
eval(code)
consumeOut.shouldEqual(List("1", "foobar"))
}

"not be callable on instances when static" in {
val code =
"""
Expand All @@ -200,5 +218,18 @@ class MethodsTest extends InterpreterTest {
code
) should have message "Method `new` of Mk_Foo could not be found."
}

"not be callable on Nothing when non-static" in {
val code =
"""
|import Standard.Base.Nothing.Nothing
|
|main = Nothing.is_nothing Nothing
|""".stripMargin
the[InterpreterException] thrownBy eval(
code
) should have message "Type error: expected a function, but got True."
}

}
}
Loading

0 comments on commit 5ed8df8

Please sign in to comment.