Skip to content

Commit

Permalink
[epilogue] Generate unique names for variables used in instanceof cha…
Browse files Browse the repository at this point in the history 
…ins (#7323)

Fixes an issue where the variable names would clash with the field names of the associated VarHandles.
  • Loading branch information
@SamCarlberg
SamCarlberg authored Nov 3, 2024
1 parent a48f3c3 commit 44c0bbc
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 16 deletions.
4 changes: 2 additions & 2 deletions epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/LoggableHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,9 +111,9 @@ public String logInvocation(Element element) {
private static String cacheVariableName(Element element) {
// Generate unique names in case a field and a method share the same name
if (element instanceof VariableElement) {
return "$%s".formatted(element.getSimpleName().toString());
return "$$%s".formatted(element.getSimpleName().toString());
} else if (element instanceof ExecutableElement) {
return "_%s".formatted(element.getSimpleName().toString());
return "__%s".formatted(element.getSimpleName().toString());
} else {
// Generic fallback (shouldn't get here, since only fields and methods are logged)
return element.getSimpleName().toString();
Expand Down
91 changes: 77 additions & 14 deletions ...gue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1134,29 +1134,29 @@ public ExampleLogger() {
@Override
public void update(DataLogger dataLogger, Example object) {
if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
var $asInterface = object.asInterface;
if ($asInterface instanceof edu.wpi.first.epilogue.Impl1 edu_wpi_first_epilogue_Impl1) {
var $$asInterface = object.asInterface;
if ($$asInterface instanceof edu.wpi.first.epilogue.Impl1 edu_wpi_first_epilogue_Impl1) {
Epilogue.impl1Logger.tryUpdate(dataLogger.getSubLogger("asInterface"), edu_wpi_first_epilogue_Impl1, Epilogue.getConfig().errorHandler);
} else if ($asInterface instanceof edu.wpi.first.epilogue.Impl2 edu_wpi_first_epilogue_Impl2) {
} else if ($$asInterface instanceof edu.wpi.first.epilogue.Impl2 edu_wpi_first_epilogue_Impl2) {
Epilogue.impl2Logger.tryUpdate(dataLogger.getSubLogger("asInterface"), edu_wpi_first_epilogue_Impl2, Epilogue.getConfig().errorHandler);
} else {
// Base type edu.wpi.first.epilogue.IFace
Epilogue.iFaceLogger.tryUpdate(dataLogger.getSubLogger("asInterface"), $asInterface, Epilogue.getConfig().errorHandler);
Epilogue.iFaceLogger.tryUpdate(dataLogger.getSubLogger("asInterface"), $$asInterface, Epilogue.getConfig().errorHandler);
};
Epilogue.impl1Logger.tryUpdate(dataLogger.getSubLogger("firstImpl"), object.firstImpl, Epilogue.getConfig().errorHandler);
Epilogue.impl2Logger.tryUpdate(dataLogger.getSubLogger("secondImpl"), object.secondImpl, Epilogue.getConfig().errorHandler);
var $complex = object.complex;
if ($complex instanceof edu.wpi.first.epilogue.ConcreteLogged edu_wpi_first_epilogue_ConcreteLogged) {
var $$complex = object.complex;
if ($$complex instanceof edu.wpi.first.epilogue.ConcreteLogged edu_wpi_first_epilogue_ConcreteLogged) {
Epilogue.concreteLoggedLogger.tryUpdate(dataLogger.getSubLogger("complex"), edu_wpi_first_epilogue_ConcreteLogged, Epilogue.getConfig().errorHandler);
} else if ($complex instanceof edu.wpi.first.epilogue.I4 edu_wpi_first_epilogue_I4) {
} else if ($$complex instanceof edu.wpi.first.epilogue.I4 edu_wpi_first_epilogue_I4) {
Epilogue.i4Logger.tryUpdate(dataLogger.getSubLogger("complex"), edu_wpi_first_epilogue_I4, Epilogue.getConfig().errorHandler);
} else if ($complex instanceof edu.wpi.first.epilogue.I2 edu_wpi_first_epilogue_I2) {
} else if ($$complex instanceof edu.wpi.first.epilogue.I2 edu_wpi_first_epilogue_I2) {
Epilogue.i2Logger.tryUpdate(dataLogger.getSubLogger("complex"), edu_wpi_first_epilogue_I2, Epilogue.getConfig().errorHandler);
} else if ($complex instanceof edu.wpi.first.epilogue.I3 edu_wpi_first_epilogue_I3) {
} else if ($$complex instanceof edu.wpi.first.epilogue.I3 edu_wpi_first_epilogue_I3) {
Epilogue.i3Logger.tryUpdate(dataLogger.getSubLogger("complex"), edu_wpi_first_epilogue_I3, Epilogue.getConfig().errorHandler);
} else {
// Base type edu.wpi.first.epilogue.I
Epilogue.iLogger.tryUpdate(dataLogger.getSubLogger("complex"), $complex, Epilogue.getConfig().errorHandler);
Epilogue.iLogger.tryUpdate(dataLogger.getSubLogger("complex"), $$complex, Epilogue.getConfig().errorHandler);
};
}
}
Expand Down Expand Up @@ -1210,14 +1210,77 @@ public ExampleLogger() {
@Override
public void update(DataLogger dataLogger, Example object) {
if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
var $theField = object.theField;
if ($theField instanceof edu.wpi.first.epilogue.Base edu_wpi_first_epilogue_Base) {
var $$theField = object.theField;
if ($$theField instanceof edu.wpi.first.epilogue.Base edu_wpi_first_epilogue_Base) {
Epilogue.baseLogger.tryUpdate(dataLogger.getSubLogger("theField"), edu_wpi_first_epilogue_Base, Epilogue.getConfig().errorHandler);
} else if ($theField instanceof edu.wpi.first.epilogue.ExtendingInterface edu_wpi_first_epilogue_ExtendingInterface) {
} else if ($$theField instanceof edu.wpi.first.epilogue.ExtendingInterface edu_wpi_first_epilogue_ExtendingInterface) {
Epilogue.extendingInterfaceLogger.tryUpdate(dataLogger.getSubLogger("theField"), edu_wpi_first_epilogue_ExtendingInterface, Epilogue.getConfig().errorHandler);
} else {
// Base type edu.wpi.first.epilogue.I
Epilogue.iLogger.tryUpdate(dataLogger.getSubLogger("theField"), $theField, Epilogue.getConfig().errorHandler);
Epilogue.iLogger.tryUpdate(dataLogger.getSubLogger("theField"), $$theField, Epilogue.getConfig().errorHandler);
};
}
}
}
""";

assertLoggerGenerates(source, expectedRootLogger);
}

@Test
void instanceofChainWithField() {
String source =
"""
package edu.wpi.first.epilogue;
@Logged
interface I {}
@Logged
class Base implements I {}
@Logged
class Example {
private I theField;
}
""";

String expectedRootLogger =
"""
package edu.wpi.first.epilogue;
import edu.wpi.first.epilogue.Logged;
import edu.wpi.first.epilogue.Epilogue;
import edu.wpi.first.epilogue.logging.ClassSpecificLogger;
import edu.wpi.first.epilogue.logging.DataLogger;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.VarHandle;
public class ExampleLogger extends ClassSpecificLogger<Example> {
private static final VarHandle $theField;
static {
try {
var lookup = MethodHandles.privateLookupIn(Example.class, MethodHandles.lookup());
$theField = lookup.findVarHandle(Example.class, "theField", edu.wpi.first.epilogue.I.class);
} catch (ReflectiveOperationException e) {
throw new RuntimeException("[EPILOGUE] Could not load private fields for logging!", e);
}
}
public ExampleLogger() {
super(Example.class);
}
@Override
public void update(DataLogger dataLogger, Example object) {
if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
var $$theField = (edu.wpi.first.epilogue.I) $theField.get(object);
if ($$theField instanceof edu.wpi.first.epilogue.Base edu_wpi_first_epilogue_Base) {
Epilogue.baseLogger.tryUpdate(dataLogger.getSubLogger("theField"), edu_wpi_first_epilogue_Base, Epilogue.getConfig().errorHandler);
} else {
// Base type edu.wpi.first.epilogue.I
Epilogue.iLogger.tryUpdate(dataLogger.getSubLogger("theField"), $$theField, Epilogue.getConfig().errorHandler);
};
}
}
Expand Down

0 comments on commit 44c0bbc

Please sign in to comment.