Skip to content

[c2me-ocl fix] Fix remapping of mixin annotation targets #2208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
lap2ka wants to merge 1 commit into
base: 1.21.x
Choose a base branch
from
+49 −2
Draft

[c2me-ocl fix] Fix remapping of mixin annotation targets #2208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions transformer/src/main/java/org/sinytra/connector/transformer/jar/IntermediateMapping.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class IntermediateMapping {
private final Map<String, String> mappings;
// Original + Descriptor -> Mapped
private final Map<String, String> extendedMappings;
// Original -> Mapped name + original descriptor, when the original method name is unambiguous
private final Map<String, MethodMapping> methodMappings;

public static IntermediateMapping get(String sourceNamespace) {
IntermediateMapping existing = INTERMEDIATE_MAPPINGS_CACHE.get(sourceNamespace);
Expand All @@ -40,12 +42,25 @@ public static IntermediateMapping get(String sourceNamespace) {
Map<String, String> resolved = new HashMap<>();
Map<String, IMappingFile.INode> buffer = new HashMap<>();
Map<String, String> extendedMappings = new HashMap<>();
Map<String, MethodMapping> methodMappings = new HashMap<>();
Set<String> ambiguousMethods = new HashSet<>();
resolver.getCurrentMap(sourceNamespace).getClasses().stream()
.flatMap(cls -> Stream.concat(Stream.of(cls), Stream.concat(cls.getFields().stream(), cls.getMethods().stream()))
.filter(node -> prefixes.stream().anyMatch(node.getOriginal()::startsWith)))
.forEach(node -> {
String original = node.getOriginal();
String mapped = node.getMapped();
if (node instanceof IMappingFile.IMethod method && method.getDescriptor() != null) {
MethodMapping methodMapping = new MethodMapping(mapped, method.getDescriptor());
MethodMapping existingMethod = methodMappings.get(original);
if (existingMethod == null && !ambiguousMethods.contains(original)) {
methodMappings.put(original, methodMapping);
}
else if (!methodMapping.equals(existingMethod)) {
methodMappings.remove(original);
ambiguousMethods.add(original);
}
}
String mapping = resolved.get(original);
if (mapping != null && !mapping.equals(mapped)) {
resolved.remove(original);
Expand All @@ -57,7 +72,7 @@ else if (!extendedMappings.containsKey(getMappingKey(node))) {
buffer.put(original, node);
}
});
IntermediateMapping mapping = new IntermediateMapping(resolved, extendedMappings);
IntermediateMapping mapping = new IntermediateMapping(resolved, extendedMappings, methodMappings);
INTERMEDIATE_MAPPINGS_CACHE.put(sourceNamespace, mapping);
return mapping;
}
Expand All @@ -76,9 +91,10 @@ else if (node instanceof IMappingFile.IMethod method) {
return node.getOriginal();
}

public IntermediateMapping(Map<String, String> mappings, Map<String, String> extendedMappings) {
public IntermediateMapping(Map<String, String> mappings, Map<String, String> extendedMappings, Map<String, MethodMapping> methodMappings) {
this.mappings = mappings;
this.extendedMappings = extendedMappings;
this.methodMappings = methodMappings;
}

@Nullable
Expand All @@ -101,6 +117,11 @@ public String mapMethodOrDefault(String name, String desc) {
return mapped != null ? mapped : name;
}

@Nullable
public MethodMapping getMethodMapping(String name) {
return this.methodMappings.get(name);
}

@Nullable
public String mapMethod(String name, String desc) {
String mapped = this.mappings.get(name);
Expand All @@ -110,4 +131,7 @@ public String mapMethod(String name, String desc) {
}
return mapped;
}

public record MethodMapping(String mappedName, String descriptor) {
}
}
23 changes: 23 additions & 0 deletions ...c/main/java/org/sinytra/connector/transformer/transform/OptimizedRenamingTransformer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ protected void postProcess(ClassNode node) {
processMixinAnnotation(annotation, postProcessRemapper);
}
}
if (method.invisibleAnnotations != null) {
for (AnnotationNode annotation : method.invisibleAnnotations) {
processMixinAnnotation(annotation, postProcessRemapper);
}
}
for (AbstractInsnNode insn : method.instructions) {
if (insn instanceof LdcInsnNode ldc) {
ldc.cst = postProcessRemapper.mapValue(ldc.cst);
Expand Down Expand Up @@ -141,10 +146,17 @@ private void processMixinAnnotation(AnnotationNode annotation, PostProcessRemapp
});
// If remap has been set to false during compilation, we must manually map the annotation values ourselves instead of relying on the provided refmap
if (this.remapRefs || handle.<Boolean>getValue("remap").map(h -> !h.get()).orElse(false)) {
qualifyMixinMethodSelectors(handle, postProcessRemapper);
postProcessRemapper.mapAnnotationValues(annotation.values);
}
}

private static void qualifyMixinMethodSelectors(AnnotationHandle handle, PostProcessRemapper postProcessRemapper) {
handle.<List<String>>getValue("method")
.map(AnnotationValueHandle::get)
.ifPresent(methods -> methods.replaceAll(postProcessRemapper::qualifyMethodSelector));
}

private record PostProcessRemapper(IntermediateMapping flatMappings, Remapper remapper) {
public void mapAnnotationValues(List values) {
if (values != null) {
Expand Down Expand Up @@ -218,6 +230,17 @@ else if (str.matches(INTERNAL_CLASS_NAME_PATTERN)) {
}
return this.remapper.mapValue(value);
}

public String qualifyMethodSelector(String selector) {
MethodQualifier qualifier = MethodQualifier.parse(selector).orElse(null);
if (qualifier != null && qualifier.desc() == null && qualifier.name() != null) {
IntermediateMapping.MethodMapping methodMapping = this.flatMappings.getMethodMapping(qualifier.name());
if (methodMapping != null) {
return methodMapping.mappedName() + this.remapper.mapMethodDesc(methodMapping.descriptor());
}
}
return selector;
}
}

public static final class IntermediaryClassProvider implements ClassProvider {
Expand Down