Skip to content

Commit

Permalink
Fix a problem in separate compilation
Browse files Browse the repository at this point in the history
Discovered a problem with separate compilation when generating exception
wrappers. The latter requires information about all builtin types
annotations but that is not present in separate compilation.
Instead we do a similar trick as in other processors - we read metadata
and diff the results.

The change is bigger than a single method because I didn't want to write
a yet another parser of metadata entries. Instead, every annotation
processor has to provide that implementation and it can be re-used.
  • Loading branch information
hubertp committed May 18, 2022
1 parent cbef1ec commit d6a3614
Show file tree
Hide file tree
Showing 4 changed files with 129 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

import javax.annotation.processing.AbstractProcessor;
import javax.annotation.processing.RoundEnvironment;
import javax.lang.model.element.Element;
import javax.lang.model.element.TypeElement;
import javax.tools.Diagnostic;
import javax.tools.FileObject;
Expand All @@ -19,7 +18,8 @@
* generating code, {@code BuiltinsMetadataProcessor} detects when the processing of the last
* annotation in the round is being processed and allows for dumping any collected metadata once.
*/
public abstract class BuiltinsMetadataProcessor extends AbstractProcessor {
public abstract class BuiltinsMetadataProcessor<T extends BuiltinsMetadataProcessor.MetadataEntry>
extends AbstractProcessor {

/**
* Processes annotated elements, generating code for each of them, if necessary.
Expand All @@ -44,15 +44,16 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
// we read the exisitng metadata.
// Deletes/renaming are still not going to work nicely but that would be the same case
// if we were writing metadata information per source file anyway.
Map<String, String> pastEntries;
Map<String, T> pastEntries;
try {
FileObject existingFile =
processingEnv.getFiler().getResource(StandardLocation.CLASS_OUTPUT, "", metadataPath());
try (InputStream resource = existingFile.openInputStream()) {
pastEntries =
new BufferedReader(new InputStreamReader(resource, StandardCharsets.UTF_8))
.lines()
.collect(Collectors.toMap(l -> l.split(":")[0], Function.identity()));
.map(l -> toMetadataEntry(l))
.collect(Collectors.toMap(e -> e.key(), Function.identity()));
}
} catch (NoSuchFileException notFoundException) {
// This is the first time we are generating the metadata file, ignore the exception.
Expand All @@ -70,8 +71,8 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
try {
storeMetadata(writer, pastEntries);
// Dump past entries, to workaround separate compilation + annotation processing issues
for (String value : pastEntries.values()) {
writer.append(value + "\n");
for (MetadataEntry value : pastEntries.values()) {
writer.append(value.toString() + "\n");
}
} finally {
writer.close();
Expand Down Expand Up @@ -104,7 +105,7 @@ public final boolean process(Set<? extends TypeElement> annotations, RoundEnviro
* should not be appended to {@code writer} should be removed
* @throws IOException
*/
protected abstract void storeMetadata(Writer writer, Map<String, String> pastEntries)
protected abstract void storeMetadata(Writer writer, Map<String, T> pastEntries)
throws IOException;

/**
Expand All @@ -123,4 +124,22 @@ protected abstract boolean handleProcess(
* processing is done.
*/
protected abstract void cleanup();

/**
* A common interface that all metadata entries have to provide.
* In the future can avoid plain text representation.
*/
public interface MetadataEntry {
String toString();

String key();
}

/**
* A conversion method for a single raw metadata entry. For now represented as a string.
*
* @param line raw representation of the metadata entry
* @return parsed metedata entry, specific to the given processor
*/
protected abstract T toMetadataEntry(String line);
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
import javax.lang.model.type.TypeMirror;
import javax.lang.model.util.Types;
import javax.tools.Diagnostic;
import javax.tools.FileObject;
import javax.tools.JavaFileObject;
import javax.tools.StandardLocation;

import java.io.IOException;
import java.io.PrintWriter;
import java.io.*;
import java.lang.annotation.Annotation;
import java.nio.charset.StandardCharsets;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.IntStream;
Expand Down Expand Up @@ -164,11 +166,40 @@ public void handleMethodElement(Element element, RoundEnvironment roundEnv) thro
}
}

/**
* Returns a map of builtin types and the number of their paramneters.
* Takes into account the possibility of separate compilation by reading entries from metadate, if any.
* @param roundEnv current round environment
* @return a map from a builtin type name to the number of its parameters
*/
private Map<String, Integer> builtinTypesParametersCount(RoundEnvironment roundEnv) {
return roundEnv
// For separate compilation we need to read that information from BuiltinTypes metadata file
Map<String, Integer> pastEntries;
try {
FileObject existingFile =
processingEnv.getFiler().getResource(StandardLocation.CLASS_OUTPUT, "", TypeProcessor.META_PATH);

try (InputStream resource = existingFile.openInputStream()) {
pastEntries =
new BufferedReader(new InputStreamReader(resource, StandardCharsets.UTF_8))
.lines()
.map(l -> TypeProcessor.fromStringToMetadataEntry(l))
.collect(Collectors.toMap(e -> e.key().replaceAll("_", ""), e -> e.paramNames().length));
}
} catch (IOException e) {
// Ignore, this is a clean run
pastEntries = new HashMap<>();
}

Map<String, Integer> currentRoundEntries =
roundEnv
.getElementsAnnotatedWith(BuiltinType.class)
.stream()
.collect(Collectors.toMap(e -> e.getSimpleName().toString(), e -> e.getAnnotation(BuiltinType.class).params().length));

pastEntries.forEach((k, v) -> currentRoundEntries.merge(k, v, (v1, v2) -> v1));

return currentRoundEntries;
}

