Skip to content
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

Jump to bottom

[epilogue] Add extra parentheses around cast when using varhandle #7428

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+60 −5
Open

[epilogue] Add extra parentheses around cast when using varhandle #7428

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
75f13b5
[epilogue] Add extra parentheses around cast when using varhandle
shueja Nov 23, 2024
ee625d4
Fixed tests and added one for private suppliers
shueja Nov 23, 2024
155c4ce
Fix tests
shueja Nov 23, 2024
cdaf665
Formatting fixes
github-actions[bot] Nov 24, 2024
ff30994
Update epilogue-processor/src/test/java/edu/wpi/first/epilogue/proces…
shueja Nov 24, 2024
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
6 changes: 4 additions & 2 deletions epilogue-processor/src/main/java/edu/wpi/first/epilogue/processor/ElementHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
59 changes: 56 additions & 3 deletions ...gue-processor/src/test/java/edu/wpi/first/epilogue/processor/AnnotationProcessorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ 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)));
}
}
}
Expand All @@ -150,6 +150,59 @@ public void update(DataLogger dataLogger, Example 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;
shueja marked this conversation as resolved.
Show resolved Hide resolved

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 $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) $x.get(object)).getAsDouble());
}
}
}
""";

assertLoggerGenerates(source, expectedGeneratedSource);
}

@Test
void privateWithGenerics() {
String source =
Expand Down Expand Up @@ -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<java.lang.String>) $chooser.get(object));
logSendable(dataLogger.getSubLogger("chooser"), ((edu.wpi.first.wpilibj.smartdashboard.SendableChooser<java.lang.String>) $chooser.get(object)));
}
}
}
Expand Down Expand Up @@ -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 {
Expand Down
Loading