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

Stop writing solutions that are equivalent to default annotations #339

Merged
merged 23 commits into from
Feb 10, 2022
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
151 changes: 147 additions & 4 deletions src/checkers/inference/DefaultSlotManager.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
package checkers.inference;

import checkers.inference.util.SlotDefaultTypeResolver;
import com.sun.source.tree.CompilationUnitTree;
import com.sun.source.tree.Tree;
import com.sun.tools.javac.code.Symbol;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory;
import org.checkerframework.framework.type.AnnotatedTypeFactory;
import org.checkerframework.framework.type.AnnotatedTypeMirror;
import org.checkerframework.javacutil.AnnotationBuilder;
import org.checkerframework.javacutil.AnnotationUtils;
import org.checkerframework.javacutil.BugInCF;
import org.checkerframework.javacutil.TypeKindUtils;
import org.checkerframework.javacutil.TypesUtils;

import java.lang.annotation.Annotation;
import java.util.ArrayList;
import java.util.Comparator;
import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -17,6 +27,9 @@
import javax.annotation.processing.ProcessingEnvironment;
import javax.lang.model.element.AnnotationMirror;
import javax.lang.model.element.AnnotationValue;
import javax.lang.model.element.Element;
import javax.lang.model.element.Name;
import javax.lang.model.element.TypeElement;
import javax.lang.model.type.TypeKind;
import javax.lang.model.type.TypeMirror;

Expand All @@ -35,8 +48,8 @@
import checkers.inference.model.SourceVariableSlot;
import checkers.inference.model.VariableSlot;
import checkers.inference.qual.VarAnnot;
import org.checkerframework.javacutil.TypeKindUtils;
import org.checkerframework.javacutil.TypesUtils;
import scenelib.annotations.io.ASTIndex;
import scenelib.annotations.io.ASTRecord;

/**
* The default implementation of SlotManager.
Expand All @@ -46,6 +59,12 @@ public class DefaultSlotManager implements SlotManager {

private final AnnotationMirror varAnnot;

/**
* The top annotation in the real qualifier hierarchy.
* Currently, we assume there's only one top.
*/
private final AnnotationMirror realTop;

