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.

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

[epilogue] allow arrays with @Logged types to be logged #7499

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -104,8 +104,8 @@ public boolean process(Set<? extends TypeElement> annotations, RoundEnvironment
processingEnv, loggedTypes), // prioritize epilogue logging over Sendable
new ConfiguredLoggerHandler(
processingEnv, customLoggers), // then customized logging configs
new ArrayHandler(processingEnv),
new CollectionHandler(processingEnv),
new ArrayHandler(processingEnv, loggedTypes),
new CollectionHandler(processingEnv, loggedTypes),
new EnumHandler(processingEnv),
new MeasureHandler(processingEnv),
new PrimitiveHandler(processingEnv),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,33 +4,39 @@

package edu.wpi.first.epilogue.processor;

import java.util.Collection;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.Element;
import javax.lang.model.element.Modifier;
import javax.lang.model.type.ArrayType;
import javax.lang.model.type.PrimitiveType;
import javax.lang.model.type.TypeMirror;
import javax.tools.Diagnostic;

/**
* Arrays of bytes, ints, flats, doubles, booleans, Strings, and struct-serializable objects can be
* logged. No other array types - including multidimensional arrays - are loggable.
*/
public class ArrayHandler extends ElementHandler {
private final StructHandler m_structHandler;
private final LoggableHandler m_loggableHandler;
private final TypeMirror m_javaLangString;

protected ArrayHandler(ProcessingEnvironment processingEnv) {
protected ArrayHandler(
ProcessingEnvironment processingEnv, Collection<? extends Element> loggedTypes) {
super(processingEnv);

// use a struct handler for managing struct arrays
m_structHandler = new StructHandler(processingEnv);

m_loggableHandler = new LoggableHandler(processingEnv, loggedTypes);
m_javaLangString = lookupTypeElement(processingEnv, "java.lang.String").asType();
}

@Override
public boolean isLoggable(Element element) {
return dataType(element) instanceof ArrayType arr
&& isLoggableComponentType(arr.getComponentType());
&& (isLoggableComponentType(arr.getComponentType())
|| isCustomLoggableArray(arr.getComponentType(), element));
}

/**
Expand All @@ -51,6 +57,30 @@ public boolean isLoggableComponentType(TypeMirror type) {
|| m_processingEnv.getTypeUtils().isAssignable(type, m_javaLangString);
}

/**
* Checks to see if an array has a type that either contains a @Logged annotation or has a custom
* logger. Will fail if the array is not final.
*
* @param componentType The component type of the array
* @param arrayElement The element of the array
* @return Whether the array
*/
private boolean isCustomLoggableArray(TypeMirror componentType, Element arrayElement) {
if (arrayElement.getModifiers().contains(Modifier.FINAL)) {
return m_loggableHandler.isLoggableType(componentType);
} else {
m_processingEnv
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this will cause two messages to be emitted: one for the general "not a loggable type", and this message

.getMessager()
.printMessage(
Diagnostic.Kind.NOTE,
"[EPILOGUE] Excluded from logs because array "
+ arrayElement
+ " isn't marked as final(or is returned from a method).",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could check the type of the element to be VariableElement or ExecutableElement to provide a more specific message

arrayElement);
return false;
}
}

@Override
public String logInvocation(Element element) {
var dataType = dataType(element);
Expand All @@ -67,6 +97,18 @@ public String logInvocation(Element element) {
+ ", "
+ m_structHandler.structAccess(componentType)
+ ")";
} else if (m_loggableHandler.isLoggableType(componentType)) {
var elementAccess = elementAccess(element);
var logInvocation =
m_loggableHandler.addLogPathSuffix(
m_loggableHandler.logInvocation(element, componentType, elementAccess + "[i]"),
"\"/\" + i");
return """
for (int i = 0; i < %s.length; i++) {
%s;
}
"""
.formatted(elementAccess, logInvocation);
} else {
// Primitive or string array
return "backend.log(\"" + loggedName(element) + "\", " + elementAccess(element) + ")";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

package edu.wpi.first.epilogue.processor;

import java.util.Collection;
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.Element;
import javax.lang.model.type.DeclaredType;
Expand All @@ -17,9 +18,10 @@ public class CollectionHandler extends ElementHandler {
private final TypeMirror m_collectionType;
private final StructHandler m_structHandler;

protected CollectionHandler(ProcessingEnvironment processingEnv) {
protected CollectionHandler(
ProcessingEnvironment processingEnv, Collection<? extends Element> loggedTypes) {
super(processingEnv);
m_arrayHandler = new ArrayHandler(processingEnv);
m_arrayHandler = new ArrayHandler(processingEnv, loggedTypes);
m_collectionType =
processingEnv.getElementUtils().getTypeElement("java.util.Collection").asType();
m_structHandler = new StructHandler(processingEnv);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,28 @@ protected LoggableHandler(

@Override
public boolean isLoggable(Element element) {
return m_processingEnv.getTypeUtils().asElement(dataType(element)) instanceof TypeElement t
return isLoggableType(dataType(element));
}

public boolean isLoggableType(TypeMirror typeMirror) {
return m_processingEnv.getTypeUtils().asElement(typeMirror) instanceof TypeElement t
&& m_loggedTypes.contains(t);
}

@Override
public String logInvocation(Element element) {
TypeMirror dataType = dataType(element);
return logInvocation(element, dataType(element), elementAccess(element));
}

/**
* Creates a log invocation with a custom data type/element access.
*
* @param element The base element of the type to log
* @param dataType The base type of the element
* @param elementAccess A code string that fetches the value when created in java
* @return A log call that logs the element
*/
public String logInvocation(Element element, TypeMirror dataType, String elementAccess) {
var declaredType =
m_processingEnv
.getElementUtils()
Expand All @@ -61,7 +76,7 @@ public String logInvocation(Element element) {

// If there are no known loggable subtypes, return just the single logger call
if (size == 1) {
return generateLoggerCall(element, declaredType, elementAccess(element));
return generateLoggerCall(element, declaredType, elementAccess);
}

// Otherwise, generate an if-else chain to compare the element with its known loggable subtypes
Expand All @@ -73,7 +88,7 @@ public String logInvocation(Element element) {
StringBuilder builder = new StringBuilder();

// Cache the value in a variable so it's only read once
builder.append("var %s = %s;\n".formatted(varName, elementAccess(element)));
builder.append("var %s = %s;\n".formatted(varName, elementAccess));

for (int i = 0; i < size; i++) {
TypeElement type = loggableSubtypes.get(i);
Expand All @@ -99,6 +114,17 @@ public String logInvocation(Element element) {
return builder.toString();
}

/**
* Adds a log path "suffix" to the end of a single log invocation.
*
* @param logInvocation The log invocation generated by {@link LoggableHandler#logInvocation}
* @param suffix The suffix to add. This suffix is added "as-is" in code.
* @return the new log invocation
*/
public String addLogPathSuffix(String logInvocation, String suffix) {
Daniel1464 marked this conversation as resolved.
Show resolved Hide resolved
return logInvocation.replaceAll("\"\\)", "\" + " + suffix + ")");
Daniel1464 marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Generates the name of the cache variable to use for a logged element.
*
Expand Down Expand Up @@ -129,9 +155,7 @@ private static String cacheVariableName(Element element) {
private Comparator<TypeElement> inheritanceComparatorFor(TypeElement declaredType) {
Comparator<TypeElement> byDistance =
Comparator.comparingInt(
inheritor -> {
return inheritanceDistance(inheritor.asType(), declaredType.asType());
});
inheritor -> inheritanceDistance(inheritor.asType(), declaredType.asType()));

return byDistance
.reversed()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,54 @@ public void update(EpilogueBackend backend, Example object) {
assertLoggerGenerates(source, expectedGeneratedSource);
}

@Test
void customLoggableArrays() {
String source =
"""
package edu.wpi.first.epilogue;
import java.util.List;

@Logged
class Example {
final SubLoggable[] x = new SubLoggable[] {}; // logged
SubLoggable[] y; // not logged; not marked final
final List<SubLoggable> z = List.of(); // not logged; lists cannot be logged
}

@Logged
class SubLoggable {
double y;
}
""";

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.EpilogueBackend;

public class ExampleLogger extends ClassSpecificLogger<Example> {
public ExampleLogger() {
super(Example.class);
}

@Override
public void update(EpilogueBackend backend, Example object) {
if (Epilogue.shouldLog(Logged.Importance.DEBUG)) {
for (int i = 0; i < object.x.length; i++) {
Epilogue.subLoggableLogger.tryUpdate(backend.getNested("x" + "/" + i), object.x[i], Epilogue.getConfig().errorHandler);
};
Daniel1464 marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
""";

assertLoggerGenerates(source, expectedGeneratedSource);
}

@Test
void badLogSetup() {
String source =
Expand Down
Loading