Skip to content

Commit

Permalink
Improve major version handling
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@cushon
cushon authored and Javac Team committed Sep 23, 2021
1 parent 6b0f9e9 commit 006a74b
Show file tree
Hide file tree
Showing 22 changed files with 397 additions and 236 deletions.
10 changes: 4 additions & 6 deletions java/com/google/turbine/binder/CtSymClassBinder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
import com.google.common.base.Supplier;
import com.google.common.base.Suppliers;
import com.google.common.collect.ImmutableMap;
import com.google.common.primitives.Ints;
import com.google.turbine.binder.bound.ModuleInfo;
import com.google.turbine.binder.bytecode.BytecodeBinder;
import com.google.turbine.binder.bytecode.BytecodeBoundClass;
Expand All @@ -47,7 +46,7 @@
/** Constructs a platform {@link ClassPath} from the current JDK's ct.sym file. */
public final class CtSymClassBinder {

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

@VisibleForTesting
static String formatReleaseVersion(String version) {
Integer n = Ints.tryParse(version);
if (n == null || n <= 4 || n >= 36) {
throw new IllegalArgumentException("invalid release version: " + version);
static String formatReleaseVersion(int n) {
if (n <= 4 || n >= 36) {
throw new IllegalArgumentException("invalid release version: " + n);
}
return toUpperCase(Integer.toString(n, 36));
}
Expand Down
50 changes: 1 addition & 49 deletions java/com/google/turbine/binder/Processing.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import static java.util.Objects.requireNonNull;

import com.google.auto.value.AutoValue;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Function;
import com.google.common.base.Joiner;
import com.google.common.base.Stopwatch;
Expand All @@ -30,7 +29,6 @@
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
import com.google.common.collect.Sets;
import com.google.common.primitives.Ints;
import com.google.turbine.binder.Binder.BindingResult;
import com.google.turbine.binder.Binder.Statistics;
import com.google.turbine.binder.bound.SourceTypeBoundClass;
Expand Down Expand Up @@ -61,7 +59,6 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
Expand Down Expand Up @@ -392,6 +389,7 @@ private static void addAnno(
}

public static ProcessorInfo initializeProcessors(
SourceVersion sourceVersion,
ImmutableList<String> javacopts,
ImmutableSet<String> processorNames,
ClassLoader processorLoader) {
Expand All @@ -400,7 +398,6 @@ public static ProcessorInfo initializeProcessors(
}
ImmutableList<Processor> processors = instantiateProcessors(processorNames, processorLoader);
ImmutableMap<String, String> processorOptions = processorOptions(javacopts);
SourceVersion sourceVersion = parseSourceVersion(javacopts);
return ProcessorInfo.create(processors, processorLoader, processorOptions, sourceVersion);
}

Expand Down Expand Up @@ -451,51 +448,6 @@ protected Class<?> findClass(String name) throws ClassNotFoundException {
});
}

@VisibleForTesting
static SourceVersion parseSourceVersion(ImmutableList<String> javacopts) {
SourceVersion sourceVersion = SourceVersion.latestSupported();
Iterator<String> it = javacopts.iterator();
while (it.hasNext()) {
String option = it.next();
switch (option) {
case "-source":
if (!it.hasNext()) {
throw new IllegalArgumentException("-source requires an argument");
}
sourceVersion = parseSourceVersion(it.next());
break;
default:
break;
}
}
return sourceVersion;
}

private static SourceVersion parseSourceVersion(String value) {
boolean hasPrefix = value.startsWith("1.");
Integer version = Ints.tryParse(hasPrefix ? value.substring("1.".length()) : value);
if (version == null || !isValidSourceVersion(version, hasPrefix)) {
throw new IllegalArgumentException("invalid -source version: " + value);
}
try {
return SourceVersion.valueOf("RELEASE_" + version);
} catch (IllegalArgumentException unused) {
throw new IllegalArgumentException("invalid -source version: " + value);
}
}

private static boolean isValidSourceVersion(int version, boolean hasPrefix) {
if (version < 5) {
// the earliest source version supported by JDK 8 is Java 5
return false;
}
if (hasPrefix && version > 10) {
// javac supports legacy `1.*` version numbers for source versions up to Java 10
return false;
}
return true;
}

private static URL[] toUrls(ImmutableList<String> processorPath) throws MalformedURLException {
URL[] urls = new URL[processorPath.size()];
int i = 0;
Expand Down
8 changes: 8 additions & 0 deletions java/com/google/turbine/bytecode/ClassFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
public class ClassFile {

private final int access;
private final int majorVersion;
private final String name;
private final @Nullable String signature;
private final @Nullable String superClass;
Expand All @@ -49,6 +50,7 @@ public class ClassFile {

public ClassFile(
int access,
int majorVersion,
String name,
@Nullable String signature,
@Nullable String superClass,
Expand All @@ -64,6 +66,7 @@ public ClassFile(
@Nullable RecordInfo record,
@Nullable String transitiveJar) {
this.access = access;
this.majorVersion = majorVersion;
this.name = name;
this.signature = signature;
this.superClass = superClass;
Expand All @@ -85,6 +88,11 @@ public int access() {
return access;
}

/** Class file major version. */
public int majorVersion() {
return majorVersion;
}

/** The name of the class or interface. */
public String name() {
return name;
Expand Down
1 change: 1 addition & 0 deletions java/com/google/turbine/bytecode/ClassReader.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ private ClassFile read() {

return new ClassFile(
accessFlags,
majorVersion,
thisClass,
signature,
superClass,
Expand Down
16 changes: 1 addition & 15 deletions java/com/google/turbine/bytecode/ClassWriter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,25 +115,11 @@ private static byte[] finishClass(
ByteArrayDataOutput result = ByteStreams.newDataOutput();
result.writeInt(MAGIC);
result.writeShort(MINOR_VERSION);
result.writeShort(majorVersion(classfile));
result.writeShort(classfile.majorVersion());
writeConstantPool(pool, result);
result.write(body.toByteArray());
return result.toByteArray();
}

// use the lowest classfile version possible given the class file features
// TODO(cushon): is there a reason to support --release?
private static int majorVersion(ClassFile classfile) {
if (classfile.nestHost() != null
|| !classfile.nestMembers().isEmpty()
|| classfile.record() != null) {
return 60;
}
if (classfile.module() != null) {
return 53;
}
return 52;
}

private ClassWriter() {}
}
1 change: 1 addition & 0 deletions java/com/google/turbine/deps/Transitive.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public static ClassFile trimClass(ClassFile cf, @Nullable String jarFile) {
}
return new ClassFile(
cf.access(),
cf.majorVersion(),
cf.name(),
cf.signature(),
cf.superName(),
Expand Down
28 changes: 19 additions & 9 deletions java/com/google/turbine/lower/Lower.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@
import com.google.turbine.model.TurbineFlag;
import com.google.turbine.model.TurbineTyKind;
import com.google.turbine.model.TurbineVisibility;
import com.google.turbine.options.LanguageVersion;
import com.google.turbine.type.AnnoInfo;
import com.google.turbine.type.Type;
import com.google.turbine.type.Type.ArrayTy;
Expand Down Expand Up @@ -112,24 +113,28 @@ public ImmutableSet<ClassSymbol> symbols() {

/** Lowers all given classes to bytecode. */
public static Lowered lowerAll(
LanguageVersion languageVersion,
ImmutableMap<ClassSymbol, SourceTypeBoundClass> units,
ImmutableList<SourceModuleInfo> modules,
Env<ClassSymbol, BytecodeBoundClass> classpath) {
CompoundEnv<ClassSymbol, TypeBoundClass> env =
CompoundEnv.<ClassSymbol, TypeBoundClass>of(classpath).append(new SimpleEnv<>(units));
ImmutableMap.Builder<String, byte[]> result = ImmutableMap.builder();
Set<ClassSymbol> symbols = new LinkedHashSet<>();
int majorVersion = languageVersion.majorVersion();
for (ClassSymbol sym : units.keySet()) {
result.put(sym.binaryName(), lower(units.get(sym), env, sym, symbols));
result.put(sym.binaryName(), lower(units.get(sym), env, sym, symbols, majorVersion));
}
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));
result.put("module-info", lower(getOnlyElement(modules), env, 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));
result.put(
module.name().replace('.', '/') + "/module-info",
lower(module, env, symbols, majorVersion));
}
}
return new Lowered(result.build(), ImmutableSet.copyOf(symbols));
Expand All @@ -140,15 +145,17 @@ public static byte[] lower(
SourceTypeBoundClass info,
Env<ClassSymbol, TypeBoundClass> env,
ClassSymbol sym,
Set<ClassSymbol> symbols) {
return new Lower(env).lower(info, sym, symbols);
Set<ClassSymbol> symbols,
int majorVersion) {
return new Lower(env).lower(info, sym, symbols, majorVersion);
}

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

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

private byte[] lower(SourceModuleInfo module, Set<ClassSymbol> symbols) {
private byte[] lower(SourceModuleInfo module, Set<ClassSymbol> symbols, int majorVersion) {
String name = "module-info";
ImmutableList<AnnotationInfo> annotations = lowerAnnotations(module.annos());
ClassFile.ModuleInfo moduleInfo = lowerModule(module);
Expand All @@ -177,6 +184,7 @@ private byte[] lower(SourceModuleInfo module, Set<ClassSymbol> symbols) {
ClassFile classfile =
new ClassFile(
/* access= */ TurbineFlag.ACC_MODULE,
majorVersion,
name,
/* signature= */ null,
/* superClass= */ null,
Expand Down Expand Up @@ -238,7 +246,8 @@ private ClassFile.ModuleInfo lowerModule(SourceModuleInfo module) {
provides.build());
}

private byte[] lower(SourceTypeBoundClass info, ClassSymbol sym, Set<ClassSymbol> symbols) {
private byte[] lower(
SourceTypeBoundClass info, ClassSymbol sym, Set<ClassSymbol> symbols, int majorVersion) {
int access = classAccess(info);
String name = sig.descriptor(sym);
String signature = sig.classSignature(info, env);
Expand Down Expand Up @@ -275,6 +284,7 @@ private byte[] lower(SourceTypeBoundClass info, ClassSymbol sym, Set<ClassSymbol
ClassFile classfile =
new ClassFile(
access,
majorVersion,
name,
signature,
superName,
Expand Down
16 changes: 10 additions & 6 deletions java/com/google/turbine/main/Main.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import java.util.Collection;
import java.util.Map;
import java.util.Optional;
import java.util.OptionalInt;
import java.util.jar.Attributes;
import java.util.jar.JarEntry;
import java.util.jar.JarFile;
Expand Down Expand Up @@ -193,7 +194,9 @@ public static Result compile(TurbineOptions options) throws IOException {
|| options.output().isPresent()
|| options.outputManifest().isPresent()) {
// TODO(cushon): parallelize
Lowered lowered = Lower.lowerAll(bound.units(), bound.modules(), bound.classPathEnv());
Lowered lowered =
Lower.lowerAll(
options.languageVersion(), bound.units(), bound.modules(), bound.classPathEnv());

if (options.outputDeps().isPresent()) {
DepsProto.Dependencies deps =
Expand Down Expand Up @@ -258,6 +261,7 @@ private static BindingResult bind(
units,
ClassPathBinder.bindClasspath(toPaths(classpath)),
Processing.initializeProcessors(
/* sourceVersion= */ options.languageVersion().sourceVersion(),
/* javacopts= */ options.javacOpts(),
/* processorNames= */ options.processors(),
Processing.processorLoader(
Expand All @@ -281,18 +285,18 @@ private static void usage(TurbineOptions options) {

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

if (options.release().isPresent()) {
String release = options.release().get();
if (release.equals(JAVA_SPECIFICATION_VERSION.value())) {
if (release.isPresent()) {
if (release.getAsInt() == Integer.parseInt(JAVA_SPECIFICATION_VERSION.value())) {
// if --release matches the host JDK, use its jimage instead of ct.sym
return JimageClassBinder.bindDefault();
}
// ... otherwise, search ct.sym for a matching release
ClassPath bootclasspath = CtSymClassBinder.bind(release);
ClassPath bootclasspath = CtSymClassBinder.bind(release.getAsInt());
if (bootclasspath == null) {
throw new UsageException("not a supported release: " + release);
}
Expand Down
Loading

0 comments on commit 006a74b

Please sign in to comment.