[classic] Class org.eclipse.ocl.util.Bag violates general contract for Object.hashCode()

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 469159 | | Status | RESOLVED WONTFIX | | Importance | P3 normal | | Reported | Jun 02, 2015 11:07 EDT | | Modified | Jun 02, 2015 16:13 EDT | | Version | 6.0.0 | | Reporter | Sergey Boyko |

Description

JUnit snippet:

org.eclipse.ocl.util.Bag<Integer> bag1 = org.eclipse.ocl.util.CollectionUtil.createNewBag(Collections.singleton(1));\
org.eclipse.ocl.util.Bag<Integer> bag2 = org.eclipse.ocl.util.CollectionUtil.createNewBag(Collections.singleton(1));\
\
assertNotNull("Valid bag should be created", bag1);\
assertEquals("Bags with equal content should be equal", bag1, bag2);\
assertEquals("Bag class must obey the general contract for Object.hashCode(). See: Effective Java, 2nd edition, Item 9", bag1.hashCode(), bag2.hashCode());

[eclipse-ocl-bot]

By Sergey Boyko on Jun 02, 2015 11:10

(In reply to Sergey Boyko from comment #0)

JUnit snippet:

org.eclipse.ocl.util.Bag bag1 = org.eclipse.ocl.util.CollectionUtil.createNewBag(Collections.singleton(1)); org.eclipse.ocl.util.Bag bag2 = org.eclipse.ocl.util.CollectionUtil.createNewBag(Collections.singleton(1));

assertNotNull("Valid bag should be created", bag1); assertEquals("Bags with equal content should be equal", bag1, bag2); assertEquals("Bag class must obey the general contract for Object.hashCode(). See: Effective Java, 2nd edition, Item 9", bag1.hashCode(), bag2.hashCode());

Remediation: class 'BagImpl$MutableInteger' should override equals() and hashCode() similar to 'java.lang.Integer' class.

[eclipse-ocl-bot]

By Ed Willink on Jun 02, 2015 11:28

This is just one of many symptoms of some fundamental problems in the Classic evaluator.

Use of Java types is pervasive and fundamentally unsound, since the semantics of Java's Object::equals(Object) are not the same as OclAny::_'='(OclAny).

The most trivial example is 1 = 1.0, which was undefined in OCL until I clarified it to true on the basis that OCL is a specification language using numbers, whose implementation is irrelevant to their value. In Java 1 = 1.0 is false. And worse, Integer.valueof(1) = (int)1 is also false! This difference gets really serious for Set's where if Java semantics are used then Java equals rather than OCL equals is used for Set collisions.

Because of these problems the Pivot OCL introduces boxed values.

IntegerValue, RealValue are polymorphic interfaces for a variety of precisions.

BagValue, SetValue etc isolate the distinct equals semantics.

The Pivot solution to these problems is promoted to non-examples in Mars.

The Classic evaluator has established practice that is slightly dangerous to change.

[eclipse-ocl-bot]

By Sergey Boyko on Jun 02, 2015 11:42

(In reply to Ed Willink from comment #2)

This is just one of many symptoms of some fundamental problems in the Classic evaluator.

Use of Java types is pervasive and fundamentally unsound, since the semantics of Java's Object::equals(Object) are not the same as OclAny::_'='(OclAny).

The most trivial example is 1 = 1.0, which was undefined in OCL until I clarified it to true on the basis that OCL is a specification language using numbers, whose implementation is irrelevant to their value. In Java 1 = 1.0 is false. And worse, Integer.valueof(1) = (int)1 is also false! This difference gets really serious for Set's where if Java semantics are used then Java equals rather than OCL equals is used for Set collisions.

Because of these problems the Pivot OCL introduces boxed values.

IntegerValue, RealValue are polymorphic interfaces for a variety of precisions.

BagValue, SetValue etc isolate the distinct equals semantics.

The Pivot solution to these problems is promoted to non-examples in Mars.

The Classic evaluator has established practice that is slightly dangerous to change.

The bug is rather about fundamental problem in Java implementation of OCL abstraction. \ Of course when we talk about equality in OCL this absolutely different comparing to Java. But as soon as Java is chosen as an implementation tool for OCL then Java abstraction should follow Java practise.

[eclipse-ocl-bot]

By Sergey Boyko on Jun 02, 2015 11:44

(In reply to Sergey Boyko from comment #3)

(In reply to Ed Willink from comment #2)

This is just one of many symptoms of some fundamental problems in the Classic evaluator.

Use of Java types is pervasive and fundamentally unsound, since the semantics of Java's Object::equals(Object) are not the same as OclAny::_'='(OclAny).

The most trivial example is 1 = 1.0, which was undefined in OCL until I clarified it to true on the basis that OCL is a specification language using numbers, whose implementation is irrelevant to their value. In Java 1 = 1.0 is false. And worse, Integer.valueof(1) = (int)1 is also false! This difference gets really serious for Set's where if Java semantics are used then Java equals rather than OCL equals is used for Set collisions.

Because of these problems the Pivot OCL introduces boxed values.

IntegerValue, RealValue are polymorphic interfaces for a variety of precisions.

BagValue, SetValue etc isolate the distinct equals semantics.

The Pivot solution to these problems is promoted to non-examples in Mars.

The Classic evaluator has established practice that is slightly dangerous to change.

The bug is rather about fundamental problem in Java implementation of OCL abstraction. Of course when we talk about equality in OCL this absolutely different comparing to Java. But as soon as Java is chosen as an implementation tool for OCL then Java abstraction should follow Java practise.

Thus I'm inclined to reopen the bug.

[eclipse-ocl-bot]

By Ed Willink on Jun 02, 2015 11:49

There are ceratinly some flaws in the HjVa coherencve, but Eclipse OCL is an OCL tool not a JOCL tool.

THe behavior of Classic OCL is well established and so dangerous to change and has been superseded by fixes.

If you have a real problem please address Bug 469156#c1

"But I am surprised that the classic Ecore gets this right. And I'm surprised that you're reporting the bug. Are you seeing a regression?"

and we'll see what can be done.

[eclipse-ocl-bot]

By Sergey Boyko on Jun 02, 2015 11:52

(In reply to Ed Willink from comment #5)

There are ceratinly some flaws in the HjVa coherencve, but Eclipse OCL is an OCL tool not a JOCL tool.

THe behavior of Classic OCL is well established and so dangerous to change and has been superseded by fixes.

If you have a real problem please address Bug 469156#c1

"But I am surprised that the classic Ecore gets this right. And I'm surprised that you're reporting the bug. Are you seeing a regression?"

and we'll see what can be done.

The simplest result of incorrect implementation is that expression \ Set{Bag{1}}->includes(Bag{1})

evaluates to 'false' while should be 'true'.

[eclipse-ocl-bot]

By Ed Willink on Jun 02, 2015 11:55

(In reply to Sergey Boyko from comment #6)

Are you seeing a regression?"

The simplest result of incorrect implementation is that expression Set{Bag{1}}->includes(Bag{1})

evaluates to 'false' while should be 'true'.

and has been for at least 5 probably 10 years.

[eclipse-ocl-bot]

By Sergey Boyko on Jun 02, 2015 12:04

(In reply to Ed Willink from comment #7)

(In reply to Sergey Boyko from comment #6)

Are you seeing a regression?"

The simplest result of incorrect implementation is that expression Set{Bag{1}}->includes(Bag{1})

evaluates to 'false' while should be 'true'.

and has been for at least 5 probably 10 years.

Hm, good justification. \ The bug which I recently submitted to UML (bug 465283) also was there for years. It's not an impossible event when some bug is (re)discovered in very old and "stable" code.

[eclipse-ocl-bot]

By Ed Willink on Jun 02, 2015 12:43

(In reply to Sergey Boyko from comment #8)

Hm, good justification.

No, a lousy justification. Once I became aware of the pervasive limitations of the Classic code and the extreme difficulty of refactoring while retaining API compatibility, I opted for a replacement rather than incremental agonies, each of which upsets existing users. Your bugs are not new discoveries, I found many of them 5 years ago, though I must admit that BagImpl$MutableInteger is a new horror that I'm not sure your 'fix' solves.

The same BagImpl$MutableInteger is present in the Pivot implementation, but the use of the BagValueImpl wrapper avoids it contributing to hashCodes. It is never used for equality anyway.

The bug which I recently submitted to UML (bug 465283) also was there for years. It's not an impossible event when some bug is (re)discovered in very old and "stable" code.

Certainly, but I find EMF and UML2 very resistant to fixing traditional behaviors. Neither has any plans for replacement, so they should consider a fix.

[eclipse-ocl-bot]

By Sergey Boyko on Jun 02, 2015 15:53

Hi Ed,

I'm a bit surprised with (In reply to Ed Willink from comment #9)

(In reply to Sergey Boyko from comment #8)

Hm, good justification.

No, a lousy justification. Once I became aware of the pervasive limitations of the Classic code and the extreme difficulty of refactoring while retaining API compatibility, I opted for a replacement rather than incremental agonies, each of which upsets existing users. Your bugs are not new discoveries, I found many of them 5 years ago, though I must admit that BagImpl$MutableInteger is a new horror that I'm not sure your 'fix' solves.

The same BagImpl$MutableInteger is present in the Pivot implementation, but the use of the BagValueImpl wrapper avoids it contributing to hashCodes. It is never used for equality anyway.

Once again, BagImpl class violates fundamental design principles for Java classes:\ "If two objects are equal according to the equals(Object) method, then calling\ the hashCode method on each of the two objects must produce the same\ integer result."\ In short: "equal objects must have equal hash codes."

It does not matter which wrapper will you use - violating mentioned rule leads to unexpected problems when using instances of these class with for example standard collections.

[eclipse-ocl-bot]

By Sergey Boyko on Jun 02, 2015 15:57

(In reply to Sergey Boyko from comment #10)

Hi Ed,

I'm a bit surprised with (In reply to Ed Willink from comment #9)

(In reply to Sergey Boyko from comment #8)

Hm, good justification.

No, a lousy justification. Once I became aware of the pervasive limitations of the Classic code and the extreme difficulty of refactoring while retaining API compatibility, I opted for a replacement rather than incremental agonies, each of which upsets existing users. Your bugs are not new discoveries, I found many of them 5 years ago, though I must admit that BagImpl$MutableInteger is a new horror that I'm not sure your 'fix' solves.

The same BagImpl$MutableInteger is present in the Pivot implementation, but the use of the BagValueImpl wrapper avoids it contributing to hashCodes. It is never used for equality anyway.

Once again, BagImpl class violates fundamental design principles for Java classes: "If two objects are equal according to the equals(Object) method, then calling the hashCode method on each of the two objects must produce the same integer result." In short: "equal objects must have equal hash codes."

It does not matter which wrapper will you use - violating mentioned rule leads to unexpected problems when using instances of these class with for example standard collections.

Look at "Effective Java (2nd Edition)" by Joshua Bloch (http://www.amazon.com/Effective-Java-Edition-Joshua-Bloch/dp/0321356683) for a full examination of the problem.

[eclipse-ocl-bot]

By Ed Willink on Jun 02, 2015 16:13

(In reply to Sergey Boyko from comment #11)\

Look at "Effective Java (2nd Edition)" by Joshua Bloch (http://www.amazon.com/Effective-Java-Edition-Joshua-Bloch/dp/0321356683) for a full examination of the problem.

You do not need to tell me how bad these problems are.

Some like this one are superficially easily fixed,. It is not just the MutableInteger side Map<E, MutableInteger> equals/hashCode that is broken. The E is too; it should delegate to ObjectUtil/CollectionUtil.

Some are really hard to fix without unacceptable API change.

It is therefore my judgement that tinkering with these problems is not profitable and just delays real progress.