From 75f13b5a144fc9df066a6cbe5c47fbd26d75d9d4 Mon Sep 17 00:00:00 2001 From: shueja <32416547+shueja@users.noreply.github.com> Date: Fri, 22 Nov 2024 22:04:23 -0800 Subject: [PATCH 1/5] [epilogue] Add extra parentheses around cast when using varhandle --- .../edu/wpi/first/epilogue/processor/ElementHandler.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/ElementHandler.java b/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/ElementHandler.java index 894225046f8..b3ccee6f80a 100644 --- a/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/ElementHandler.java +++ b/epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/ElementHandler.java @@ -127,8 +127,10 @@ public String elementAccess(Element element) { private static String fieldAccess(VariableElement field) { if (field.getModifiers().contains(Modifier.PRIVATE)) { - // (com.example.Foo) $fooField.get(object) - return "(" + field.asType() + ") $" + field.getSimpleName() + ".get(object)"; + // ((com.example.Foo) $fooField.get(object)) + // Extra parentheses so cast evaluates before appended methods + // (e.g. when appending .getAsDouble()) + return "((" + field.asType() + ") $" + field.getSimpleName() + ".get(object))"; } else { // object.fooField return "object." + field.getSimpleName(); From ee625d4f4b37de3be5b435fa40c7facc33d47129 Mon Sep 17 00:00:00 2001 From: shueja <32416547+shueja@users.noreply.github.com> Date: Fri, 22 Nov 2024 22:27:08 -0800 Subject: [PATCH 2/5] Fixed tests and added one for private suppliers --- .../processor/AnnotationProcessorTest.java | 61 ++++++++++++++++++- 1 file changed, 58 insertions(+), 3 deletions(-) diff --git a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java index 19b2ae43d1a..4900f095321 100644 --- a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java +++ b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java @@ -141,7 +141,60 @@ public ExampleLogger() { @Override public void update(DataLogger dataLogger, Example object) { if (Epilogue.shouldLog(Logged.Importance.DEBUG)) { - dataLogger.log("x", (double) $x.get(object)); + dataLogger.log("x", ((double) $x.get(object))); + } + } + } + """; + + assertLoggerGenerates(source, expectedGeneratedSource); + } + + @Test + void privateSuppliers() { + String source = + """ + package edu.wpi.first.epilogue; + + import java.util.function.DoubleSupplier; + + @Logged + class Example { + private DoubleSupplier x; + } + """; + + String expectedGeneratedSource = + """ + 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 { + private static final VarHandle $x; + + static { + try { + var lookup = MethodHandles.privateLookupIn(Example.class, MethodHandles.lookup()); + $x = lookup.findVarHandle(Example.class, "x", java.util.function.DoubleSupplier.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)) { + dataLogger.log("x", ((java.util.function.DoubleSupplier) $sup.get(object)).getAsDouble()); } } } @@ -192,7 +245,7 @@ public ExampleLogger() { @Override public void update(DataLogger dataLogger, Example object) { if (Epilogue.shouldLog(Logged.Importance.DEBUG)) { - logSendable(dataLogger.getSubLogger("chooser"), (edu.wpi.first.wpilibj.smartdashboard.SendableChooser) $chooser.get(object)); + logSendable(dataLogger.getSubLogger("chooser"), ((edu.wpi.first.wpilibj.smartdashboard.SendableChooser) $chooser.get(object))); } } } @@ -1275,7 +1328,7 @@ public ExampleLogger() { @Override public void update(DataLogger dataLogger, Example object) { if (Epilogue.shouldLog(Logged.Importance.DEBUG)) { - var $$theField = (edu.wpi.first.epilogue.I) $theField.get(object); + 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 { @@ -1344,6 +1397,8 @@ public void update(DataLogger dataLogger, Example object) { assertLoggerGenerates(source, expectedGeneratedSource); } + + @Test void warnsAboutNonLoggableFields() { String source = From 155c4ce8b814b4508a4d87a9b4f56b90f9bd35e8 Mon Sep 17 00:00:00 2001 From: shueja <32416547+shueja@users.noreply.github.com> Date: Fri, 22 Nov 2024 22:50:28 -0800 Subject: [PATCH 3/5] Fix tests --- .../processor/AnnotationProcessorTest.java | 50 +++++++++---------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java index 4900f095321..7918870a9a6 100644 --- a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java +++ b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java @@ -166,39 +166,39 @@ class Example { String expectedGeneratedSource = """ - package edu.wpi.first.epilogue; +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; + 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 { - private static final VarHandle $x; + public class ExampleLogger extends ClassSpecificLogger { + private static final VarHandle $x; - static { - try { - var lookup = MethodHandles.privateLookupIn(Example.class, MethodHandles.lookup()); - $x = lookup.findVarHandle(Example.class, "x", java.util.function.DoubleSupplier.class); - } catch (ReflectiveOperationException e) { - throw new RuntimeException("[EPILOGUE] Could not load private fields for logging!", e); - } + static { + try { + var lookup = MethodHandles.privateLookupIn(Example.class, MethodHandles.lookup()); + $x = lookup.findVarHandle(Example.class, "x", java.util.function.DoubleSupplier.class); + } catch (ReflectiveOperationException e) { + throw new RuntimeException("[EPILOGUE] Could not load private fields for logging!", e); } + } - public ExampleLogger() { - super(Example.class); - } + public ExampleLogger() { + super(Example.class); + } - @Override - public void update(DataLogger dataLogger, Example object) { - if (Epilogue.shouldLog(Logged.Importance.DEBUG)) { - dataLogger.log("x", ((java.util.function.DoubleSupplier) $sup.get(object)).getAsDouble()); - } + @Override + public void update(DataLogger dataLogger, Example object) { + if (Epilogue.shouldLog(Logged.Importance.DEBUG)) { + dataLogger.log("x", ((java.util.function.DoubleSupplier) $x.get(object)).getAsDouble()); } } - """; + } + """; assertLoggerGenerates(source, expectedGeneratedSource); } From cdaf6651d3258a03a94b60455fa5aeaa6553f724 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sun, 24 Nov 2024 04:36:46 +0000 Subject: [PATCH 4/5] Formatting fixes --- .../wpi/first/epilogue/processor/AnnotationProcessorTest.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java index 7918870a9a6..b4275f2ff4b 100644 --- a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java +++ b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java @@ -1397,8 +1397,6 @@ public void update(DataLogger dataLogger, Example object) { assertLoggerGenerates(source, expectedGeneratedSource); } - - @Test void warnsAboutNonLoggableFields() { String source = From ff30994ccbfe698ceb8425e4d031bce37d7dcc66 Mon Sep 17 00:00:00 2001 From: shueja <32416547+shueja@users.noreply.github.com> Date: Sun, 24 Nov 2024 14:43:01 -0800 Subject: [PATCH 5/5] Update epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java Co-authored-by: Gold856 <117957790+Gold856@users.noreply.github.com> --- .../wpi/first/epilogue/processor/AnnotationProcessorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java index b4275f2ff4b..b3ae8c22433 100644 --- a/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java +++ b/epilogue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java @@ -166,7 +166,7 @@ class Example { String expectedGeneratedSource = """ -package edu.wpi.first.epilogue; + package edu.wpi.first.epilogue; import edu.wpi.first.epilogue.Logged; import edu.wpi.first.epilogue.Epilogue;