// This id starts at 1 because in some serializer's
// (CnfSerializer) 0 is used as line delimiters.
// Monotonically increasing id for all VariableSlots (including
Expand Down Expand Up @@ -94,6 +113,22 @@ public class DefaultSlotManager implements SlotManager {
private final Map<Pair<Slot, Slot>, Integer> lubSlotPairCache;
private final Map<Pair<Slot, Slot>, Integer> glbSlotPairCache;

/**
* The hashcode of the latest compilation unit. This is currently used to
* determine whether we should reset the defaultAnnotationsCache.
*/
// TODO: we may use this to clear other caches
private Integer currentRootHash;
zcai1 marked this conversation as resolved.
Show resolved Hide resolved

/**
* A map of tree hashcode to {@link AnnotationMirror} for caching
* a set of pre-computed default types for the current compilation unit.
*
* The reason we're storing the hashcode is to avoid holding memory
zcai1 marked this conversation as resolved.
Show resolved Hide resolved
* occupied by trees.
*/
private Map<Integer, AnnotationMirror> defaultAnnotationsCache;
zcai1 marked this conversation as resolved.
Show resolved Hide resolved

/**
* A map of {@link AnnotationLocation} to {@link Integer} for caching
* {@link ArithmeticVariableSlot}s. The annotation location uniquely identifies an
Expand Down Expand Up @@ -122,9 +157,12 @@ public class DefaultSlotManager implements SlotManager {
private final ProcessingEnvironment processingEnvironment;

public DefaultSlotManager( final ProcessingEnvironment processingEnvironment,
final AnnotationMirror realTop,
final Set<Class<? extends Annotation>> realQualifiers,
boolean storeConstants) {
this.processingEnvironment = processingEnvironment;
this.realTop = realTop;
this.currentRootHash = null;
// sort the qualifiers so that they are always assigned the same varId
this.realQualifiers = sortAnnotationClasses(realQualifiers);
slots = new LinkedHashMap<>();
Expand All @@ -143,6 +181,7 @@ public DefaultSlotManager( final ProcessingEnvironment processingEnvironment,
arithmeticSlotCache = new LinkedHashMap<>();
comparisonThenSlotCache = new LinkedHashMap<>();
comparisonElseSlotCache = new LinkedHashMap<>();
defaultAnnotationsCache = null;
zcai1 marked this conversation as resolved.
Show resolved Hide resolved

if (storeConstants) {
for (Class<? extends Annotation> annoClass : this.realQualifiers) {
Expand Down Expand Up @@ -321,11 +360,17 @@ public int getNumberOfSlots() {

@Override
public SourceVariableSlot createSourceVariableSlot(AnnotationLocation location, TypeMirror type) {
AnnotationMirror defaultAnnotation = null;
if (InferenceOptions.ignoreDefaultAnnotations) {
// retrieve the default annotation when needed
defaultAnnotation = getDefaultAnnotationForLocation(location, type);
}

SourceVariableSlot sourceVarSlot;
if (location.getKind() == AnnotationLocation.Kind.MISSING) {
if (InferenceMain.isHackMode()) {
//Don't cache slot for MISSING LOCATION. Just create a new one and return.
sourceVarSlot = new SourceVariableSlot(nextId(), location, type, true);
sourceVarSlot = new SourceVariableSlot(nextId(), location, type, defaultAnnotation, true);
addToSlots(sourceVarSlot);
} else {
throw new BugInCF("Creating SourceVariableSlot on MISSING_LOCATION!");
Expand All @@ -335,13 +380,111 @@ public SourceVariableSlot createSourceVariableSlot(AnnotationLocation location,
int id = locationCache.get(location);
sourceVarSlot = (SourceVariableSlot) getSlot(id);
} else {
sourceVarSlot = new SourceVariableSlot(nextId(), location, type, true);
sourceVarSlot = new SourceVariableSlot(nextId(), location, type, defaultAnnotation, true);
addToSlots(sourceVarSlot);
locationCache.put(location, sourceVarSlot.getId());
}
return sourceVarSlot;
}

/**
* Find the default annotation for this location by checking the real type factory.
* @param location Location to create a new {@link SourceVariableSlot}.
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I think the @param and @return text fragments should not end with a .. Can you find and suggest a good javadoc style guide?

Copy link
Author

@zcai1 zcai1 Feb 9, 2022

Choose a reason for hiding this comment

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

Parameter names are lowercase by convention. The data type starts with a lowercase letter to indicate an object rather than a class. The description begins with a lowercase letter if it is a phrase (contains no verb), or an uppercase letter if it is a sentence. End the phrase with a period only if another phrase or sentence follows it.

For return:

Use the same capitalization and punctuation as you used in @ param.

from https://www.oracle.com/technical-resources/articles/java/javadoc-tool.html

* @return The default annotation for the given location.
*/
private @Nullable AnnotationMirror getDefaultAnnotationForLocation(AnnotationLocation location, TypeMirror type) {
if (location == AnnotationLocation.MISSING_LOCATION) {
zcai1 marked this conversation as resolved.
Show resolved Hide resolved
return null;
}

BaseAnnotatedTypeFactory realTypeFactory = InferenceMain.getInstance().getRealTypeFactory();
if (location instanceof AnnotationLocation.AstPathLocation) {
return getRealAnnotationForLocation(
realTypeFactory,
(AnnotationLocation.AstPathLocation) location);
} else if (location instanceof AnnotationLocation.ClassDeclLocation) {
return getRealAnnotationForLocation(
realTypeFactory,
(AnnotationLocation.ClassDeclLocation) location,
type);
}
zcai1 marked this conversation as resolved.
Show resolved Hide resolved

return null;
}

/**
* Find the default annotation for {@link AnnotationLocation.AstPathLocation} by
* checking the real type factory.
* @param realTypeFactory The current real {@link BaseAnnotatedTypeFactory}.
* @param location Location to create a new {@link SourceVariableSlot}.
* @return The default annotation for the given location.
*/
private @Nullable AnnotationMirror getRealAnnotationForLocation(
BaseAnnotatedTypeFactory realTypeFactory,
AnnotationLocation.AstPathLocation location
) {
ASTRecord astRecord = location.getAstRecord();
CompilationUnitTree root = astRecord.ast;
Tree tree = ASTIndex.getNode(root, astRecord);

if (tree == null) {
return null;
}

int rootHash = root.hashCode();
if (this.currentRootHash == null || this.currentRootHash != rootHash) {
// should update both root and cache
this.currentRootHash = rootHash;
this.defaultAnnotationsCache = new HashMap<>();
zcai1 marked this conversation as resolved.
Show resolved Hide resolved

Map<Tree, AnnotatedTypeMirror> defaultTypes = SlotDefaultTypeResolver.resolve(root, realTypeFactory);
for (Map.Entry<Tree, AnnotatedTypeMirror> entry : defaultTypes.entrySet()) {
this.defaultAnnotationsCache.put(
entry.getKey().hashCode(),
entry.getValue().getAnnotationInHierarchy(this.realTop)
);
}
}

AnnotationMirror realAnnotation = this.defaultAnnotationsCache.get(tree.hashCode());
if (realAnnotation == null) {
// If its default type can't be found in the cache, we can
// fallback to the simplest method.
realAnnotation = realTypeFactory.getAnnotatedType(tree).getAnnotationInHierarchy(this.realTop);
}

return realAnnotation;
}

/**
* Find the default annotation for ClassDeclLocation by checking the real type factory.
* @param realTypeFactory The current real {@link BaseAnnotatedTypeFactory}.
* @param location Location to create a new {@link SourceVariableSlot}.
* @param type Type of the associated class.
* @return The default annotation for the given location.
*/
private AnnotationMirror getRealAnnotationForLocation(
BaseAnnotatedTypeFactory realTypeFactory,
AnnotationLocation.ClassDeclLocation location,
TypeMirror type
) {
Element element = realTypeFactory.getProcessingEnv().getTypeUtils().asElement(type);
zcai1 marked this conversation as resolved.
Show resolved Hide resolved
if (!(element instanceof TypeElement)) {
throw new BugInCF(
"Expected to get a TypeElement for %s at %s, but received %s.", type, location, element);
}

TypeElement typeElement = (TypeElement) element;
Name fullyQualifiedName = ((Symbol.ClassSymbol)typeElement).flatName();
if (!fullyQualifiedName.contentEquals(location.getFullyQualifiedClassName())) {
throw new BugInCF(
"TypeElement for %s has qualified name %s, and it doesn't match with the location %s",
type, fullyQualifiedName, location);
}

return realTypeFactory.getAnnotatedType(typeElement).getAnnotationInHierarchy(this.realTop);
zcai1 marked this conversation as resolved.
Show resolved Hide resolved
}

@Override
public RefinementVariableSlot createRefinementVariableSlot(AnnotationLocation location, Slot declarationSlot, Slot valueSlot) {
// If the location is already cached, return the corresponding refinement slot in the cache
Expand Down
8 changes: 7 additions & 1 deletion src/checkers/inference/InferenceLauncher.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

import org.checkerframework.framework.util.CheckerMain;
import org.checkerframework.framework.util.ExecUtil;
import org.checkerframework.javacutil.SystemUtil;

import java.io.BufferedReader;
import java.io.ByteArrayOutputStream;
Expand Down Expand Up @@ -190,6 +189,13 @@ public void infer() {

addIfTrue("--hacks", InferenceOptions.hacks, argList);

Mode mode = Mode.valueOf(InferenceOptions.mode);
if (InferenceOptions.ignoreDefaultAnnotations
zcai1 marked this conversation as resolved.
Show resolved Hide resolved
&& (mode == Mode.ROUNDTRIP || mode == Mode.ROUNDTRIP_TYPECHECK)) {
// this flag is useful only if we will insert the solutions back
argList.add("--ignoreDefaultAnnotations");
}

argList.add("--");

String compilationBcp = getInferenceCompilationBootclassPath();
Expand Down
58 changes: 43 additions & 15 deletions src/checkers/inference/InferenceMain.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package checkers.inference;

import checkers.inference.model.SourceVariableSlot;
import org.checkerframework.checker.nullness.qual.Nullable;
import org.checkerframework.common.basetype.BaseAnnotatedTypeFactory;

import java.io.FileOutputStream;
Expand All @@ -26,6 +28,8 @@
import checkers.inference.qual.VarAnnot;
import checkers.inference.util.InferenceUtil;
import checkers.inference.util.JaifBuilder;
import org.checkerframework.javacutil.AnnotationUtils;
import org.checkerframework.javacutil.BugInCF;
import org.checkerframework.javacutil.SystemUtil;

/**
Expand Down Expand Up @@ -233,21 +237,13 @@ private void writeJaif() {
annotationClasses.add(annotation);
}
}

for (VariableSlot slot : varSlots) {
if (slot.getLocation() != null && slot.isInsertable()
&& (solverResult == null || solverResult.containsSolutionForVariable(slot.getId()))) {
if (slot.getLocation() != null) {
wmdietl marked this conversation as resolved.
Show resolved Hide resolved
// TODO: String serialization of annotations.
if (solverResult != null) {
// Not all VariableSlots will have an inferred value.
// This happens for VariableSlots that have no constraints.
AnnotationMirror result = solverResult.getSolutionForVariable(slot.getId());
if (result != null) {
values.put(slot.getLocation(), result.toString());
}
} else {
// Just use the VarAnnot in the jaif.
String value = slotManager.getAnnotation(slot).toString();
values.put(slot.getLocation(), value);
AnnotationMirror annotationToWrite = getAnnotationToWrite(slot);
if (annotationToWrite != null) {
values.put(slot.getLocation(), annotationToWrite.toString());
}
}
}
Expand Down Expand Up @@ -285,6 +281,29 @@ private void solve() {
}
}

private @Nullable AnnotationMirror getAnnotationToWrite(VariableSlot slot) {
if (!slot.isInsertable()) {
return null;
} else if (solverResult == null) {
// Just use the VarAnnot in the jaif.
return slotManager.getAnnotation(slot);
}

// Not all VariableSlots will have an inferred value.
// This happens for VariableSlots that have no constraints.
AnnotationMirror result = solverResult.getSolutionForVariable(slot.getId());
if (result != null && slot instanceof SourceVariableSlot) {
zcai1 marked this conversation as resolved.
Show resolved Hide resolved
AnnotationMirror defaultAnnotation = ((SourceVariableSlot) slot).getDefaultAnnotation();

if (defaultAnnotation != null
&& AnnotationUtils.compareAnnotationMirrors(defaultAnnotation, result) == 0) {
zcai1 marked this conversation as resolved.
Show resolved Hide resolved
// Don't need to write a solution that's equivalent to the default annotation.
result = null;
}
}
return result;
}

// ================================================================================
// Component Initialization
// ================================================================================
Expand Down Expand Up @@ -339,8 +358,17 @@ public BaseAnnotatedTypeFactory getRealTypeFactory() {

public SlotManager getSlotManager() {
if (slotManager == null ) {
slotManager = new DefaultSlotManager(inferenceChecker.getProcessingEnvironment(),
realTypeFactory.getSupportedTypeQualifiers(), true );
Set<? extends AnnotationMirror> tops = realTypeFactory.getQualifierHierarchy().getTopAnnotations();
if (tops.size() != 1) {
Copy link
Member

Choose a reason for hiding this comment

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

In PICO I think we have two tops, one for immutability and one for initialization. Only one hierarchy, immutability, is inferred, the other one is not. Haifeng will run in problems with this, but I'm fine with solving it then.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, we should discuss more about this in his PR.

throw new BugInCF("Expected 1 real top qualifier, but received %d instead", tops.size());
}

slotManager = new DefaultSlotManager(
inferenceChecker.getProcessingEnvironment(),
tops.iterator().next(),
realTypeFactory.getSupportedTypeQualifiers(),
true
);
logger.finer("Created slot manager" + slotManager);
}
return slotManager;
Expand Down
3 changes: 3 additions & 0 deletions src/checkers/inference/InferenceOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ public class InferenceOptions {
@Option("For inference, add debug on the port indicated")
public static String debug;

@Option("If ture, don't insert solutions that are equivalent to the default ones back to the code.")
Copy link
Member

Choose a reason for hiding this comment

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

"Debugging" doesn't sound like the best option group for this.

Copy link
Author

Choose a reason for hiding this comment

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

Moved to "Annotation File Utilities options" because I think it's more related to annotation insertion.

public static boolean ignoreDefaultAnnotations = true;
zcai1 marked this conversation as resolved.
Show resolved Hide resolved
zcai1 marked this conversation as resolved.
Show resolved Hide resolved

// end of command-line options
// ------------------------------------------------------

Expand Down
Loading