/**
Expand Down Expand Up @@ -409,11 +440,17 @@ List<String> toCatchClause(List<MethodParameter> methodParameters, Map<String, I
String from = fromAttributeToClassName(from());
String to = fromAttributeToClassName(to());
int toParamCount = errorParametersCount(to(), builtinTypesParameterCounts);
String errorParamsApplied = StringUtils.join(methodParameters.stream().limit(toParamCount - 1).flatMap(x -> x.names(Optional.empty())).toArray(), ", ");
List<String> errorParameters =
methodParameters
.stream()
.limit(toParamCount - 1)
.flatMap(x -> x.names(Optional.empty()))
.collect(Collectors.toList());
String errorParameterCode = errorParameters.isEmpty() ? "" : ", " + StringUtils.join(errorParameters, ", ");
return List.of(
" } catch (" + from + " exception) {",
" Builtins builtins = Context.get(this).getBuiltins();",
" throw new PanicException(builtins.error().make" + to + "(_this, " + errorParamsApplied + "), this);"
" throw new PanicException(builtins.error().make" + to + "(_this" + errorParameterCode + "), this);"
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.enso.interpreter.dsl;

import org.apache.commons.lang3.StringUtils;
import org.enso.interpreter.dsl.model.MethodDefinition;

import javax.annotation.processing.*;
Expand All @@ -20,7 +21,7 @@
*/
@SupportedAnnotationTypes("org.enso.interpreter.dsl.BuiltinMethod")
@ServiceProvider(service = Processor.class)
public class MethodProcessor extends BuiltinsMetadataProcessor {
public class MethodProcessor extends BuiltinsMetadataProcessor<MethodProcessor.MethodMetadataEntry> {

private final Map<Filer, Map<String, String>> builtinMethods = new HashMap<>();

Expand Down Expand Up @@ -436,7 +437,7 @@ private boolean generateWarningsCheck(
* should not be appended to {@code writer} should be removed
* @throws IOException
*/
protected void storeMetadata(Writer writer, Map<String, String> pastEntries) throws IOException {
protected void storeMetadata(Writer writer, Map<String, MethodMetadataEntry> pastEntries) throws IOException {
for (Filer f : builtinMethods.keySet()) {
for (Map.Entry<String, String> entry : builtinMethods.get(f).entrySet()) {
writer.append(entry.getKey() + ":" + entry.getValue() + "\n");
Expand Down Expand Up @@ -485,4 +486,24 @@ private String capitalize(String name) {
public SourceVersion getSupportedSourceVersion() {
return SourceVersion.latest();
}

public record MethodMetadataEntry(String fullEnsoName, String clazzName) implements MetadataEntry {

@Override
public String toString() {
return fullEnsoName + ":" + clazzName;
}

@Override
public String key() {
return fullEnsoName;
}
}

@Override
protected MethodMetadataEntry toMetadataEntry(String line) {
String[] elements = line.split(":");
if (elements.length != 2) throw new RuntimeException("invalid builtin metadata entry: " + line);
return new MethodMetadataEntry(elements[0], elements[1]);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

@SupportedAnnotationTypes("org.enso.interpreter.dsl.BuiltinType")
@ServiceProvider(service = Processor.class)
public class TypeProcessor extends BuiltinsMetadataProcessor {
public class TypeProcessor extends BuiltinsMetadataProcessor<TypeProcessor.TypeMetadataEntry> {

private final Map<Filer, Map<String, BuiltinTypeConstr>> builtinTypes = new HashMap<>();

Expand Down Expand Up @@ -84,7 +84,7 @@ protected boolean handleProcess(
* @throws IOException
*/
@Override
protected void storeMetadata(Writer writer, Map<String, String> pastEntries) throws IOException {
protected void storeMetadata(Writer writer, Map<String, TypeMetadataEntry> pastEntries) throws IOException {
JavaFileObject gen = processingEnv.getFiler().createSourceFile(ConstantsGenFullClassname);
for (Filer f : builtinTypes.keySet()) {
System.out.println("foo" + f.toString());
Expand Down Expand Up @@ -126,17 +126,16 @@ protected void storeMetadata(Writer writer, Map<String, String> pastEntries) thr
pastEntries
.values()
.forEach(
entry -> {
String[] elements = entry.split(":");
if (elements.length == 4 && !elements[3].isEmpty()) {
out.println(
" public static final String "
+ elements[0].toUpperCase()
+ " = \""
+ elements[3]
+ "\";");
}
});
entry ->
entry.stdlibName().ifPresent(n ->
out.println(
" public static final String "
+ entry.ensoName().toUpperCase()
+ " = \""
+ n
+ "\";")
)
);

out.println();
out.println("}");
Expand Down Expand Up @@ -171,4 +170,30 @@ protected void cleanup() {
public SourceVersion getSupportedSourceVersion() {
return SourceVersion.latest();
}

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

@Override
public String toString() {
return ensoName + ":" + clazzName + ":" + StringUtils.join(paramNames, ",") + ":" + stdlibName.orElse("");
}

@Override
public String key() {
return ensoName;
}
}

@Override
protected TypeMetadataEntry toMetadataEntry(String line) {
return fromStringToMetadataEntry(line);
}

public static TypeMetadataEntry fromStringToMetadataEntry(String line) {
String[] elements = line.split(":");
if (elements.length < 2) throw new RuntimeException("invalid builtin metadata entry: " + line);
String[] params = elements.length >= 3 ? elements[2].split(",") : new String[0];
Optional<String> stdLibName = elements.length == 4 ? Optional.of(elements[3]) : Optional.empty();
return new TypeMetadataEntry(elements[0], elements[1], params, stdLibName);
}
}

0 comments on commit d6a3614

Please sign in to comment.