Skip to content

Commit 006a74b

Browse files
cushonJavac Team
cushon
authored and
Javac Team
committed
Improve major version handling
Parse -source/-target/--release flags from provided javacopts, and use them to set the major version of the classfile outputs, instead of trying to use the lowest possible version that supported the features in the output. PiperOrigin-RevId: 398583014
1 parent 6b0f9e9 commit 006a74b

22 files changed

Lines changed: 397 additions & 236 deletions

‎java/com/google/turbine/binder/CtSymClassBinder.java‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import com.google.common.base.Supplier;
2525
import com.google.common.base.Suppliers;
2626
import com.google.common.collect.ImmutableMap;
27-
import com.google.common.primitives.Ints;
2827
import com.google.turbine.binder.bound.ModuleInfo;
2928
import com.google.turbine.binder.bytecode.BytecodeBinder;
3029
import com.google.turbine.binder.bytecode.BytecodeBoundClass;
@@ -47,7 +46,7 @@
4746
/** Constructs a platform {@link ClassPath} from the current JDK's ct.sym file. */
4847
public final class CtSymClassBinder {
4948

50-
public static @Nullable ClassPath bind(String version) throws IOException {
49+
public static @Nullable ClassPath bind(int version) throws IOException {
5150
String javaHome = JAVA_HOME.value();
5251
requireNonNull(javaHome, "attempted to use --release, but JAVA_HOME is not set");
5352
Path ctSym = Paths.get(javaHome).resolve("lib/ct.sym");
@@ -134,10 +133,9 @@ public byte[] get() {
134133
}
135134

136135
@VisibleForTesting
137-
static String formatReleaseVersion(String version) {
138-
Integer n = Ints.tryParse(version);
139-
if (n == null || n <= 4 || n >= 36) {
140-
throw new IllegalArgumentException("invalid release version: " + version);
136+
static String formatReleaseVersion(int n) {
137+
if (n <= 4 || n >= 36) {
138+
throw new IllegalArgumentException("invalid release version: " + n);
141139
}
142140
return toUpperCase(Integer.toString(n, 36));
143141
}

‎java/com/google/turbine/binder/Processing.java‎

Lines changed: 1 addition & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import static java.util.Objects.requireNonNull;
2020

2121
import com.google.auto.value.AutoValue;
22-
import com.google.common.annotations.VisibleForTesting;
2322
import com.google.common.base.Function;
2423
import com.google.common.base.Joiner;
2524
import com.google.common.base.Stopwatch;
@@ -30,7 +29,6 @@
3029
import com.google.common.collect.ImmutableSet;
3130
import com.google.common.collect.ImmutableSetMultimap;
3231
import com.google.common.collect.Sets;
33-
import com.google.common.primitives.Ints;
3432
import com.google.turbine.binder.Binder.BindingResult;
3533
import com.google.turbine.binder.Binder.Statistics;
3634
import com.google.turbine.binder.bound.SourceTypeBoundClass;
@@ -61,7 +59,6 @@
6159
import java.util.ArrayList;
6260
import java.util.Collection;
6361
import java.util.HashSet;
64-
import java.util.Iterator;
6562
import java.util.LinkedHashMap;
6663
import java.util.LinkedHashSet;
6764
import java.util.List;
@@ -392,6 +389,7 @@ private static void addAnno(
392389
}
393390

394391
public static ProcessorInfo initializeProcessors(
392+
SourceVersion sourceVersion,
395393
ImmutableList<String> javacopts,
396394
ImmutableSet<String> processorNames,
397395
ClassLoader processorLoader) {
@@ -400,7 +398,6 @@ public static ProcessorInfo initializeProcessors(
400398
}
401399
ImmutableList<Processor> processors = instantiateProcessors(processorNames, processorLoader);
402400
ImmutableMap<String, String> processorOptions = processorOptions(javacopts);
403-
SourceVersion sourceVersion = parseSourceVersion(javacopts);
404401
return ProcessorInfo.create(processors, processorLoader, processorOptions, sourceVersion);
405402
}
406403

@@ -451,51 +448,6 @@ protected Class<?> findClass(String name) throws ClassNotFoundException {
451448
});
452449
}
453450

454-
@VisibleForTesting
455-
static SourceVersion parseSourceVersion(ImmutableList<String> javacopts) {
456-
SourceVersion sourceVersion = SourceVersion.latestSupported();
457-
Iterator<String> it = javacopts.iterator();
458-
while (it.hasNext()) {
459-
String option = it.next();
460-
switch (option) {
461-
case "-source":
462-
if (!it.hasNext()) {
463-
throw new IllegalArgumentException("-source requires an argument");
464-
}
465-
sourceVersion = parseSourceVersion(it.next());
466-
break;
467-
default:
468-
break;
469-
}
470-
}
471-
return sourceVersion;
472-
}
473-
474-
private static SourceVersion parseSourceVersion(String value) {
475-
boolean hasPrefix = value.startsWith("1.");
476-
Integer version = Ints.tryParse(hasPrefix ? value.substring("1.".length()) : value);
477-
if (version == null || !isValidSourceVersion(version, hasPrefix)) {
478-
throw new IllegalArgumentException("invalid -source version: " + value);
479-
}
480-
try {
481-
return SourceVersion.valueOf("RELEASE_" + version);
482-
} catch (IllegalArgumentException unused) {
483-
throw new IllegalArgumentException("invalid -source version: " + value);
484-
}
485-
}
486-
487-
private static boolean isValidSourceVersion(int version, boolean hasPrefix) {
488-
if (version < 5) {
489-
// the earliest source version supported by JDK 8 is Java 5
490-
return false;
491-
}
492-
if (hasPrefix && version > 10) {
493-
// javac supports legacy `1.*` version numbers for source versions up to Java 10
494-
return false;
495-
}
496-
return true;
497-
}
498-
499451
private static URL[] toUrls(ImmutableList<String> processorPath) throws MalformedURLException {
500452
URL[] urls = new URL[processorPath.size()];
501453
int i = 0;

‎java/com/google/turbine/bytecode/ClassFile.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
public class ClassFile {
3333

3434
private final int access;
35+
private final int majorVersion;
3536
private final String name;
3637
private final @Nullable String signature;
3738
private final @Nullable String superClass;
@@ -49,6 +50,7 @@ public class ClassFile {
4950

5051
public ClassFile(
5152
int access,
53+
int majorVersion,
5254
String name,
5355
@Nullable String signature,
5456
@Nullable String superClass,
@@ -64,6 +66,7 @@ public ClassFile(
6466
@Nullable RecordInfo record,
6567
@Nullable String transitiveJar) {
6668
this.access = access;
69+
this.majorVersion = majorVersion;
6770
this.name = name;
6871
this.signature = signature;
6972
this.superClass = superClass;
@@ -85,6 +88,11 @@ public int access() {
8588
return access;
8689
}
8790

91+
/** Class file major version. */
92+
public int majorVersion() {
93+
return majorVersion;
94+
}
95+
8896
/** The name of the class or interface. */
8997
public String name() {
9098
return name;

‎java/com/google/turbine/bytecode/ClassReader.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,7 @@ private ClassFile read() {
138138

139139
return new ClassFile(
140140
accessFlags,
141+
majorVersion,
141142
thisClass,
142143
signature,
143144
superClass,

‎java/com/google/turbine/bytecode/ClassWriter.java‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -115,25 +115,11 @@ private static byte[] finishClass(
115115
ByteArrayDataOutput result = ByteStreams.newDataOutput();
116116
result.writeInt(MAGIC);
117117
result.writeShort(MINOR_VERSION);
118-
result.writeShort(majorVersion(classfile));
118+
result.writeShort(classfile.majorVersion());
119119
writeConstantPool(pool, result);
120120
result.write(body.toByteArray());
121121
return result.toByteArray();
122122
}
123123

124-
// use the lowest classfile version possible given the class file features
125-
// TODO(cushon): is there a reason to support --release?
126-
private static int majorVersion(ClassFile classfile) {
127-
if (classfile.nestHost() != null
128-
|| !classfile.nestMembers().isEmpty()
129-
|| classfile.record() != null) {
130-
return 60;
131-
}
132-
if (classfile.module() != null) {
133-
return 53;
134-
}
135-
return 52;
136-
}
137-
138124
private ClassWriter() {}
139125
}

‎java/com/google/turbine/deps/Transitive.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ public static ClassFile trimClass(ClassFile cf, @Nullable String jarFile) {
9090
}
9191
return new ClassFile(
9292
cf.access(),
93+
cf.majorVersion(),
9394
cf.name(),
9495
cf.signature(),
9596
cf.superName(),

‎java/com/google/turbine/lower/Lower.java‎

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@
6767
import com.google.turbine.model.TurbineFlag;
6868
import com.google.turbine.model.TurbineTyKind;
6969
import com.google.turbine.model.TurbineVisibility;
70+
import com.google.turbine.options.LanguageVersion;
7071
import com.google.turbine.type.AnnoInfo;
7172
import com.google.turbine.type.Type;
7273
import com.google.turbine.type.Type.ArrayTy;
@@ -112,24 +113,28 @@ public ImmutableSet<ClassSymbol> symbols() {
112113

113114
/** Lowers all given classes to bytecode. */
114115
public static Lowered lowerAll(
116+
LanguageVersion languageVersion,
115117
ImmutableMap<ClassSymbol, SourceTypeBoundClass> units,
116118
ImmutableList<SourceModuleInfo> modules,
117119
Env<ClassSymbol, BytecodeBoundClass> classpath) {
118120
CompoundEnv<ClassSymbol, TypeBoundClass> env =
119121
CompoundEnv.<ClassSymbol, TypeBoundClass>of(classpath).append(new SimpleEnv<>(units));
120122
ImmutableMap.Builder<String, byte[]> result = ImmutableMap.builder();
121123
Set<ClassSymbol> symbols = new LinkedHashSet<>();
124+
int majorVersion = languageVersion.majorVersion();
122125
for (ClassSymbol sym : units.keySet()) {
123-
result.put(sym.binaryName(), lower(units.get(sym), env, sym, symbols));
126+
result.put(sym.binaryName(), lower(units.get(sym), env, sym, symbols, majorVersion));
124127
}
125128
if (modules.size() == 1) {
126129
// single module mode: the module-info.class file is at the root
127-
result.put("module-info", lower(getOnlyElement(modules), env, symbols));
130+
result.put("module-info", lower(getOnlyElement(modules), env, symbols, majorVersion));
128131
} else {
129132
// multi-module mode: the output module-info.class are in a directory corresponding to their
130133
// package
131134
for (SourceModuleInfo module : modules) {
132-
result.put(module.name().replace('.', '/') + "/module-info", lower(module, env, symbols));
135+
result.put(
136+
module.name().replace('.', '/') + "/module-info",
137+
lower(module, env, symbols, majorVersion));
133138
}
134139
}
135140
return new Lowered(result.build(), ImmutableSet.copyOf(symbols));
@@ -140,15 +145,17 @@ public static byte[] lower(
140145
SourceTypeBoundClass info,
141146
Env<ClassSymbol, TypeBoundClass> env,
142147
ClassSymbol sym,
143-
Set<ClassSymbol> symbols) {
144-
return new Lower(env).lower(info, sym, symbols);
148+
Set<ClassSymbol> symbols,
149+
int majorVersion) {
150+
return new Lower(env).lower(info, sym, symbols, majorVersion);
145151
}
146152

147153
private static byte[] lower(
148154
SourceModuleInfo module,
149155
CompoundEnv<ClassSymbol, TypeBoundClass> env,
150-
Set<ClassSymbol> symbols) {
151-
return new Lower(env).lower(module, symbols);
156+
Set<ClassSymbol> symbols,
157+
int majorVersion) {
158+
return new Lower(env).lower(module, symbols, majorVersion);
152159
}
153160

154161
private final LowerSignature sig = new LowerSignature();
@@ -158,7 +165,7 @@ public Lower(Env<ClassSymbol, TypeBoundClass> env) {
158165
this.env = env;
159166
}
160167

161-
private byte[] lower(SourceModuleInfo module, Set<ClassSymbol> symbols) {
168+
private byte[] lower(SourceModuleInfo module, Set<ClassSymbol> symbols, int majorVersion) {
162169
String name = "module-info";
163170
ImmutableList<AnnotationInfo> annotations = lowerAnnotations(module.annos());
164171
ClassFile.ModuleInfo moduleInfo = lowerModule(module);
@@ -177,6 +184,7 @@ private byte[] lower(SourceModuleInfo module, Set<ClassSymbol> symbols) {
177184
ClassFile classfile =
178185
new ClassFile(
179186
/* access= */ TurbineFlag.ACC_MODULE,
187+
majorVersion,
180188
name,
181189
/* signature= */ null,
182190
/* superClass= */ null,
@@ -238,7 +246,8 @@ private ClassFile.ModuleInfo lowerModule(SourceModuleInfo module) {
238246
provides.build());
239247
}
240248

241-
private byte[] lower(SourceTypeBoundClass info, ClassSymbol sym, Set<ClassSymbol> symbols) {
249+
private byte[] lower(
250+
SourceTypeBoundClass info, ClassSymbol sym, Set<ClassSymbol> symbols, int majorVersion) {
242251
int access = classAccess(info);
243252
String name = sig.descriptor(sym);
244253
String signature = sig.classSignature(info, env);
@@ -275,6 +284,7 @@ private byte[] lower(SourceTypeBoundClass info, ClassSymbol sym, Set<ClassSymbol
275284
ClassFile classfile =
276285
new ClassFile(
277286
access,
287+
majorVersion,
278288
name,
279289
signature,
280290
superName,

‎java/com/google/turbine/main/Main.java‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import java.util.Collection;
6464
import java.util.Map;
6565
import java.util.Optional;
66+
import java.util.OptionalInt;
6667
import java.util.jar.Attributes;
6768
import java.util.jar.JarEntry;
6869
import java.util.jar.JarFile;
@@ -193,7 +194,9 @@ public static Result compile(TurbineOptions options) throws IOException {
193194
|| options.output().isPresent()
194195
|| options.outputManifest().isPresent()) {
195196
// TODO(cushon): parallelize
196-
Lowered lowered = Lower.lowerAll(bound.units(), bound.modules(), bound.classPathEnv());
197+
Lowered lowered =
198+
Lower.lowerAll(
199+
options.languageVersion(), bound.units(), bound.modules(), bound.classPathEnv());
197200

198201
if (options.outputDeps().isPresent()) {
199202
DepsProto.Dependencies deps =
@@ -258,6 +261,7 @@ private static BindingResult bind(
258261
units,
259262
ClassPathBinder.bindClasspath(toPaths(classpath)),
260263
Processing.initializeProcessors(
264+
/* sourceVersion= */ options.languageVersion().sourceVersion(),
261265
/* javacopts= */ options.javacOpts(),
262266
/* processorNames= */ options.processors(),
263267
Processing.processorLoader(
@@ -281,18 +285,18 @@ private static void usage(TurbineOptions options) {
281285

282286
private static ClassPath bootclasspath(TurbineOptions options) throws IOException {
283287
// if both --release and --bootclasspath are specified, --release wins
284-
if (options.release().isPresent() && options.system().isPresent()) {
288+
OptionalInt release = options.languageVersion().release();
289+
if (release.isPresent() && options.system().isPresent()) {
285290
throw new UsageException("expected at most one of --release and --system");
286291
}
287292

288-
if (options.release().isPresent()) {
289-
String release = options.release().get();
290-
if (release.equals(JAVA_SPECIFICATION_VERSION.value())) {
293+
if (release.isPresent()) {
294+
if (release.getAsInt() == Integer.parseInt(JAVA_SPECIFICATION_VERSION.value())) {
291295
// if --release matches the host JDK, use its jimage instead of ct.sym
292296
return JimageClassBinder.bindDefault();
293297
}
294298
// ... otherwise, search ct.sym for a matching release
295-
ClassPath bootclasspath = CtSymClassBinder.bind(release);
299+
ClassPath bootclasspath = CtSymClassBinder.bind(release.getAsInt());
296300
if (bootclasspath == null) {
297301
throw new UsageException("not a supported release: " + release);
298302
}

0 commit comments

Comments
 (0)