From 5ed8df8a415e2c0fda57a5c20186e20b347ca8cd Mon Sep 17 00:00:00 2001 From: Hubert Plociniczak Date: Tue, 24 Jan 2023 14:37:21 +0100 Subject: [PATCH] Create static wrappers for builtin types https://github.com/enso-org/enso/pull/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. --- .../node/expression/builtin/Any.java | 2 +- .../interpreter/runtime/builtin/Builtins.java | 19 ++-- .../enso/compiler/codegen/IrToTruffle.scala | 59 +++++++--- .../pass/resolve/MethodDefinitions.scala | 18 +++- .../test/semantic/MethodsTest.scala | 31 ++++++ .../enso/interpreter/dsl/MethodProcessor.java | 102 +++++++++++------- test/Tests/src/Semantic/Error_Spec.enso | 2 +- .../Tests/src/Semantic/Java_Interop_Spec.enso | 2 +- test/Tests/src/Semantic/Warnings_Spec.enso | 2 +- .../Standard/Base/0.0.0-dev/src/Nothing.enso | 3 + 10 files changed, 177 insertions(+), 63 deletions(-) diff --git a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/Any.java b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/Any.java index 29be8f201748f..693dbe290223a 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/Any.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/node/expression/builtin/Any.java @@ -11,6 +11,6 @@ public Class getSuperType() { @Override protected boolean containsValues() { - return false; + return true; } } diff --git a/engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Builtins.java b/engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Builtins.java index 302055d683446..2d8e24721e335 100644 --- a/engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Builtins.java +++ b/engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Builtins.java @@ -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 fun = entry.getValue().toFunction(language); + LoadedBuiltinMethod value = entry.getValue(); + Optional fun = value.toFunction(language, false); fun.ifPresent(f -> scope.registerMethod(tpe, entry.getKey(), f.getFunction())); + if (!value.isStatic()) { + Optional fun2 = value.toFunction(language, true); + fun2.ifPresent(f -> scope.registerMethod(tpe, entry.getKey() + "_static", f.getFunction())); + } } }); } @@ -360,7 +365,7 @@ private static Map readBuiltinMethodsMethods() { @SuppressWarnings("unchecked") Class clazz = (Class) 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(builtinMeta[0], meta); } catch (ClassNotFoundException | NoSuchMethodException e) { @@ -380,17 +385,17 @@ private static Map 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 getBuiltinFunction(String type, String methodName, EnsoLanguage language) { + public Optional getBuiltinFunction(String type, String methodName, EnsoLanguage language, boolean isStaticInstance) { // TODO: move away from String mapping once Builtins is gone Map 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 getBuiltinFunction(Type type, String methodName, EnsoLanguage language) { - return getBuiltinFunction(type.getName(), methodName, language); + return getBuiltinFunction(type.getName(), methodName, language, false); } public T getBuiltinType(Class clazz) { @@ -618,9 +623,9 @@ public Type fromTypeSystem(String typeName) { } private record LoadedBuiltinMethod(Method meth, boolean isStatic, boolean isAutoRegister) { - Optional toFunction(EnsoLanguage language) { + Optional 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(); diff --git a/engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala b/engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala index 56ce822ea5c14..8342b4994fd1a 100644 --- a/engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala +++ b/engine/runtime/src/main/scala/org/enso/compiler/codegen/IrToTruffle.scala @@ -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) { @@ -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( @@ -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 = diff --git a/engine/runtime/src/main/scala/org/enso/compiler/pass/resolve/MethodDefinitions.scala b/engine/runtime/src/main/scala/org/enso/compiler/pass/resolve/MethodDefinitions.scala index 96d98fd1a124e..bde81c963bfaa 100644 --- a/engine/runtime/src/main/scala/org/enso/compiler/pass/resolve/MethodDefinitions.scala +++ b/engine/runtime/src/main/scala/org/enso/compiler/pass/resolve/MethodDefinitions.scala @@ -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 @@ -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 { @@ -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( @@ -110,7 +111,8 @@ case object MethodDefinitions extends IRPass { ) ) List(method, static) - case _ => List(method) + case _ => + List(method) } case other => List(other) @@ -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 diff --git a/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/MethodsTest.scala b/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/MethodsTest.scala index 39ca6e5511efa..833047be0cfc7 100644 --- a/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/MethodsTest.scala +++ b/engine/runtime/src/test/scala/org/enso/interpreter/test/semantic/MethodsTest.scala @@ -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 = """ @@ -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." + } + } } diff --git a/lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/MethodProcessor.java b/lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/MethodProcessor.java index 248856c16b0b2..34ad2cfff8050 100644 --- a/lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/MethodProcessor.java +++ b/lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/MethodProcessor.java @@ -159,6 +159,7 @@ private void generateCode(MethodDefinition methodDefinition) throws IOException out.println(" description = \"\"\"\n" + methodDefinition.getDescription() + "\"\"\")"); out.println("public class " + methodDefinition.getClassName() + " extends BuiltinRootNode {"); out.println(" private @Child " + methodDefinition.getOriginalClassName() + " bodyNode;"); + out.println(" private final boolean staticOfInstanceMethod;"); out.println(); @@ -184,9 +185,10 @@ private void generateCode(MethodDefinition methodDefinition) throws IOException } out.println(" private final BranchProfile anyWarningsProfile = BranchProfile.create();"); - out.println(" private " + methodDefinition.getClassName() + "(EnsoLanguage language) {"); + out.println(" private " + methodDefinition.getClassName() + "(EnsoLanguage language, boolean staticOfInstanceMethod) {"); out.println(" super(language);"); - out.println(" bodyNode = " + methodDefinition.getConstructorExpression() + ";"); + out.println(" this.bodyNode = " + methodDefinition.getConstructorExpression() + ";"); + out.println(" this.staticOfInstanceMethod = staticOfInstanceMethod;"); out.println(" }"); out.println(); @@ -197,29 +199,27 @@ private void generateCode(MethodDefinition methodDefinition) throws IOException : "fromBuiltinRootNode"; out.println(" public static Function makeFunction(EnsoLanguage language) {"); - out.println(" return Function." + functionBuilderMethod + "("); - out.println( - " new " - + methodDefinition.getClassName() - + "(language)"); - List argumentDefs = new ArrayList<>(); - for (MethodDefinition.ArgumentDefinition arg : methodDefinition.getArguments()) { - if (arg.isPositional()) { - String executionMode = arg.isSuspended() ? "PASS_THUNK" : "EXECUTE"; - argumentDefs.add( - " new ArgumentDefinition(" - + arg.getPosition() - + ", \"" - + arg.getName() - + "\", ArgumentDefinition.ExecutionMode." - + executionMode - + ")"); - } + out.println(" return makeFunction(language, false);"); + out.println(" }"); + out.println(); + out.println(" public static Function makeFunction(EnsoLanguage language, boolean staticOfInstanceMethod) {"); + out.println(" if (staticOfInstanceMethod) {"); + out.println(" return Function." + functionBuilderMethod + "("); + out.print(" new " + methodDefinition.getClassName() + "(language, staticOfInstanceMethod)"); + List argsStaticInstace = generateMakeFunctionArgs(true, methodDefinition.getArguments()); + if (!argsStaticInstace.isEmpty()) { + out.println(","); } - if (!argumentDefs.isEmpty()) { + out.println(String.join(",\n", argsStaticInstace) + ");"); + out.println(" } else {"); + out.println(" return Function." + functionBuilderMethod + "("); + out.print(" new " + methodDefinition.getClassName() + "(language, staticOfInstanceMethod)"); + List argsInstance = generateMakeFunctionArgs(false, methodDefinition.getArguments()); + if (!argsInstance.isEmpty()) { out.println(","); } - out.println(String.join(",\n", argumentDefs) + ");"); + out.println(String.join(",\n", argsInstance) + ");"); + out.println(" }"); out.println(" }"); out.println(); @@ -233,7 +233,14 @@ private void generateCode(MethodDefinition methodDefinition) throws IOException } out.println( " Object[] arguments = Function.ArgumentsHelper.getPositionalArguments(frame.getArguments());"); + out.println(" int prefix = this.staticOfInstanceMethod ? 1 : 0;"); List callArgNames = new ArrayList<>(); + for (MethodDefinition.ArgumentDefinition arg : + methodDefinition.getArguments()) { + if (!(arg.isImplicit() || arg.isFrame() || arg.isState() || arg.isCallerInfo())) { + out.println(" int arg" + arg.getPosition() + "Idx = " + arg.getPosition() + " + prefix;"); + } + } boolean warningsPossible = generateWarningsCheck(out, methodDefinition.getArguments(), "arguments"); for (MethodDefinition.ArgumentDefinition argumentDefinition : @@ -289,7 +296,7 @@ private void generateCode(MethodDefinition methodDefinition) throws IOException out.println(" @Override"); out.println(" protected RootNode cloneUninitialized() {"); - out.println(" return new " + methodDefinition.getClassName() + "(EnsoLanguage.get(this));"); + out.println(" return new " + methodDefinition.getClassName() + "(EnsoLanguage.get(this), this.staticOfInstanceMethod);"); out.println(" }"); out.println(); @@ -298,9 +305,32 @@ private void generateCode(MethodDefinition methodDefinition) throws IOException } } + private List generateMakeFunctionArgs(boolean staticInstance, List args) { + List argumentDefs = new ArrayList<>(); + int staticPrefix = 0; + if (staticInstance) { + argumentDefs.add(" new ArgumentDefinition(0, \"selfStatic\", ArgumentDefinition.ExecutionMode.EXECUTE)"); + staticPrefix = 1; + } + for (MethodDefinition.ArgumentDefinition arg : args) { + if (arg.isPositional()) { + String executionMode = arg.isSuspended() ? "PASS_THUNK" : "EXECUTE"; + argumentDefs.add( + " new ArgumentDefinition(" + + (staticPrefix + arg.getPosition()) + + ", \"" + + arg.getName() + + "\", ArgumentDefinition.ExecutionMode." + + executionMode + + ")"); + } + } + return argumentDefs; + } + private void generateArgumentRead( PrintWriter out, MethodDefinition.ArgumentDefinition arg, String argsArray) { - String argReference = argsArray + "[" + arg.getPosition() + "]"; + String argReference = argsArray + "[arg" + arg.getPosition() + "Idx]"; if (arg.shouldCheckErrors()) { String condProfile = mkArgumentInternalVarName(arg) + DATAFLOW_ERROR_PROFILE; out.println( @@ -350,9 +380,9 @@ private void generateUncastedArgumentRead( + varName + " = " + argsArray - + "[" + + "[arg" + arg.getPosition() - + "];"); + + "Idx];"); } private void generateUncheckedArgumentRead( @@ -368,9 +398,9 @@ private void generateUncheckedArgumentRead( + castName + "(" + argsArray - + "[" + + "[arg" + arg.getPosition() - + "]);"); + + "Idx]);"); } private void generateUncheckedArrayCast( @@ -386,9 +416,9 @@ private void generateUncheckedArrayCast( + castName + ")" + argsArray - + "[" + + "[arg" + arg.getPosition() - + "];"); + + "Idx];"); } private void generateCheckedArgumentRead( @@ -398,18 +428,18 @@ private void generateCheckedArgumentRead( out.println(" " + arg.getTypeName() + " " + varName + ";"); out.println(" try {"); out.println( - " " + varName + " = " + castName + "(" + argsArray + "[" + arg.getPosition() + "]);"); + " " + varName + " = " + castName + "(" + argsArray + "[arg" + arg.getPosition() + "Idx]);"); out.println(" } catch (UnexpectedResultException e) {"); out.println(" com.oracle.truffle.api.CompilerDirectives.transferToInterpreter();"); out.println(" var builtins = EnsoContext.get(this).getBuiltins();"); out.println( - " var expected = builtins.fromTypeSystem(TypesGen.getName(arguments[" + " var expected = builtins.fromTypeSystem(TypesGen.getName(arguments[arg" + arg.getPosition() - + "]));"); + + "Idx]));"); out.println( - " var error = builtins.error().makeTypeError(expected, arguments[" + " var error = builtins.error().makeTypeError(expected, arguments[arg" + arg.getPosition() - + "], \"" + + "Idx], \"" + varName + "\");"); out.println(" throw new PanicException(error,this);"); @@ -500,7 +530,7 @@ private String mkArgumentInternalVarName(MethodDefinition.ArgumentDefinition arg } private String arrayRead(String array, int index) { - return array + "[" + index + "]"; + return array + "[arg" + index + "Idx]"; } private String capitalize(String name) { diff --git a/test/Tests/src/Semantic/Error_Spec.enso b/test/Tests/src/Semantic/Error_Spec.enso index 2bfd9835e5143..ef926c3112262 100644 --- a/test/Tests/src/Semantic/Error_Spec.enso +++ b/test/Tests/src/Semantic/Error_Spec.enso @@ -66,7 +66,7 @@ spec = Test.specify "should implement to_text" <| Error.throw Nothing . to_text . should_equal "(Error: Nothing)" - Error.to_text . should_equal "Error" + Error.to_text Error . should_equal "Error" Test.specify "should be able to be mapped" <| error = Error.throw 42 diff --git a/test/Tests/src/Semantic/Java_Interop_Spec.enso b/test/Tests/src/Semantic/Java_Interop_Spec.enso index d6d9c1b31d94c..21855c753dfa2 100644 --- a/test/Tests/src/Semantic/Java_Interop_Spec.enso +++ b/test/Tests/src/Semantic/Java_Interop_Spec.enso @@ -13,7 +13,7 @@ polyglot java import java.util.ArrayList polyglot java import java.time.LocalDate polyglot java import java.time.LocalTime -Any.test_me x = x.is_nothing +Any.test_me self x = x.is_nothing spec = Test.group "Java FFI" <| diff --git a/test/Tests/src/Semantic/Warnings_Spec.enso b/test/Tests/src/Semantic/Warnings_Spec.enso index f1ff12a60d81b..d3db157b692c5 100644 --- a/test/Tests/src/Semantic/Warnings_Spec.enso +++ b/test/Tests/src/Semantic/Warnings_Spec.enso @@ -65,7 +65,7 @@ map_odd_warnings_and_errors value = throw_a_bar = Panic.throw "bar" -Any.is_static_nothing x = x.is_nothing +Any.is_static_nothing self x = x.is_nothing spec = Test.group "Dataflow Warnings" <| Test.specify "should allow to attach multiple warnings and read them back" <| diff --git a/test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Nothing.enso b/test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Nothing.enso index 1a21011d4fc27..1a6e70d3a2366 100644 --- a/test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Nothing.enso +++ b/test/micro-distribution/lib/Standard/Base/0.0.0-dev/src/Nothing.enso @@ -1,2 +1,5 @@ +from project.Data.Boolean.Boolean import True + @Builtin_Type type Nothing + is_nothing self = True