Skip to content

Commit

Permalink
Improve diagnostic reporting for repeated annotations
Browse files Browse the repository at this point in the history 
Pass a log into DisambiguateTypeAnnotations, and use it to allow reporting multiple diagnostics for invalid repeated annotations instead of failing fast on the first one.

PiperOrigin-RevId: 725784135
  • Loading branch information
@cushon
cushon authored and Javac Team committed Feb 11, 2025
1 parent b714e8b commit 11aed5b
Show file tree
Hide file tree
Showing 5 changed files with 124 additions and 60 deletions.
10 changes: 7 additions & 3 deletions java/com/google/turbine/binder/Binder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,10 @@ static BindingResult bind(
log);
tenv =
disambiguateTypeAnnotations(
syms, tenv, CompoundEnv.<ClassSymbol, TypeBoundClass>of(classPathEnv).append(tenv));
syms,
tenv,
CompoundEnv.<ClassSymbol, TypeBoundClass>of(classPathEnv).append(tenv),
log);
tenv =
canonicalizeTypes(
syms, tenv, CompoundEnv.<ClassSymbol, TypeBoundClass>of(classPathEnv).append(tenv));
Expand Down Expand Up @@ -467,11 +470,12 @@ static boolean isConst(FieldInfo field) {
private static Env<ClassSymbol, SourceTypeBoundClass> disambiguateTypeAnnotations(
ImmutableSet<ClassSymbol> syms,
Env<ClassSymbol, SourceTypeBoundClass> stenv,
Env<ClassSymbol, TypeBoundClass> tenv) {
Env<ClassSymbol, TypeBoundClass> tenv,
TurbineLog log) {
SimpleEnv.Builder<ClassSymbol, SourceTypeBoundClass> builder = SimpleEnv.builder();
for (ClassSymbol sym : syms) {
SourceTypeBoundClass base = stenv.getNonNull(sym);
builder.put(sym, DisambiguateTypeAnnotations.bind(base, tenv));
builder.put(sym, DisambiguateTypeAnnotations.bind(base, tenv, log));
}
return builder.build();
}
Expand Down
95 changes: 46 additions & 49 deletions java/com/google/turbine/binder/DisambiguateTypeAnnotations.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@
import com.google.turbine.binder.bound.TypeBoundClass.RecordComponentInfo;
import com.google.turbine.binder.env.Env;
import com.google.turbine.binder.sym.ClassSymbol;
import com.google.turbine.diag.TurbineError;
import com.google.turbine.diag.TurbineError.ErrorKind;
import com.google.turbine.diag.TurbineLog;
import com.google.turbine.model.Const;
import com.google.turbine.model.TurbineElementType;
import com.google.turbine.type.AnnoInfo;
Expand Down Expand Up @@ -67,17 +67,20 @@
* and move it to the appropriate location.
*/
public final class DisambiguateTypeAnnotations {

public static SourceTypeBoundClass bind(
SourceTypeBoundClass base, Env<ClassSymbol, TypeBoundClass> env) {
SourceTypeBoundClass base, Env<ClassSymbol, TypeBoundClass> env, TurbineLog log) {

DisambiguateTypeAnnotations binder = new DisambiguateTypeAnnotations(env, log);
return new SourceTypeBoundClass(
base.interfaceTypes(),
base.permits(),
base.superClassType(),
base.typeParameterTypes(),
base.access(),
bindComponents(env, base.components(), TurbineElementType.RECORD_COMPONENT),
bindMethods(env, base.methods()),
bindFields(env, base.fields()),
binder.bindComponents(base.components(), TurbineElementType.RECORD_COMPONENT),
binder.bindMethods(base.methods()),
binder.bindFields(base.fields()),
base.owner(),
base.kind(),
base.children(),
Expand All @@ -86,25 +89,31 @@ public static SourceTypeBoundClass bind(
base.scope(),
base.memberImports(),
base.annotationMetadata(),
groupRepeated(env, base.annotations()),
binder.groupRepeated(base.annotations()),
base.source(),
base.decl());
}

private static ImmutableList<MethodInfo> bindMethods(
Env<ClassSymbol, TypeBoundClass> env, ImmutableList<MethodInfo> fields) {
private final Env<ClassSymbol, TypeBoundClass> env;
private final TurbineLog log;

private DisambiguateTypeAnnotations(Env<ClassSymbol, TypeBoundClass> env, TurbineLog log) {
this.env = env;
this.log = log;
}

private ImmutableList<MethodInfo> bindMethods(ImmutableList<MethodInfo> fields) {
ImmutableList.Builder<MethodInfo> result = ImmutableList.builder();
for (MethodInfo field : fields) {
result.add(bindMethod(env, field));
result.add(bindMethod(field));
}
return result.build();
}

private static MethodInfo bindMethod(Env<ClassSymbol, TypeBoundClass> env, MethodInfo base) {
private MethodInfo bindMethod(MethodInfo base) {
ImmutableList.Builder<AnnoInfo> declarationAnnotations = ImmutableList.builder();
Type returnType =
disambiguate(
env,
base.name().equals("<init>")
? TurbineElementType.CONSTRUCTOR
: TurbineElementType.METHOD,
Expand All @@ -115,51 +124,39 @@ private static MethodInfo bindMethod(Env<ClassSymbol, TypeBoundClass> env, Metho
base.sym(),
base.tyParams(),
returnType,
bindParameters(env, base.parameters(), TurbineElementType.PARAMETER),
bindParameters(base.parameters(), TurbineElementType.PARAMETER),
base.exceptions(),
base.access(),
base.defaultValue(),
base.decl(),
declarationAnnotations.build(),
base.receiver() != null
? bindParam(env, base.receiver(), TurbineElementType.PARAMETER)
: null);
base.receiver() != null ? bindParam(base.receiver(), TurbineElementType.PARAMETER) : null);
}

private static ImmutableList<ParamInfo> bindParameters(
Env<ClassSymbol, TypeBoundClass> env,
ImmutableList<ParamInfo> params,
TurbineElementType declarationTarget) {
private ImmutableList<ParamInfo> bindParameters(
ImmutableList<ParamInfo> params, TurbineElementType declarationTarget) {
ImmutableList.Builder<ParamInfo> result = ImmutableList.builder();
for (ParamInfo param : params) {
result.add(bindParam(env, param, declarationTarget));
result.add(bindParam(param, declarationTarget));
}
return result.build();
}

private static ParamInfo bindParam(
Env<ClassSymbol, TypeBoundClass> env, ParamInfo base, TurbineElementType declarationTarget) {
private ParamInfo bindParam(ParamInfo base, TurbineElementType declarationTarget) {
ImmutableList.Builder<AnnoInfo> declarationAnnotations = ImmutableList.builder();
Type type =
disambiguate(
env, declarationTarget, base.type(), base.annotations(), declarationAnnotations);
disambiguate(declarationTarget, base.type(), base.annotations(), declarationAnnotations);
return new ParamInfo(base.sym(), type, declarationAnnotations.build(), base.access());
}

private static ImmutableList<RecordComponentInfo> bindComponents(
Env<ClassSymbol, TypeBoundClass> env,
ImmutableList<RecordComponentInfo> components,
TurbineElementType declarationTarget) {
private ImmutableList<RecordComponentInfo> bindComponents(
ImmutableList<RecordComponentInfo> components, TurbineElementType declarationTarget) {
ImmutableList.Builder<RecordComponentInfo> result = ImmutableList.builder();
for (RecordComponentInfo component : components) {
ImmutableList.Builder<AnnoInfo> declarationAnnotations = ImmutableList.builder();
Type type =
disambiguate(
env,
declarationTarget,
component.type(),
component.annotations(),
declarationAnnotations);
declarationTarget, component.type(), component.annotations(), declarationAnnotations);
result.add(
new RecordComponentInfo(
component.sym(), type, declarationAnnotations.build(), component.access()));
Expand All @@ -171,18 +168,17 @@ private static ImmutableList<RecordComponentInfo> bindComponents(
* Moves type annotations in {@code annotations} to {@code type}, and adds any declaration
* annotations on {@code type} to {@code declarationAnnotations}.
*/
private static Type disambiguate(
Env<ClassSymbol, TypeBoundClass> env,
private Type disambiguate(
TurbineElementType declarationTarget,
Type type,
ImmutableList<AnnoInfo> annotations,
ImmutableList.Builder<AnnoInfo> declarationAnnotations) {
// desugar @Repeatable annotations before disambiguating: annotation containers may target
// a subset of the types targeted by their element annotation
annotations = groupRepeated(env, annotations);
annotations = groupRepeated(annotations);
ImmutableList.Builder<AnnoInfo> typeAnnotations = ImmutableList.builder();
for (AnnoInfo anno : annotations) {
ImmutableSet<TurbineElementType> target = getTarget(env, anno);
ImmutableSet<TurbineElementType> target = getTarget(anno);
if (target.contains(TurbineElementType.TYPE_USE)) {
typeAnnotations.add(anno);
}
Expand All @@ -193,8 +189,7 @@ private static Type disambiguate(
return addAnnotationsToType(type, typeAnnotations.build());
}

private static ImmutableSet<TurbineElementType> getTarget(
Env<ClassSymbol, TypeBoundClass> env, AnnoInfo anno) {
private ImmutableSet<TurbineElementType> getTarget(AnnoInfo anno) {
ClassSymbol sym = anno.sym();
if (sym == null) {
return AnnotationMetadata.DEFAULT_TARGETS;
Expand All @@ -210,20 +205,19 @@ private static ImmutableSet<TurbineElementType> getTarget(
return metadata.target();
}

private static ImmutableList<FieldInfo> bindFields(
Env<ClassSymbol, TypeBoundClass> env, ImmutableList<FieldInfo> fields) {
private ImmutableList<FieldInfo> bindFields(ImmutableList<FieldInfo> fields) {
ImmutableList.Builder<FieldInfo> result = ImmutableList.builder();
for (FieldInfo field : fields) {
result.add(bindField(env, field));
result.add(bindField(field));
}
return result.build();
}

private static FieldInfo bindField(Env<ClassSymbol, TypeBoundClass> env, FieldInfo base) {
private FieldInfo bindField(FieldInfo base) {
ImmutableList.Builder<AnnoInfo> declarationAnnotations = ImmutableList.builder();
Type type =
disambiguate(
env, TurbineElementType.FIELD, base.type(), base.annotations(), declarationAnnotations);
TurbineElementType.FIELD, base.type(), base.annotations(), declarationAnnotations);
return new FieldInfo(
base.sym(), type, base.access(), declarationAnnotations.build(), base.decl(), base.value());
}
Expand Down Expand Up @@ -284,7 +278,11 @@ private static ImmutableList<AnnoInfo> appendAnnotations(
* here, but it would require another rewrite pass.
*/
public static ImmutableList<AnnoInfo> groupRepeated(
Env<ClassSymbol, TypeBoundClass> env, ImmutableList<AnnoInfo> annotations) {
Env<ClassSymbol, TypeBoundClass> env, TurbineLog log, ImmutableList<AnnoInfo> annotations) {
return new DisambiguateTypeAnnotations(env, log).groupRepeated(annotations);
}

private ImmutableList<AnnoInfo> groupRepeated(ImmutableList<AnnoInfo> annotations) {
Multimap<ClassSymbol, AnnoInfo> repeated =
MultimapBuilder.linkedHashKeys().arrayListValues().build();
ImmutableList.Builder<AnnoInfo> result = ImmutableList.builder();
Expand All @@ -310,8 +308,9 @@ public static ImmutableList<AnnoInfo> groupRepeated(
ClassSymbol container = info.annotationMetadata().repeatable();
if (container == null) {
AnnoInfo anno = infos.iterator().next();
throw TurbineError.format(
anno.source(), anno.position(), ErrorKind.NONREPEATABLE_ANNOTATION, symbol);
log.withSource(anno.source())
.error(anno.position(), ErrorKind.NONREPEATABLE_ANNOTATION, symbol);
continue;
}
result.add(
new AnnoInfo(
Expand All @@ -325,6 +324,4 @@ public static ImmutableList<AnnoInfo> groupRepeated(
}
return result.build();
}

private DisambiguateTypeAnnotations() {}
}
23 changes: 15 additions & 8 deletions java/com/google/turbine/lower/Lower.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
import com.google.turbine.diag.SourceFile;
import com.google.turbine.diag.TurbineError;
import com.google.turbine.diag.TurbineError.ErrorKind;
import com.google.turbine.diag.TurbineLog;
import com.google.turbine.model.Const;
import com.google.turbine.model.TurbineFlag;
import com.google.turbine.model.TurbineTyKind;
Expand Down Expand Up @@ -148,50 +149,56 @@ public static Lowered lowerAll(
Set<ClassSymbol> symbols = new LinkedHashSet<>();
// Output Java 8 bytecode at minimum, for type annotations
int majorVersion = max(options.languageVersion().majorVersion(), 52);
TurbineLog log = new TurbineLog();
for (ClassSymbol sym : units.keySet()) {
result.put(
sym.binaryName(),
lower(units.get(sym), env, sym, symbols, majorVersion, options.emitPrivateFields()));
lower(units.get(sym), env, log, sym, symbols, majorVersion, options.emitPrivateFields()));
}
if (modules.size() == 1) {
// single module mode: the module-info.class file is at the root
result.put("module-info", lower(getOnlyElement(modules), env, symbols, majorVersion));
result.put("module-info", lower(getOnlyElement(modules), env, log, symbols, majorVersion));
} else {
// multi-module mode: the output module-info.class are in a directory corresponding to their
// package
for (SourceModuleInfo module : modules) {
result.put(
module.name().replace('.', '/') + "/module-info",
lower(module, env, symbols, majorVersion));
lower(module, env, log, symbols, majorVersion));
}
}
log.maybeThrow();
return Lowered.create(result.buildOrThrow(), ImmutableSet.copyOf(symbols));
}

/** Lowers a class to bytecode. */
private static byte[] lower(
SourceTypeBoundClass info,
Env<ClassSymbol, TypeBoundClass> env,
TurbineLog log,
ClassSymbol sym,
Set<ClassSymbol> symbols,
int majorVersion,
boolean emitPrivateFields) {
return new Lower(env).lower(info, sym, symbols, majorVersion, emitPrivateFields);
return new Lower(env, log).lower(info, sym, symbols, majorVersion, emitPrivateFields);
}

private static byte[] lower(
SourceModuleInfo module,
CompoundEnv<ClassSymbol, TypeBoundClass> env,
TurbineLog log,
Set<ClassSymbol> symbols,
int majorVersion) {
return new Lower(env).lower(module, symbols, majorVersion);
return new Lower(env, log).lower(module, symbols, majorVersion);
}

private final LowerSignature sig = new LowerSignature();
private final Env<ClassSymbol, TypeBoundClass> env;
private final TurbineLog log;

public Lower(Env<ClassSymbol, TypeBoundClass> env) {
public Lower(Env<ClassSymbol, TypeBoundClass> env, TurbineLog log) {
this.env = env;
this.log = log;
}

private byte[] lower(SourceModuleInfo module, Set<ClassSymbol> symbols, int majorVersion) {
Expand Down Expand Up @@ -794,7 +801,7 @@ private void typeParameterAnnotations(
TargetType boundTargetType) {
int typeParameterIndex = 0;
for (TyVarInfo p : typeParameters) {
for (AnnoInfo anno : groupRepeated(env, p.annotations())) {
for (AnnoInfo anno : groupRepeated(env, log, p.annotations())) {
AnnotationInfo info = lowerAnnotation(anno);
if (info == null) {
continue;
Expand Down Expand Up @@ -880,7 +887,7 @@ private void lowerTypeAnnotations(Type type, TypePath path) {

/** Lower a list of type annotations. */
private void lowerTypeAnnotations(ImmutableList<AnnoInfo> annos, TypePath path) {
for (AnnoInfo anno : groupRepeated(env, annos)) {
for (AnnoInfo anno : groupRepeated(env, log, annos)) {
AnnotationInfo info = lowerAnnotation(anno);
if (info == null) {
continue;
Expand Down
17 changes: 17 additions & 0 deletions javatests/com/google/turbine/binder/BinderErrorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1024,6 +1024,23 @@ public static Iterable<Object[]> parameters() {
" ^",
},
},
{
{
"@interface Anno {}", //
"class Test {",
" @Anno @Anno int x;",
" @Anno @Anno int y;",
"}",
},
{
"<>:3: error: Anno is not @Repeatable",
" @Anno @Anno int x;",
" ^",
"<>:4: error: Anno is not @Repeatable",
" @Anno @Anno int y;",
" ^",
},
},
};
return Arrays.asList((Object[][]) testCases);
}
Expand Down
Loading

0 comments on commit 11aed5b

Please sign in to comment.