From 153562aa8cdc0bc2219f80a2c2495ed277f55deb Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Mon, 28 Jan 2019 17:27:52 -0500 Subject: [PATCH 1/7] update hashcode and equals of constraints, update ExistentialConstraint creation to take in sets of constraints as constructor arguments for efficiency --- .../inference/ConstraintNormalizer.java | 4 +- .../inference/model/ArithmeticConstraint.java | 2 +- .../inference/model/CombineConstraint.java | 25 ++++------- .../inference/model/ComparableConstraint.java | 22 ++++------ .../inference/model/ConstraintManager.java | 6 +-- .../inference/model/EqualityConstraint.java | 22 ++++------ .../model/ExistentialConstraint.java | 44 ++++++++++++++----- .../model/ImplicationConstraint.java | 2 +- .../inference/model/InequalityConstraint.java | 22 ++++------ .../inference/model/PreferenceConstraint.java | 34 +++----------- .../inference/model/SubtypeConstraint.java | 28 +++--------- .../model/serialization/JsonDeserializer.java | 12 ++--- 12 files changed, 93 insertions(+), 130 deletions(-) diff --git a/src/checkers/inference/ConstraintNormalizer.java b/src/checkers/inference/ConstraintNormalizer.java index 4bc883f67..209f3920d 100644 --- a/src/checkers/inference/ConstraintNormalizer.java +++ b/src/checkers/inference/ConstraintNormalizer.java @@ -239,8 +239,8 @@ public Set toConstraints() { return constraints; } - List ifExistsConstraints = new ArrayList<>(); - List ifNotExistsConstraints = new ArrayList<>(); + Set ifExistsConstraints = new HashSet<>(); + Set ifNotExistsConstraints = new HashSet<>(); ifExistsConstraints.addAll(constraints); for (final ExistentialNode existNode : ifExists.values()) { diff --git a/src/checkers/inference/model/ArithmeticConstraint.java b/src/checkers/inference/model/ArithmeticConstraint.java index 3597e30c9..ea110dd85 100644 --- a/src/checkers/inference/model/ArithmeticConstraint.java +++ b/src/checkers/inference/model/ArithmeticConstraint.java @@ -104,7 +104,7 @@ public T serialize(Serializer serializer) { public int hashCode() { // We do not hash on annotation location as the result slot is unique for each annotation // location - return HashCodeUtils.hash(operation, leftOperand, rightOperand, result); + return HashCodeUtils.hash(3079, operation, leftOperand, rightOperand, result); } @Override diff --git a/src/checkers/inference/model/CombineConstraint.java b/src/checkers/inference/model/CombineConstraint.java index f89619205..0647478b6 100644 --- a/src/checkers/inference/model/CombineConstraint.java +++ b/src/checkers/inference/model/CombineConstraint.java @@ -1,6 +1,8 @@ package checkers.inference.model; import java.util.Arrays; + +import org.checkerframework.dataflow.util.HashCodeUtils; import org.checkerframework.javacutil.BugInCF; /** @@ -51,28 +53,19 @@ public Slot getResult() { @Override public int hashCode() { - int hc = 1; - hc += ((target == null) ? 0 : target.hashCode()); - hc += ((decl == null) ? 0 : decl.hashCode()); - hc += ((result == null) ? 0 : result.hashCode()); - return hc; + return HashCodeUtils.hash(1543, target, decl, result); } @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - CombineConstraint other = (CombineConstraint) obj; - if (target.equals(other.target) && - decl.equals(other.decl) && - result.equals(other.result)) { - return true; - } else { + } + if (obj == null || getClass() != obj.getClass()) { return false; } + CombineConstraint other = (CombineConstraint) obj; + return target.equals(other.target) && decl.equals(other.decl) + && result.equals(other.result); } } diff --git a/src/checkers/inference/model/ComparableConstraint.java b/src/checkers/inference/model/ComparableConstraint.java index a756e7a85..360621cc9 100644 --- a/src/checkers/inference/model/ComparableConstraint.java +++ b/src/checkers/inference/model/ComparableConstraint.java @@ -2,6 +2,7 @@ import java.util.Arrays; +import org.checkerframework.dataflow.util.HashCodeUtils; import org.checkerframework.framework.type.QualifierHierarchy; import org.checkerframework.javacutil.BugInCF; @@ -80,26 +81,19 @@ public Constraint make(Slot first, Slot second) { @Override public int hashCode() { - int result = 1; - result = result + ((first == null) ? 0 : first.hashCode()); - result = result + ((second == null) ? 0 : second.hashCode()); - return result; + return HashCodeUtils.hash(769, first, second); } @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - ComparableConstraint other = (ComparableConstraint) obj; - if ((first.equals(other.first) && second.equals(other.second)) - || (first.equals(other.second) && (second.equals(other.first)))) { - return true; - } else { + } + if (obj == null || getClass() != obj.getClass()) { return false; } + ComparableConstraint other = (ComparableConstraint) obj; + return (first.equals(other.first) && second.equals(other.second)) + || (first.equals(other.second) && second.equals(other.first)); } } \ No newline at end of file diff --git a/src/checkers/inference/model/ConstraintManager.java b/src/checkers/inference/model/ConstraintManager.java index aa54a2257..051b11056 100644 --- a/src/checkers/inference/model/ConstraintManager.java +++ b/src/checkers/inference/model/ConstraintManager.java @@ -138,7 +138,7 @@ public PreferenceConstraint createPreferenceConstraint(VariableSlot variable, Co * Creates an {@link ExistentialConstraint} for the given slot and lists of constraints. */ public ExistentialConstraint createExistentialConstraint(Slot slot, - List ifExistsConstraints, List ifNotExistsConstraints) { + Set ifExistsConstraints, Set ifNotExistsConstraints) { return ExistentialConstraint.create((VariableSlot) slot, ifExistsConstraints, ifNotExistsConstraints, getCurrentLocation()); } @@ -267,8 +267,8 @@ public void addPreferenceConstraint(VariableSlot variable, ConstantSlot goal, in /** * Creates and adds a {@link ExistentialConstraint} to the constraint set. */ - public void addExistentialConstraint(Slot slot, List ifExistsConstraints, - List ifNotExistsConstraints) { + public void addExistentialConstraint(Slot slot, Set ifExistsConstraints, + Set ifNotExistsConstraints) { add(createExistentialConstraint(slot, ifExistsConstraints, ifNotExistsConstraints)); } diff --git a/src/checkers/inference/model/EqualityConstraint.java b/src/checkers/inference/model/EqualityConstraint.java index d72ba9bcf..618a1841c 100644 --- a/src/checkers/inference/model/EqualityConstraint.java +++ b/src/checkers/inference/model/EqualityConstraint.java @@ -2,6 +2,7 @@ import java.util.Arrays; +import org.checkerframework.dataflow.util.HashCodeUtils; import org.checkerframework.javacutil.BugInCF; /** @@ -95,26 +96,19 @@ public Constraint make(Slot first, Slot second) { @Override public int hashCode() { - int result = 1; - result = result + ((first == null) ? 0 : first.hashCode()); - result = result + ((second == null) ? 0 : second.hashCode()); - return result; + return HashCodeUtils.hash(97, first, second); } @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - EqualityConstraint other = (EqualityConstraint) obj; - if ((first.equals(other.first) && second.equals(other.second)) - || (first.equals(other.second) && (second.equals(other.first)))) { - return true; - } else { + } + if (obj == null || getClass() != obj.getClass()) { return false; } + EqualityConstraint other = (EqualityConstraint) obj; + return (first.equals(other.first) && second.equals(other.second)) + || (first.equals(other.second) && second.equals(other.first)); } } diff --git a/src/checkers/inference/model/ExistentialConstraint.java b/src/checkers/inference/model/ExistentialConstraint.java index 94f6807f0..1be42c9f2 100644 --- a/src/checkers/inference/model/ExistentialConstraint.java +++ b/src/checkers/inference/model/ExistentialConstraint.java @@ -1,10 +1,12 @@ package checkers.inference.model; import org.checkerframework.javacutil.PluginUtil; +import org.checkerframework.dataflow.util.HashCodeUtils; import org.checkerframework.javacutil.BugInCF; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.Set; /** * An ExistentialConstraint indicates that solvers need to determine if a variable's annotation @@ -27,22 +29,22 @@ public class ExistentialConstraint extends Constraint { private final VariableSlot potentialVariable; // The constraints to enforce if potentialVariable exists - private final List potentialConstraints; + private final Set potentialConstraints; // the constraints to enforce if potentialVariable DOES NOT exist - private final List alternateConstraints; + private final Set alternateConstraints; private ExistentialConstraint(VariableSlot potentialVariable, - List potentialConstraints, - List alternateConstraints, AnnotationLocation location) { + Set potentialConstraints, + Set alternateConstraints, AnnotationLocation location) { super(combineSlots(potentialVariable, potentialConstraints, alternateConstraints), location); this.potentialVariable = potentialVariable; - this.potentialConstraints = Collections.unmodifiableList(potentialConstraints); - this.alternateConstraints = Collections.unmodifiableList(alternateConstraints); + this.potentialConstraints = Collections.unmodifiableSet(potentialConstraints); + this.alternateConstraints = Collections.unmodifiableSet(alternateConstraints); } protected static ExistentialConstraint create(VariableSlot potentialVariable, - List potentialConstraints, List alternateConstraints, + Set potentialConstraints, Set alternateConstraints, AnnotationLocation location) { if (potentialVariable == null || potentialConstraints == null || alternateConstraints == null) { @@ -57,10 +59,10 @@ protected static ExistentialConstraint create(VariableSlot potentialVariable, } @SafeVarargs - private static List combineSlots(Slot potential, final List ... constraints) { + private static List combineSlots(Slot potential, final Set ... constraints) { final List slots = new ArrayList<>(); slots.add(potential); - for (final List constraintList : constraints) { + for (final Set constraintList : constraints) { for (final Constraint constraint : constraintList) { slots.addAll(constraint.getSlots()); } @@ -72,11 +74,11 @@ public VariableSlot getPotentialVariable() { return potentialVariable; } - public List potentialConstraints() { + public Set potentialConstraints() { return potentialConstraints; } - public List getAlternateConstraints() { + public Set getAlternateConstraints() { return alternateConstraints; } @@ -85,6 +87,26 @@ public T serialize(Serializer serializer) { return serializer.serialize(this); } + @Override + public int hashCode() { + return HashCodeUtils.hash(389, potentialVariable, potentialConstraints, + alternateConstraints); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null || getClass() != obj.getClass()) { + return false; + } + ExistentialConstraint other = (ExistentialConstraint) obj; + return potentialVariable.equals(other.potentialVariable) + && potentialConstraints.equals(other.potentialConstraints) + && alternateConstraints.equals(other.alternateConstraints); + } + @Override public String toString() { String tab = " "; diff --git a/src/checkers/inference/model/ImplicationConstraint.java b/src/checkers/inference/model/ImplicationConstraint.java index 6df6ea7f6..c46c7de5b 100644 --- a/src/checkers/inference/model/ImplicationConstraint.java +++ b/src/checkers/inference/model/ImplicationConstraint.java @@ -145,7 +145,7 @@ public Constraint getConclusion() { @Override public int hashCode() { - return HashCodeUtils.hash(assumptions, conclusion); + return HashCodeUtils.hash(12289, assumptions, conclusion); } @Override diff --git a/src/checkers/inference/model/InequalityConstraint.java b/src/checkers/inference/model/InequalityConstraint.java index a4bd71381..b2aa706eb 100644 --- a/src/checkers/inference/model/InequalityConstraint.java +++ b/src/checkers/inference/model/InequalityConstraint.java @@ -2,6 +2,7 @@ import java.util.Arrays; +import org.checkerframework.dataflow.util.HashCodeUtils; import org.checkerframework.javacutil.BugInCF; public class InequalityConstraint extends Constraint implements BinaryConstraint { @@ -73,26 +74,19 @@ public Constraint make(Slot first, Slot second) { @Override public int hashCode() { - int result = 1; - result = result + ((first == null) ? 0 : first.hashCode()); - result = result + ((second == null) ? 0 : second.hashCode()); - return result; + return HashCodeUtils.hash(193, first, second); } @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - InequalityConstraint other = (InequalityConstraint) obj; - if ((first.equals(other.first) && second.equals(other.second)) - || (first.equals(other.second) && (second.equals(other.first)))) { - return true; - } else { + } + if (obj == null || getClass() != obj.getClass()) { return false; } + InequalityConstraint other = (InequalityConstraint) obj; + return (first.equals(other.first) && second.equals(other.second)) + || (first.equals(other.second) && second.equals(other.first)); } } diff --git a/src/checkers/inference/model/PreferenceConstraint.java b/src/checkers/inference/model/PreferenceConstraint.java index feb89caf5..e25679d5f 100644 --- a/src/checkers/inference/model/PreferenceConstraint.java +++ b/src/checkers/inference/model/PreferenceConstraint.java @@ -1,6 +1,8 @@ package checkers.inference.model; import java.util.Arrays; + +import org.checkerframework.dataflow.util.HashCodeUtils; import org.checkerframework.javacutil.BugInCF; /** @@ -49,40 +51,18 @@ public int getWeight() { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((variable == null) ? 0 : variable.hashCode()); - result = prime * result - + ((goal == null) ? 0 : goal.hashCode()); - result = prime * result + weight; - return result; + return HashCodeUtils.hash(6151, variable, goal); } @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) - return false; - PreferenceConstraint other = (PreferenceConstraint) obj; - if (variable == null) { - if (other.variable != null) { - return false; - } - } else if (!variable.equals(other.variable)) { - return false; } - - if (goal == null) { - if (other.goal != null) { - return false; - } - } else if (!goal.equals(other.goal)) { + if (obj == null || getClass() != obj.getClass()) { return false; } - - return weight == other.weight; + PreferenceConstraint other = (PreferenceConstraint) obj; + return variable.equals(other.variable) && goal.equals(other.goal); } } diff --git a/src/checkers/inference/model/SubtypeConstraint.java b/src/checkers/inference/model/SubtypeConstraint.java index 9b26a193d..97f6c4088 100644 --- a/src/checkers/inference/model/SubtypeConstraint.java +++ b/src/checkers/inference/model/SubtypeConstraint.java @@ -2,6 +2,7 @@ import java.util.Arrays; +import org.checkerframework.dataflow.util.HashCodeUtils; import org.checkerframework.framework.type.QualifierHierarchy; import org.checkerframework.javacutil.BugInCF; @@ -126,34 +127,19 @@ public Slot getSupertype() { @Override public int hashCode() { - final int prime = 31; - int result = 1; - result = prime * result + ((subtype == null) ? 0 : subtype.hashCode()); - result = prime * result - + ((supertype == null) ? 0 : supertype.hashCode()); - return result; + return HashCodeUtils.hash(53, subtype, supertype); } @Override public boolean equals(Object obj) { - if (this == obj) + if (this == obj) { return true; - if (obj == null) - return false; - if (getClass() != obj.getClass()) + } + if (obj == null || getClass() != obj.getClass()) { return false; + } SubtypeConstraint other = (SubtypeConstraint) obj; - if (subtype == null) { - if (other.subtype != null) - return false; - } else if (!subtype.equals(other.subtype)) - return false; - if (supertype == null) { - if (other.supertype != null) - return false; - } else if (!supertype.equals(other.supertype)) - return false; - return true; + return subtype.equals(other.subtype) && supertype.equals(other.supertype); } /** diff --git a/src/checkers/inference/model/serialization/JsonDeserializer.java b/src/checkers/inference/model/serialization/JsonDeserializer.java index 639a3aad2..a28a38b1c 100644 --- a/src/checkers/inference/model/serialization/JsonDeserializer.java +++ b/src/checkers/inference/model/serialization/JsonDeserializer.java @@ -69,14 +69,14 @@ public JsonDeserializer(AnnotationMirrorSerializer annotationSerializer, String this.constraintManager = InferenceMain.getInstance().getConstraintManager(); } - public List parseConstraints() throws ParseException { + public Set parseConstraints() throws ParseException { JSONArray constraints = (JSONArray) root.get(CONSTRAINTS_KEY); - List results = jsonArrayToConstraints(constraints); + Set results = jsonArrayToConstraints(constraints); return results; } - public List jsonArrayToConstraints(final JSONArray jsonConstraints) { - List results = new LinkedList(); + public Set jsonArrayToConstraints(final JSONArray jsonConstraints) { + Set results = new HashSet<>(); for (Object obj: jsonConstraints) { if (obj instanceof String) { @@ -111,9 +111,9 @@ public List jsonArrayToConstraints(final JSONArray jsonConstraints) results.add(constraintManager.createComparableConstraint(lhs, rhs)); } else if (EXISTENTIAL_CONSTRAINT_KEY.equals(constraintType)) { Slot potential = parseSlot((String) constraint.get(EXISTENTIAL_ID)); - List thenConstraints = + Set thenConstraints = jsonArrayToConstraints((JSONArray) constraint.get(EXISTENTIAL_THEN)); - List elseConstraints = + Set elseConstraints = jsonArrayToConstraints((JSONArray) constraint.get(EXISTENTIAL_ELSE)); results.add(constraintManager.createExistentialConstraint((VariableSlot) potential, thenConstraints, elseConstraints)); From 90db1ff4137e6184c4072bb588d195a660190333 Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Mon, 28 Jan 2019 19:02:13 -0500 Subject: [PATCH 2/7] add hashcode and equals test case --- .../model/ConstraintsHashcodeEqualsTest.java | 177 ++++++++++++++++++ 1 file changed, 177 insertions(+) create mode 100644 tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java diff --git a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java new file mode 100644 index 000000000..05cae0529 --- /dev/null +++ b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java @@ -0,0 +1,177 @@ +package checkers.inference.model; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.mockito.runners.MockitoJUnitRunner; + +import checkers.inference.model.ArithmeticConstraint.ArithmeticOperationKind; + +@RunWith(MockitoJUnitRunner.class) +public class ConstraintsHashcodeEqualsTest { + ConstraintManager manager; + Set constraintSet; + Map constraintMap; + + AnnotationLocation dummyLocation; + VariableSlot one; + VariableSlot two; + VariableSlot three; + ArithmeticVariableSlot arithRes; + CombVariableSlot combRes; + // use Mockito to build a constant slot as AnnotationBuilder requires a ProcessingEnvironment, + // this CS has id 0 + @Mock ConstantSlot goal; + + AlwaysTrueConstraint atCon; + AlwaysFalseConstraint afCon; + ArithmeticConstraint addCon; + ArithmeticConstraint mulCon; + CombineConstraint combCon; + Constraint compCon; + Constraint compCon2; + Constraint eqCon; + Constraint eqCon2; + Constraint neqCon; + Constraint neqCon2; + Constraint exCon; + Constraint impCon; + PreferenceConstraint prefCon; + Constraint stCon; + + @Before + public void setUp() { + MockitoAnnotations.initMocks(this); + + manager = new ConstraintManager(); + constraintSet = new HashSet<>(); + constraintMap = new HashMap<>(); + + dummyLocation = new AnnotationLocation.ClassDeclLocation("java.lang.Number"); + one = new VariableSlot(5); + two = new VariableSlot(6); + three = new VariableSlot(7); + arithRes = new ArithmeticVariableSlot(dummyLocation, 8); + combRes = new CombVariableSlot(dummyLocation, 9, one, two); + + atCon = AlwaysTrueConstraint.create(); + afCon = AlwaysFalseConstraint.create(); + addCon = ArithmeticConstraint.create(ArithmeticOperationKind.PLUS, one, two, arithRes, + dummyLocation); + mulCon = ArithmeticConstraint.create(ArithmeticOperationKind.MULTIPLY, one, two, arithRes, + dummyLocation); + combCon = CombineConstraint.create(one, two, combRes, dummyLocation); + compCon = ComparableConstraint.create(one, two, dummyLocation, null); + compCon2 = ComparableConstraint.create(two, one, dummyLocation, null); + eqCon = EqualityConstraint.create(one, two, dummyLocation); + eqCon2 = EqualityConstraint.create(two, one, dummyLocation); + neqCon = InequalityConstraint.create(one, two, dummyLocation); + neqCon2 = InequalityConstraint.create(two, one, dummyLocation); + exCon = ExistentialConstraint.create(one, new HashSet<>(Arrays.asList(atCon)), + new HashSet<>(Arrays.asList(atCon)), dummyLocation); + impCon = ImplicationConstraint.create(Arrays.asList(addCon), mulCon, dummyLocation); + prefCon = PreferenceConstraint.create(three, goal, 100, dummyLocation); + stCon = SubtypeConstraint.create(one, two, dummyLocation, null); + } + + /** + * Tests the hashcode methods. + */ + @Test + public void testConstraintHashcodeEqualsMethods() { + isDistinctHashcodes(atCon, afCon, addCon, mulCon, combCon, compCon, compCon2, eqCon, eqCon2, + neqCon, neqCon2, exCon, impCon, prefCon, stCon); + } + + /** + * checks and ensures every constraint is distinct from each other + */ + private void isDistinctHashcodes(Constraint... constraints) { + for (Constraint c : constraints) { + for (Constraint d : constraints) { + // if equals then hashcode must be same + if (c.equals(d)) { + assertTrue("hashcode of " + c + " and " + d + " must be same.", + c.hashCode() == d.hashCode()); + } + // if not equals then hashcode must be different + else { + assertFalse("hashcode of " + c + " and " + d + " must be different.", + c.hashCode() == d.hashCode()); + } + } + } + } + + @Test + public void testConstraintSet() { + constraintSet.add(atCon); + constraintSet.add(afCon); + constraintSet.add(addCon); + constraintSet.add(mulCon); + constraintSet.add(combCon); + constraintSet.add(compCon); + constraintSet.add(compCon2); + constraintSet.add(eqCon); + constraintSet.add(neqCon); + constraintSet.add(exCon); + constraintSet.add(impCon); + constraintSet.add(prefCon); + constraintSet.add(stCon); + + assertTrue(constraintSet.contains(atCon)); + assertTrue(constraintSet.contains(afCon)); + assertTrue(constraintSet.contains(addCon)); + assertTrue(constraintSet.contains(mulCon)); + assertTrue(constraintSet.contains(combCon)); + assertTrue(constraintSet.contains(compCon)); + assertTrue(constraintSet.contains(compCon2)); + assertTrue(constraintSet.contains(eqCon)); + assertTrue(constraintSet.contains(neqCon)); + assertTrue(constraintSet.contains(exCon)); + assertTrue(constraintSet.contains(impCon)); + assertTrue(constraintSet.contains(prefCon)); + assertTrue(constraintSet.contains(stCon)); + } + + @Test + public void testConstraintMap() { + constraintMap.put(atCon, 1); + constraintMap.put(afCon, 2); + constraintMap.put(addCon, 3); + constraintMap.put(mulCon, 4); + constraintMap.put(combCon, 5); + constraintMap.put(compCon, 6); + constraintMap.put(compCon2, 66); + constraintMap.put(eqCon, 7); + constraintMap.put(neqCon, 8); + constraintMap.put(exCon, 9); + constraintMap.put(impCon, 10); + constraintMap.put(prefCon, 11); + constraintMap.put(stCon, 12); + + assertTrue(constraintMap.containsKey(atCon) && constraintMap.get(atCon) == 1); + assertTrue(constraintMap.containsKey(afCon) && constraintMap.get(afCon) == 2); + assertTrue(constraintMap.containsKey(addCon) && constraintMap.get(addCon) == 3); + assertTrue(constraintMap.containsKey(mulCon) && constraintMap.get(mulCon) == 4); + assertTrue(constraintMap.containsKey(combCon) && constraintMap.get(combCon) == 5); + assertTrue(constraintMap.containsKey(compCon) && constraintMap.get(compCon) == 66); + assertTrue(constraintMap.containsKey(eqCon) && constraintMap.get(eqCon) == 7); + assertTrue(constraintMap.containsKey(neqCon) && constraintMap.get(neqCon) == 8); + assertTrue(constraintMap.containsKey(exCon) && constraintMap.get(exCon) == 9); + assertTrue(constraintMap.containsKey(impCon) && constraintMap.get(impCon) == 10); + assertTrue(constraintMap.containsKey(prefCon) && constraintMap.get(prefCon) == 11); + assertTrue(constraintMap.containsKey(stCon) && constraintMap.get(stCon) == 12); + } +} From 152541efc7c59b87f17e44a03b2d20a99f8ea459 Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Mon, 28 Jan 2019 19:15:00 -0500 Subject: [PATCH 3/7] update hash of order insensitive constraints --- src/checkers/inference/model/ComparableConstraint.java | 3 ++- src/checkers/inference/model/EqualityConstraint.java | 3 ++- src/checkers/inference/model/InequalityConstraint.java | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/checkers/inference/model/ComparableConstraint.java b/src/checkers/inference/model/ComparableConstraint.java index 360621cc9..088dcd61d 100644 --- a/src/checkers/inference/model/ComparableConstraint.java +++ b/src/checkers/inference/model/ComparableConstraint.java @@ -81,7 +81,8 @@ public Constraint make(Slot first, Slot second) { @Override public int hashCode() { - return HashCodeUtils.hash(769, first, second); + // ComparableConstraint is insensitive to the order of the slots + return HashCodeUtils.hash(769, first.hashCode() + second.hashCode()); } @Override diff --git a/src/checkers/inference/model/EqualityConstraint.java b/src/checkers/inference/model/EqualityConstraint.java index 618a1841c..5a14f1754 100644 --- a/src/checkers/inference/model/EqualityConstraint.java +++ b/src/checkers/inference/model/EqualityConstraint.java @@ -96,7 +96,8 @@ public Constraint make(Slot first, Slot second) { @Override public int hashCode() { - return HashCodeUtils.hash(97, first, second); + // EqualityConstraint is insensitive to the order of the slots + return HashCodeUtils.hash(97, first.hashCode() + second.hashCode()); } @Override diff --git a/src/checkers/inference/model/InequalityConstraint.java b/src/checkers/inference/model/InequalityConstraint.java index b2aa706eb..a29efdc1c 100644 --- a/src/checkers/inference/model/InequalityConstraint.java +++ b/src/checkers/inference/model/InequalityConstraint.java @@ -74,7 +74,8 @@ public Constraint make(Slot first, Slot second) { @Override public int hashCode() { - return HashCodeUtils.hash(193, first, second); + // InequalityConstraint is insensitive to the order of the slots + return HashCodeUtils.hash(193, first.hashCode() + second.hashCode()); } @Override From 2eff215c1f792aaaf3af96d94146edcdce452895 Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Mon, 28 Jan 2019 19:35:06 -0500 Subject: [PATCH 4/7] update comment --- .../checkers/inference/model/ConstraintsHashcodeEqualsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java index 05cae0529..c7d54c28f 100644 --- a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java +++ b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java @@ -153,7 +153,7 @@ public void testConstraintMap() { constraintMap.put(mulCon, 4); constraintMap.put(combCon, 5); constraintMap.put(compCon, 6); - constraintMap.put(compCon2, 66); + constraintMap.put(compCon2, 66); // compCon2 has the same hashcode as compCon constraintMap.put(eqCon, 7); constraintMap.put(neqCon, 8); constraintMap.put(exCon, 9); From 08c596caba5b7f33f34339d81dc6855ba712afbd Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Mon, 28 Jan 2019 19:38:01 -0500 Subject: [PATCH 5/7] update test --- .../inference/model/ConstraintsHashcodeEqualsTest.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java index c7d54c28f..c64e96125 100644 --- a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java +++ b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java @@ -124,7 +124,9 @@ public void testConstraintSet() { constraintSet.add(compCon); constraintSet.add(compCon2); constraintSet.add(eqCon); + constraintSet.add(eqCon2); constraintSet.add(neqCon); + constraintSet.add(neqCon2); constraintSet.add(exCon); constraintSet.add(impCon); constraintSet.add(prefCon); @@ -138,7 +140,9 @@ public void testConstraintSet() { assertTrue(constraintSet.contains(compCon)); assertTrue(constraintSet.contains(compCon2)); assertTrue(constraintSet.contains(eqCon)); + assertTrue(constraintSet.contains(eqCon2)); assertTrue(constraintSet.contains(neqCon)); + assertTrue(constraintSet.contains(neqCon2)); assertTrue(constraintSet.contains(exCon)); assertTrue(constraintSet.contains(impCon)); assertTrue(constraintSet.contains(prefCon)); @@ -155,7 +159,9 @@ public void testConstraintMap() { constraintMap.put(compCon, 6); constraintMap.put(compCon2, 66); // compCon2 has the same hashcode as compCon constraintMap.put(eqCon, 7); + constraintMap.put(eqCon2, 77); // eqCon2 has the same hashcode as eqCon constraintMap.put(neqCon, 8); + constraintMap.put(neqCon2, 88); // neqCon2 has the same hashcode as neqCon2 constraintMap.put(exCon, 9); constraintMap.put(impCon, 10); constraintMap.put(prefCon, 11); @@ -167,8 +173,8 @@ public void testConstraintMap() { assertTrue(constraintMap.containsKey(mulCon) && constraintMap.get(mulCon) == 4); assertTrue(constraintMap.containsKey(combCon) && constraintMap.get(combCon) == 5); assertTrue(constraintMap.containsKey(compCon) && constraintMap.get(compCon) == 66); - assertTrue(constraintMap.containsKey(eqCon) && constraintMap.get(eqCon) == 7); - assertTrue(constraintMap.containsKey(neqCon) && constraintMap.get(neqCon) == 8); + assertTrue(constraintMap.containsKey(eqCon) && constraintMap.get(eqCon) == 77); + assertTrue(constraintMap.containsKey(neqCon) && constraintMap.get(neqCon) == 88); assertTrue(constraintMap.containsKey(exCon) && constraintMap.get(exCon) == 9); assertTrue(constraintMap.containsKey(impCon) && constraintMap.get(impCon) == 10); assertTrue(constraintMap.containsKey(prefCon) && constraintMap.get(prefCon) == 11); From bc47f1c8cfb3fdb66e4f108d8985c3cce6ddbe2c Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Mon, 28 Jan 2019 19:50:08 -0500 Subject: [PATCH 6/7] fix comment --- .../checkers/inference/model/ConstraintsHashcodeEqualsTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java index c64e96125..aee14451c 100644 --- a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java +++ b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java @@ -161,7 +161,7 @@ public void testConstraintMap() { constraintMap.put(eqCon, 7); constraintMap.put(eqCon2, 77); // eqCon2 has the same hashcode as eqCon constraintMap.put(neqCon, 8); - constraintMap.put(neqCon2, 88); // neqCon2 has the same hashcode as neqCon2 + constraintMap.put(neqCon2, 88); // neqCon2 has the same hashcode as neqCon constraintMap.put(exCon, 9); constraintMap.put(impCon, 10); constraintMap.put(prefCon, 11); From 9e4ff262594cc19cb547115a76d06b5020144b6c Mon Sep 17 00:00:00 2001 From: Jeff Luo Date: Mon, 28 Jan 2019 21:20:35 -0500 Subject: [PATCH 7/7] add check in ConstraintManager to warn creation of duplicate preference constraints with different weights --- .../inference/model/ConstraintManager.java | 14 ++++++++- .../model/ConstraintsHashcodeEqualsTest.java | 29 +++++++++++++++++-- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/src/checkers/inference/model/ConstraintManager.java b/src/checkers/inference/model/ConstraintManager.java index 051b11056..1373cf91e 100644 --- a/src/checkers/inference/model/ConstraintManager.java +++ b/src/checkers/inference/model/ConstraintManager.java @@ -261,7 +261,19 @@ public void addCombineConstraint(Slot target, Slot decl, Slot result) { * Creates and adds a {@link PreferenceConstraint} to the constraint set. */ public void addPreferenceConstraint(VariableSlot variable, ConstantSlot goal, int weight) { - add(createPreferenceConstraint(variable, goal, weight)); + PreferenceConstraint pc = createPreferenceConstraint(variable, goal, weight); + if (constraints.contains(pc)) { + PreferenceConstraint existingPC = (PreferenceConstraint) constraints.stream() + .filter(c -> c.hashCode() == pc.hashCode()).findFirst().get(); + + if (existingPC.getWeight() != weight) { + throw new BugInCF( + "Constraint " + pc + " already exists in the constraint set with weight " + + existingPC.getWeight() + "."); + } + } else { + add(pc); + } } /** diff --git a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java index aee14451c..baa51ac22 100644 --- a/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java +++ b/tests/checkers/inference/model/ConstraintsHashcodeEqualsTest.java @@ -1,5 +1,6 @@ package checkers.inference.model; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -7,6 +8,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Map; +import java.util.Optional; import java.util.Set; import org.junit.Before; @@ -20,7 +22,6 @@ @RunWith(MockitoJUnitRunner.class) public class ConstraintsHashcodeEqualsTest { - ConstraintManager manager; Set constraintSet; Map constraintMap; @@ -54,7 +55,6 @@ public class ConstraintsHashcodeEqualsTest { public void setUp() { MockitoAnnotations.initMocks(this); - manager = new ConstraintManager(); constraintSet = new HashSet<>(); constraintMap = new HashMap<>(); @@ -180,4 +180,29 @@ public void testConstraintMap() { assertTrue(constraintMap.containsKey(prefCon) && constraintMap.get(prefCon) == 11); assertTrue(constraintMap.containsKey(stCon) && constraintMap.get(stCon) == 12); } + + /** + * Ensure that a preference constraint expressed over the same slot and goal can only have 1 + * weight. + */ + @Test + public void testDuplicatePreferenceConstraint() { + constraintSet.add(atCon); + constraintSet.add(afCon); + constraintSet.add(addCon); + constraintSet.add(mulCon); + + constraintSet.add(prefCon); + PreferenceConstraint prefCon2 = PreferenceConstraint.create(three, goal, 200, + dummyLocation); + + assertTrue(constraintSet.contains(prefCon2)); + + Optional matchingPrefCon = constraintSet.stream() + .filter(c -> c.hashCode() == prefCon2.hashCode()).findFirst(); + assertTrue(matchingPrefCon.isPresent()); + + PreferenceConstraint pcInSet = (PreferenceConstraint) matchingPrefCon.get(); + assertEquals(prefCon.getWeight(), pcInSet.getWeight()); + } }