Flaw in generic mapping

[Chocohead]

Synthetic bridges methods, most commonly seen from implementing generic supertypes, are not logically mapped to be deobfuscated. Taking azb$a => class_1867$class_1868 => ShapelessRecipe$Serializer (which extendsayy => class_1862 => RecipeSerializer) as an example:

• We start with explicit bridge a(Lit;Layw;)V for a(Lit;Lazb;)V
• Intermediary skips mapping the bridge (as it overrides class_1862#method_8124) and maps the actual method to method_8143
• Mapping method_8124 (as Yarn does) carries over to the mapping (here)
• The actual method remains unmapped and is left as method_8143

This process is masked from decompiling hiding bridge methods by default, but the same effect is visibly present from other methods:

• We start with implicit (ie only synthetic) bridge b(Lqc;Lit;)Layw; for a(Lqc;Lit;)Lazb;
• Intermediary skips mapping the bridge (method_8122) and maps the actual method to method_8141
• As before the naming is carried forward
• The actual method remains unmapped, resulting in a mapped read bouncing to an unmapped method_8141

Practically speaking as it stands nothing appears to break from this (after all, Mojang uses different names of a and b), but hiding the synthetic methods as decompilers often do results in classes appearing to not consistently implement interfaces.
This also leaves the door open for the implementing method name to differ from the interface it is implementing, purely as they have different Intermediary names and nothing forces them to be the same. Enigma will happily let you do this as to it they are different methods. Whilst again this isn't technically wrong from the JVM's point of view, you could never write Java code that would naturally compile like that (as well as the decompile having no chance of recompiling).

If the actual methods that are bridged to shared the same Intermediary name as the bridges (and thus the original interface) this would go away as the whole system would change together. Doesn't fix Yarn's problems from mapping to Notch directly, but that is arguably an Enigma problem.

[asiekierka]

An internal Stitch branch now performs bridge lookup.

The question is: Should this change be introduced on the 19w update? Does it have the potential to break any existing mod?

[Chocohead]

If you take the simplest example of calling the methods:

Serializer testA = new Serializer();
testA.read(new Identifier(""), new JsonObject());
testA.method_8142(new Identifier(""), new JsonObject());

The first call will get obfuscated back to the interface's name (method_8121) whilst the second will keep it's naming. Thus the second direct call would break but the first abstract one would be fine.

This is only a problem if the concrete types have been used too:

RecipeSerializer<ShapelessRecipe> testB = new Serializer();
testB.read(new Identifier(""), new JsonObject());

Would get obfuscated to method_8121 and thus would carry on working fine. It is most likely people have been using the abstract methods as the concrete ones appear to lack any non-Intermediary names.

As for actually extending the types, due to the clunky naming Java already doesn't see them as actually implementing the interfaces:

public class Test extends Serializer {

	@Override
	public ShapelessRecipe read(Identifier var1, JsonObject var2) {
		// TODO Auto-generated method stub
		return null;
	}

	@Override
	public ShapelessRecipe read(Identifier var1, PacketByteBuf var2) {
		// TODO Auto-generated method stub
		return null;
	}

	@Override
	public void write(PacketByteBuf var1, ShapelessRecipe var2) {
		// TODO Auto-generated method stub

	}

Is what you get when Eclipse automatically fills the missing methods, ie the interface's names with the proper generic signatures. The generated bridges override correctly too, so the only issue people would have is if they called the concrete methods directly rather than using the interface's abstract methods.

[asiekierka]

I think its best to stall this for the first 19w update, or hopefully an 18w50b (though unlikely).

[asiekierka]

FabricMC/stitch@2e001d7

Implemented. Awaiting snapshot.

[asiekierka]

Not done, see discussion in #2

[valoeghese]

Wow this hasn't been touched since 2018 lol