Skip to content

Commit

Permalink
Adjust generateCheckedArgumentRead to return correct expected type (#…
Browse files Browse the repository at this point in the history 
…5769)

Correctly get the expected type and return an IllegalArgument if no Enso type.

![image](https://user-images.githubusercontent.com/4699705/221263862-bbc122ca-b11f-49f3-b7cf-6294ea811a22.png)
  • Loading branch information
@jdunkerley
jdunkerley authored Feb 27, 2023
1 parent 43144e1 commit ba3d45e
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 39 deletions.
2 changes: 1 addition & 1 deletion engine/runtime/src/main/java/org/enso/interpreter/runtime/builtin/Builtins.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ private static List<Constructor<? extends Builtin>> readBuiltinTypes() {
.map(
line -> {
String[] builtinMeta = line.split(":");
if (builtinMeta.length < 2 || builtinMeta.length > 3) {
if (builtinMeta.length < 2 || builtinMeta.length > 4) {
java.lang.System.out.println(Arrays.toString(builtinMeta));
throw new CompilerError("Invalid builtin metadata in: " + line);
}
Expand Down
3 changes: 3 additions & 0 deletions lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/BuiltinType.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@

/** Fully qualified name as available in stdlib */
String name() default "";

/** Underlying type name of the builtin */
String underlyingTypeName() default "";
}
13 changes: 5 additions & 8 deletions lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/BuiltinsProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class BuiltinsProcessor extends AbstractProcessor {

private record Specialized(String owner, String methodName) {}

private Map<Specialized, List<ExecutableElement>> specializedMethods = new HashMap<>();
private final Map<Specialized, List<ExecutableElement>> specializedMethods = new HashMap<>();

@Override
public final boolean process(Set<? extends TypeElement> annotations, RoundEnvironment roundEnv) {
Expand Down Expand Up @@ -91,20 +91,20 @@ public void handleClassElement(Element element, RoundEnvironment roundEnv) throw
processingEnv.getFiler().createSourceFile(builtinType.fullyQualifiedName());
Optional<String> stdLibName =
annotation.stdlibName().isEmpty() ? Optional.empty() : Optional.of(annotation.stdlibName());
generateBuiltinType(gen, builtinType, stdLibName);
generateBuiltinType(gen, builtinType, stdLibName, elt.getSimpleName().toString());
}

private void generateBuiltinType(
JavaFileObject gen, ClassName builtinType, Optional<String> stdLibName) throws IOException {
JavaFileObject gen, ClassName builtinType, Optional<String> stdLibName, String underlyingTypeName) throws IOException {
try (PrintWriter out = new PrintWriter(gen.openWriter())) {
out.println("package " + builtinType.pkg() + ";");
out.println();
for (String importPkg : typeNecessaryImports) {
out.println("import " + importPkg + ";");
}
out.println();
String stdLib = stdLibName.map(n -> "(name = \"" + n + "\")").orElse("");
out.println("@BuiltinType" + stdLib);
String builtinTypeAnnotation = "@BuiltinType(" + stdLibName.map(n -> "name = \"" + n + "\", ").orElse("") + "underlyingTypeName = \"" + underlyingTypeName + "\")";
out.println(builtinTypeAnnotation);
out.println("public class " + builtinType.name() + " extends Builtin {");
out.println("""
@Override
Expand Down Expand Up @@ -238,11 +238,8 @@ public void handleMethodElement(Element element, RoundEnvironment roundEnv) thro
annotation.description(),
method.getSimpleName().toString(),
annotation.autoRegister());
} else {
return;
}
} else {

MethodNodeClassGenerator classGenerator =
new NoSpecializationClassGenerator(
method, builtinMethodNode, ownerClass, stdLibOwnerClass);
Expand Down
29 changes: 17 additions & 12 deletions lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/MethodProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,8 @@ private void generateUncheckedArrayCast(

private void generateCheckedArgumentRead(
PrintWriter out, MethodDefinition.ArgumentDefinition arg, String argsArray) {
String castName = "TypesGen.expect" + capitalize(arg.getTypeName());
String builtinName = capitalize(arg.getTypeName());
String castName = "TypesGen.expect" + builtinName;
String varName = mkArgumentInternalVarName(arg);
out.println(" " + arg.getTypeName() + " " + varName + ";");
out.println(" try {");
Expand All @@ -432,16 +433,20 @@ private void generateCheckedArgumentRead(
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[arg"
+ arg.getPosition()
+ "Idx]));");
out.println(
" var error = builtins.error().makeTypeError(expected, arguments[arg"
+ arg.getPosition()
+ "Idx], \""
+ varName
+ "\");");
out.println(" var ensoTypeName = org.enso.interpreter.runtime.type.ConstantsGen.getEnsoTypeName(\"" + builtinName + "\");");
out.println(" var error = (ensoTypeName != null)");
out.println(" ? builtins.error().makeTypeError(builtins.fromTypeSystem(ensoTypeName), arguments[arg"
+ arg.getPosition()
+ "Idx], \""
+ varName
+ "\")");
out.println(" : builtins.error().makeUnsupportedArgumentsError(new Object[] { arguments[arg"
+ arg.getPosition()
+ "Idx] }, \"Unsupported argument for "
+ varName
+ " expected a "
+ builtinName
+ "\");");
out.println(" throw new PanicException(error,this);");
out.println(" }");
}
Expand Down Expand Up @@ -559,7 +564,7 @@ public String key() {
protected MethodMetadataEntry toMetadataEntry(String line) {
String[] elements = line.split(":");
if (elements.length != 4) throw new RuntimeException("invalid builtin metadata entry: " + line);
return new MethodMetadataEntry(elements[0], elements[1], Boolean.valueOf(elements[2]), Boolean.valueOf(elements[3]));
return new MethodMetadataEntry(elements[0], elements[1], Boolean.parseBoolean(elements[2]), Boolean.parseBoolean(elements[3]));
}

private static final String DATAFLOW_ERROR_PROFILE = "IsDataflowErrorConditionProfile";
Expand Down
64 changes: 46 additions & 18 deletions lib/scala/interpreter-dsl/src/main/java/org/enso/interpreter/dsl/TypeProcessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import java.io.Writer;
import java.util.*;

import org.apache.commons.lang3.StringUtils;
import org.openide.util.lookup.ServiceProvider;

@SupportedAnnotationTypes("org.enso.interpreter.dsl.BuiltinType")
Expand All @@ -22,12 +21,14 @@ public class TypeProcessor extends BuiltinsMetadataProcessor<TypeProcessor.TypeM
private JavaFileObject jfo = null;

private class BuiltinTypeConstr {
private String tpeName;
private String fullName;
private final String tpeName;
private final String fullName;
private final String builtinTypeName;

BuiltinTypeConstr(String tpeName, String fullName) {
BuiltinTypeConstr(String tpeName, String fullName, String builtinTypeName) {
this.tpeName = tpeName;
this.fullName = fullName;
this.builtinTypeName = builtinTypeName;
}

public String getFullName() {
Expand All @@ -37,6 +38,10 @@ public String getFullName() {
public String getTpeName() {
return tpeName;
}

public String getBuiltinTypeName() {
return builtinTypeName;
}
}

public static final String NODE_PKG = "org.enso.interpreter.node.expression.builtin";
Expand Down Expand Up @@ -71,7 +76,8 @@ protected boolean handleProcess(
processingEnv.getFiler(),
ensoTypeName,
pkgName + "." + clazzName,
builtinTypeAnnotation.name());
builtinTypeAnnotation.name(),
builtinTypeAnnotation.underlyingTypeName());
}
}
return true;
Expand All @@ -87,7 +93,7 @@ protected boolean handleProcess(
* @param writer a writer to the metadata resource
* @param pastEntries entries from the previously created metadata file, if any. Entries that
* should not be appended to {@code writer} should be removed
* @throws IOException
* @throws IOException - if an I/O error occurred
*/
@Override
protected void storeMetadata(Writer writer, Map<String, TypeMetadataEntry> pastEntries) throws IOException {
Expand All @@ -100,6 +106,8 @@ protected void storeMetadata(Writer writer, Map<String, TypeMetadataEntry> pastE
+ constr.getTpeName()
+ ":"
+ constr.getFullName()
+ ":"
+ constr.builtinTypeName
+ "\n");
if (pastEntries.containsKey(entry.getKey())) {
pastEntries.remove(entry.getKey());
Expand All @@ -111,23 +119,42 @@ protected void storeMetadata(Writer writer, Map<String, TypeMetadataEntry> pastE
out.println();
out.println("public class " + ConstantsGenClass + " {");
out.println();

var lookup = new HashMap<String, String>();
for (Filer f : builtinTypes.keySet()) {
for (Map.Entry<String, BuiltinTypeConstr> entry : builtinTypes.get(f).entrySet()) {
BuiltinTypeConstr constr = entry.getValue();
if (!constr.getFullName().isEmpty()) {
generateEntry(entry.getKey().toUpperCase(), constr.getFullName(), out);
var name = entry.getKey().toUpperCase();
generateEntry(name, constr.getFullName(), out);
if (!constr.getBuiltinTypeName().equals("")) {
lookup.put(constr.getBuiltinTypeName(), name);
}
}
}
}

pastEntries
.values()
.forEach(
entry ->
entry.stdlibName().ifPresent(n -> generateEntry(entry.ensoName().toUpperCase(), n, out))
);
.forEach((k, v) -> {
v.stdlibName().ifPresent(name -> generateEntry(v.ensoName().toUpperCase(), name, out));
v.builtinTypeName().ifPresent(builtinTypeName -> lookup.put(builtinTypeName, k.toUpperCase()));
});

out.println();

out.println(" public static String getEnsoTypeName(String builtinName) {");
out.println(" return switch (builtinName) {");
out.println(" case \"Long\" -> " + ConstantsGenClass + ".INTEGER;");
out.println(" case \"Double\" -> " + ConstantsGenClass + ".DECIMAL;");
out.println(" case \"Text\" -> " + ConstantsGenClass + ".TEXT;");
lookup.forEach((k, v) ->
out.println(" case \"" + k + "\" -> " + ConstantsGenClass + "." + v + ";")
);
out.println(" default -> null;");
out.println(" };");
out.println(" }");
out.println();

out.println("}");
}
}
Expand All @@ -152,13 +179,13 @@ private String toBuiltinName(String name) {
}

protected void registerBuiltinType(
Filer f, String name, String clazzName, String fullName) {
Filer f, String name, String clazzName, String fullName, String builtinTypeName) {
Map<String, BuiltinTypeConstr> classes = builtinTypes.get(f);
if (classes == null) {
classes = new HashMap<>();
builtinTypes.put(f, classes);
}
classes.put(name, new BuiltinTypeConstr(clazzName, fullName));
classes.put(name, new BuiltinTypeConstr(clazzName, fullName, builtinTypeName));
}

@Override
Expand All @@ -180,11 +207,11 @@ public SourceVersion getSupportedSourceVersion() {
return SourceVersion.latest();
}

public record TypeMetadataEntry(String ensoName, String clazzName, Optional<String> stdlibName) implements MetadataEntry {
public record TypeMetadataEntry(String ensoName, String clazzName, Optional<String> stdlibName, Optional<String> builtinTypeName) implements MetadataEntry {

@Override
public String toString() {
return ensoName + ":" + clazzName + ":" + stdlibName.orElse("");
return ensoName + ":" + clazzName + ":" + stdlibName.orElse("") + ":" + builtinTypeName.orElse("");
}

@Override
Expand All @@ -201,7 +228,8 @@ protected TypeMetadataEntry toMetadataEntry(String line) {
public static TypeMetadataEntry fromStringToMetadataEntry(String line) {
String[] elements = line.split(":");
if (elements.length < 2) throw new RuntimeException("invalid builtin metadata entry: " + line);
Optional<String> stdLibName = elements.length == 3 ? Optional.of(elements[2]) : Optional.empty();
return new TypeMetadataEntry(elements[0], elements[1], stdLibName);
Optional<String> stdLibName = elements.length >= 3 ? Optional.of(elements[2]) : Optional.empty();
Optional<String> builtinTypeName = elements.length == 4 ? Optional.of(elements[3]) : Optional.empty();
return new TypeMetadataEntry(elements[0], elements[1], stdLibName, builtinTypeName);
}
}

0 comments on commit ba3d45e

Please sign in to comment.