Fields that shadow remappable fields get inadvertently remapped when referred to from a subclass

[quat1024]

Sry for the mouthful of a title, this is a pretty specific issue, also i produced this from inside ForgeGradle 3 and 5 and have been sent here https://github.com/MinecraftForge/ForgeGradle/issues/821 !
gonna put the original issue text below Okay

---

forgegradle-based test case is at https://github.com/quat1024/bugtest if you wanna try it

Easiest to explain by example, so here's the scene:

public abstract class Superclass extends DirectionalBlock {
  public Superclass(Properties props) {
    super(props);

    registerDefaultState(defaultBlockState()
      .setValue(FACING, Direction.UP)
      .setValue(POWERED, false));
  }

  public static final EnumProperty<Direction> FACING = DirectionalBlock.FACING; //shadows a field from superclass (!)
  public static final BooleanProperty POWERED = BlockStateProperties.POWERED; //for comparison's sake

  @Override
  protected void createBlockStateDefinition(StateContainer.Builder<Block, BlockState> builder) {
    builder.add(FACING, POWERED);
  }
}

//...

public class Subclass extends Superclass {
  public Subclass(Properties props) {
    super(props);
  }

  @Nullable
  @Override
  public BlockState getStateForPlacement(BlockItemUseContext ctx) {
    return defaultBlockState()
      .setValue(FACING, ctx.getClickedFace().getOpposite())
      .setValue(POWERED, false);
  }
}

The important parts:

• Superclass extends Minecraft's DirectionalBlock, and both classes have a field named FACING. Mine shadows theirs.
• References to DirectionalBlock.FACING should be remapped to DirectionalBlock.field_176387_N, but references to Superclass.FACING should stay the same.
• Subclass refers to that field.

I built a reobf jar using ForgeGradle's standard build task. One of its subtasks remaps the mod to SRG names using SpecialSource, and here's what i found:

• Superclass correctly contains two fields named FACING and POWERED,
• Superclass#createBlockStateDefinition correctly refers to the fields named FACING and POWERED,
• Subclass#getStateForPlacement incorrectly refers to fields named field_176387_N and POWERED instead.

This fails at runtime with a NoSuchFieldError, see https://github.com/quat1024/incorporeal-2-forge/issues/11.

👀 Only accesses from subclasses are incorrectly remapped. Referring to the public static field Superclass.FACING from an unrelated class uses the correct name of FACING

[md-5]

Are you using an inheritance map or live remap with Minecraft on the classpath? If you're not, that's probably why this happens as the bytecode for your classes does not tell us enough info to know how to remap.

This would be easiest to verify by doing a test case with the SpecialSource CLI as I'm not sure what Forgegradle does

[LexManos]

Just for note, cuz I was curious. The end bytecode reference is:

#28 Fieldref           #1.#29         // Test$Subclass.POWERED:Lnet/minecraft/world/level/block/state/properties/BooleanProperty;
getstatic     #28                 // Field POWERED:Lnet/minecraft/world/level/block/state/properties/BooleanProperty;

So it's looking for the static field on the subclass itself, which is odd to say the least...

FG passes in the full classpath and --live So it should be all that is needed.