Skip to content

Commit

Permalink
Make compat warnings less spammy but more verbose. Add MAX_SUPPORTED.
Browse files Browse the repository at this point in the history
  • Loading branch information
Mumfrey committed Jun 30, 2021
1 parent 07d2c8c commit 4827207
Show file tree
Hide file tree
Showing 3 changed files with 111 additions and 24 deletions.
49 changes: 43 additions & 6 deletions src/main/java/org/spongepowered/asm/mixin/MixinEnvironment.java
Original file line number Diff line number Diff line change
Expand Up @@ -663,6 +663,28 @@ boolean isSupported() {
*/
public static CompatibilityLevel DEFAULT = CompatibilityLevel.JAVA_6;

/**
* Maximum compatibility level actually supported. Other compatibility
* levels might exist but we don't actually have any internal code in
* place which supports those features. This is mainly used to indicate
* that mixin classes compiled with newer JDKs might have bytecode-level
* class features that this version of mixin doesn't understand, even
* when the current ASM or JRE do.
*
* <p>This is particularly important for the case where a config
* declares a higher version (eg. JAVA_14) which has been added to the
* enum but no code actually exists within Mixin as a library to handle
* language features from that version. In other words adding values to
* this enum doesn't magically add support for language features, and
* this field should point to the highest <em>known <b>supported</b>
* </em> version regardless of other <em>known</em> versions.</p>
*
* <p>This comment mainly added to avoid stuff in the future like
* PR #500 which demonstrates that the nature of compatibility levels
* in mixin are not understood that well.</p>
*/
public static CompatibilityLevel MAX_SUPPORTED = CompatibilityLevel.JAVA_11;

private final int ver;

private final int classVersion;
Expand Down Expand Up @@ -808,13 +830,28 @@ public static CompatibilityLevel requiredFor(int languageFeatures) {
static String getSupportedVersions() {
StringBuilder sb = new StringBuilder();
boolean comma = false;
int rangeStart = 0, rangeEnd = 0;
for (CompatibilityLevel level : CompatibilityLevel.values()) {
if (level.isSupported()) {
if (comma) {
sb.append(", ");
if (level.ver == rangeEnd + 1) {
rangeEnd = level.ver;
} else {
if (rangeStart > 0) {
sb.append(comma ? "," : "").append(rangeStart);
if (rangeEnd > rangeStart) {
sb.append(rangeEnd > rangeStart + 1 ? '-' : ',').append(rangeEnd);
}
comma = true;
rangeStart = rangeEnd = level.ver;
}
rangeStart = rangeEnd = level.ver;
}
sb.append(level.ver);
comma = true;
}
}
if (rangeStart > 0) {
sb.append(comma ? "," : "").append(rangeStart);
if (rangeEnd > rangeStart) {
sb.append(rangeEnd > rangeStart + 1 ? '-' : ',').append(rangeEnd);
}
}
return sb.toString();
Expand Down Expand Up @@ -998,8 +1035,8 @@ private void printHeader(Object version) {
printer.add("SpongePowered MIXIN%s", verbose ? " (Verbose debugging enabled)" : "").centre().hr();
printer.kv("Code source", codeSource);
printer.kv("Internal Version", version);
printer.kv("Java Versions Supported", CompatibilityLevel.getSupportedVersions());
printer.kv("Current Compatibility Level", MixinEnvironment.getCompatibilityLevel());
printer.kv("Java Version", "%s (supports compatibility %s)", JavaVersion.current(), CompatibilityLevel.getSupportedVersions());
printer.kv("Default Compatibility Level", MixinEnvironment.getCompatibilityLevel());
printer.kv("Detected ASM API Version", ASM.getApiVersionString());
printer.kv("Detected ASM Supports Java", ASM.getClassVersionString()).hr();
printer.kv("Service Name", serviceName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,17 @@ interface IListener {
* Track whether this mixin has been evaluated for selection yet
*/
private transient boolean visited = false;

/**
* Compatibility level read from the config (or default if none specified)
*/
private transient CompatibilityLevel compatibilityLevel = CompatibilityLevel.DEFAULT;

/**
* Only emit the compatibility level warning for any increase in the class
* version, track warned level here
*/
private transient int warnedClassVersion = 0;

/**
* Spawn via GSON, no public ctor for you
Expand Down Expand Up @@ -397,6 +408,7 @@ private boolean onLoad(IMixinService service, String name, MixinEnvironment fall

// If no parent, initialise config options
this.env = this.parseSelector(this.selector, fallbackEnvironment);
this.verboseLogging |= this.env.getOption(Option.DEBUG_VERBOSE);
this.required = this.requiredValue != null && this.requiredValue.booleanValue() && !this.env.getOption(Option.IGNORE_REQUIRED);
this.initPriority(IMixinConfig.DEFAULT_PRIORITY, IMixinConfig.DEFAULT_PRIORITY);

Expand Down Expand Up @@ -440,6 +452,7 @@ boolean assignParent(Config parentConfig) {
}

this.env = this.parseSelector(this.selector, this.parent.env);
this.verboseLogging |= this.env.getOption(Option.DEBUG_VERBOSE);
this.required = this.requiredValue == null ? this.parent.required
: this.requiredValue.booleanValue() && !this.env.getOption(Option.IGNORE_REQUIRED);

Expand Down Expand Up @@ -486,31 +499,74 @@ private boolean postInit() throws MixinInitialisationError {

@SuppressWarnings("deprecation")
private void initCompatibilityLevel() {
this.compatibilityLevel = MixinEnvironment.getCompatibilityLevel();

if (this.compatibility == null) {
return;
}

CompatibilityLevel level = CompatibilityLevel.valueOf(this.compatibility.trim().toUpperCase(Locale.ROOT));
CompatibilityLevel current = MixinEnvironment.getCompatibilityLevel();
String strCompatibility = this.compatibility.trim().toUpperCase(Locale.ROOT);
try {
this.compatibilityLevel = CompatibilityLevel.valueOf(strCompatibility);
} catch (IllegalArgumentException ex) {
throw new MixinInitialisationError(String.format("Mixin config %s specifies compatibility level %s which is not recognised",
this.name, strCompatibility));
}

if (level == current) {
CompatibilityLevel currentLevel = MixinEnvironment.getCompatibilityLevel();
if (this.compatibilityLevel == currentLevel) {
return;
}

// Current level is higher than required but too new to support it
if (current.isAtLeast(level)) {
if (!current.canSupport(level)) {
throw new MixinInitialisationError("Mixin config " + this.name + " requires compatibility level " + level + " which is too old");
}
if (currentLevel.isAtLeast(this.compatibilityLevel) && !currentLevel.canSupport(this.compatibilityLevel)) {
throw new MixinInitialisationError(String.format("Mixin config %s requires compatibility level %s which is too old",
this.name, this.compatibilityLevel));
}

// Current level is lower than required but current level prohibits elevation
if (!current.canElevateTo(level)) {
throw new MixinInitialisationError("Mixin config " + this.name + " requires compatibility level " + level + " which is prohibited by "
+ current);
if (!currentLevel.canElevateTo(this.compatibilityLevel)) {
throw new MixinInitialisationError(String.format("Mixin config %s requires compatibility level %s which is prohibited by %s",
this.name, this.compatibilityLevel, currentLevel));
}

// Required level is higher than highest version we support, this possibly
// means that a shaded mixin dependency has been usurped by an old version,
// or the mixin author is trying to elevate the compatibility level beyond
// the versions currently supported
if (CompatibilityLevel.MAX_SUPPORTED.isLessThan(this.compatibilityLevel)) {
Level logLevel = this.verboseLogging ? Level.WARN : Level.DEBUG;
this.logger.log(logLevel,
"Compatibility level {} specified by {} is higher than the maximum level supported by this version of mixin ({}).",
this.compatibilityLevel, this, CompatibilityLevel.MAX_SUPPORTED);
}

MixinEnvironment.setCompatibilityLevel(this.compatibilityLevel);
}

/**
* Called by MixinTargetContext when class version is elevated, allows us to
* warn devs (or end-users with verbose turned on, for whatever reason) that
* the current compatibility level is too low for the classes being
* processed. The warning is only emitted at WARN for each new class version
* and at DEBUG thereafter.
*
* <p>The logic here is that we only really care about supported class
* features, but a version of mixin which doesn't actually support newer
* features may well be able to operate with classes *compiled* with a newer
* JDK, but we don't actually know that for sure).
*/
void checkCompatibilityLevel(MixinInfo mixin, int majorVersion, int minorVersion) {
if (majorVersion <= this.compatibilityLevel.getClassMajorVersion()) {
return;
}

MixinEnvironment.setCompatibilityLevel(level);
Level logLevel = this.verboseLogging && majorVersion > this.warnedClassVersion ? Level.WARN : Level.DEBUG;
String message = majorVersion > CompatibilityLevel.MAX_SUPPORTED.getClassMajorVersion()
? "the current version of Mixin" : "the declared compatibility level";
this.warnedClassVersion = majorVersion;
this.logger.log(logLevel, "{}: Class version {} required is higher than the class version supported by {} ({} supports class version {})",
mixin, majorVersion, message, this.compatibilityLevel, this.compatibilityLevel.getClassMajorVersion());
}

// AMS - temp
Expand Down Expand Up @@ -655,7 +711,6 @@ void onSelect() {
}

this.refMapper = ReferenceMapper.read(this.refMapperConfig);
this.verboseLogging |= this.env.getOption(Option.DEBUG_VERBOSE);

if (!suppressRefMapWarning && this.refMapper.isDefault() && !this.env.getOption(Option.DISABLE_REFMAP)) {
this.logger.warn("Reference map '{}' for {} could not be read. If this is a development environment you can ignore this message",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1088,12 +1088,7 @@ protected void requireVersion(int version) {
majorVersion, minorVersion, ASM.getClassVersionString()));
}

CompatibilityLevel compatibilityLevel = MixinEnvironment.getCompatibilityLevel();
if (majorVersion > compatibilityLevel.getClassMajorVersion()) {
MixinTargetContext.logger.warn(
"{}: Class version {}.{} required is higher than the class version supported by the current compatibility level {} ",
this, majorVersion, minorVersion, compatibilityLevel);
}
this.mixin.getParent().checkCompatibilityLevel(this.mixin, majorVersion, minorVersion);
}

/* (non-Javadoc)
Expand Down

0 comments on commit 4827207

Please sign in to comment.