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

Override hashCode where equals is overridden #86

Merged
merged 1 commit into from
Jun 15, 2018

Conversation

gayanW
Copy link
Collaborator

@gayanW gayanW commented Jun 14, 2018

If two objects are equal according to the equals(Object), then calling
the hashCode method on each of the two objects must produce the same
integer result.

However we have classes in src/tests that overrides equals(Object)
without overriding the hashCode method. This overrides the missing
hashCode methods.

Fixes: #85

@@ -45,6 +45,11 @@ public boolean equals(Object other) {
return ((B) other).data == data;
}

@Override
public int hashCode() {
return data;
Copy link
Member

Choose a reason for hiding this comment

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

How about
return Integer.hashCode(value);
Directly using the data does not hash it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I returned data as it is a int. And Integer.hashCode(value) always return the passed value. Anyway it will be better to use Integer.hashCode(value)

@@ -56,6 +56,10 @@ public boolean equals(Object o) {
return this.b == other.b;
}

@Override
public int hashCode() {
return b ? 1009 : 2003;
Copy link
Member

Choose a reason for hiding this comment

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

This is functionally correct but Boolean.hashCode(b) is clearer and hides this implementation detail.


@Override
public int hashCode() {
return b ? 1511 : 2503;
Copy link
Member

Choose a reason for hiding this comment

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

Again, I'd prefer Boolean.hashCode(b) unless there is a need to provide our own hash function.

@gayanW
Copy link
Collaborator Author

gayanW commented Jun 15, 2018

Thank you for the suggestions. I made the suggested changes. BTW is it okay to have the same hashCode for an instance of this class and its Integer counterpart. So for instance B(1) and Integer(1) will have the same hashCode.

If two objects are equal according to the equals(Object), then calling
the hashCode method on each of the two objects must produce the same
integer result.

However we have classes in src/tests that overrides equals(Object)
without overriding the hashCode method. This overrides the missing
hashCode methods.

Fixes: javapathfinder#85
@cyrille-artho
Copy link
Member

cyrille-artho commented Jun 15, 2018 via email

@cyrille-artho
Copy link
Member

I think it's fine if there is a small int i which is not distinguished from a given boolean b by its hash value alone. A correct implementation of equals handles these cases for hash sets/maps.
Ideally, it would be very difficult to fine a given i whose hash code matches the one of a boolean, but it seems Java's hash functions are optimized for speed, not quality.

@cyrille-artho cyrille-artho merged commit 0d65435 into javapathfinder:java-10 Jun 15, 2018
gayanW added a commit to gayanW/jpf-core that referenced this pull request Jul 19, 2018
JVMClassInfo.Initializer#setBootstrapMethod assumes to have a bootstrap
method with a structure similar to the following (with 3 bootstrap
method arguments)

BootstrapMethods:
  0: javapathfinder#83 REF_invokeStatic java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:
      javapathfinder#84 (Ljava/lang/Object;)Ljava/lang/Object;
      javapathfinder#85 REF_invokeInterface java/lang/annotation/Annotation.annotationType:()Ljava/lang/Class;
      javapathfinder#86 (Ljava/lang/annotation/Annotation;)Ljava/lang/Class;

But if JPF encounters an bootstrap method with arbitary numer of
arguments such as the StringConcatFactory.makeConcatWithConstants which
have only one bootstrap method argument it results in an
ArrayIndexOutOfBoundsException exception:

[junit] java.lang.ArrayIndexOutOfBoundsException: 1
[junit] 	at gov.nasa.jpf.jvm.JVMClassInfo$Initializer.setBootstrapMethod(JVMClassInfo.java:93)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.setBootstrapMethod(ClassFile.java:659)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.parseBootstrapMethodAttr(ClassFile.java:1422)
[junit] 	at gov.nasa.jpf.jvm.JVMClassInfo$Initializer.setClassAttribute(JVMClassInfo.java:80)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.setClassAttribute(ClassFile.java:636)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.parseClassAttributes(ClassFile.java:1306)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.parse(ClassFile.java:875)
[junit] 	at gov.nasa.jpf.jvm.JVMClassInfo$Initializer.<init>(JVMClassInfo.java:48)

This prevents the above exception from being thrown, by adding a new
constructor BootstrapMethodInfo#BootstrapMethodInfo(enclosingClass,
cpArgs) for handling bootstrap methods with arbitrary number of
bootstrap method arguments, without evaluating cpArgs in
JVMClassInfo.Initializer#setBootstrapMethod.
gayanW added a commit to gayanW/jpf-core that referenced this pull request Jul 19, 2018
JVMClassInfo.Initializer#setBootstrapMethod assumes to have a bootstrap
method with a structure similar to the following (with 3 bootstrap
method arguments)

BootstrapMethods:
  0: javapathfinder#83 REF_invokeStatic java/lang/invoke/LambdaMetafactory.metafactory:(Ljava/lang/invoke/MethodHandles$Lookup;Ljava/lang/String;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodType;Ljava/lang/invoke/MethodHandle;Ljava/lang/invoke/MethodType;)Ljava/lang/invoke/CallSite;
    Method arguments:
      javapathfinder#84 (Ljava/lang/Object;)Ljava/lang/Object;
      javapathfinder#85 REF_invokeInterface java/lang/annotation/Annotation.annotationType:()Ljava/lang/Class;
      javapathfinder#86 (Ljava/lang/annotation/Annotation;)Ljava/lang/Class;

In case JPF encounters an bootstrap method with arbitrary number of
arguments such as the StringConcatFactory.makeConcatWithConstants which
have only one bootstrap method argument
JVMClassInfo.Initializer#setBootstrapMethod throws an
ArrayIndexOutOfBoundsException exception:

[junit] java.lang.ArrayIndexOutOfBoundsException: 1
[junit] 	at gov.nasa.jpf.jvm.JVMClassInfo$Initializer.setBootstrapMethod(JVMClassInfo.java:93)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.setBootstrapMethod(ClassFile.java:659)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.parseBootstrapMethodAttr(ClassFile.java:1422)
[junit] 	at gov.nasa.jpf.jvm.JVMClassInfo$Initializer.setClassAttribute(JVMClassInfo.java:80)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.setClassAttribute(ClassFile.java:636)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.parseClassAttributes(ClassFile.java:1306)
[junit] 	at gov.nasa.jpf.jvm.ClassFile.parse(ClassFile.java:875)
[junit] 	at gov.nasa.jpf.jvm.JVMClassInfo$Initializer.<init>(JVMClassInfo.java:48)

This prevents the above exception from being thrown, by adding a new
constructor BootstrapMethodInfo#BootstrapMethodInfo(enclosingClass,
cpArgs) for handling bootstrap methods with arbitrary number of
bootstrap method arguments, without evaluating cpArgs within
JVMClassInfo.Initializer#setBootstrapMethod.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants