eclipse-ocl / org.eclipse.ocl

Eclipse Public License 2.0
0 stars 0 forks source link

[evaluator] Incorrect exact real to integer cast #747

Closed eclipse-ocl-bot closed 1 month ago

eclipse-ocl-bot commented 1 month ago

| --- | --- | | Bugzilla Link | 352950 | | Status | CLOSED FIXED | | Importance | P3 normal | | Reported | Jul 24, 2011 10:42 EDT | | Modified | May 20, 2013 11:37 EDT | | Version | 3.1.0 | | Reporter | Ed Willink |

Description

The mature evluatior has the following test

    assertResultInvalid("(3.0).oclAsType(Integer)");

which is wrong. 3.0 is exact, so the cast is valid.

eclipse-ocl-bot commented 1 month ago

By Axel Uhl on Jul 24, 2011 14:15

What about an expression 1.0/3.03.0? Generally, what should the rule be for when a numeric expression should conform to Integer? I guess I know what the spec says. But that's theoretical and unimplementable as we also observed in the Zurich workshop at OCL'11. We need a rule that users can understand and follow. What should that rule be? The above example is to show that it may not be as easy as saying "any numeric expression that can statically be evaluated to an integer value" because the evaluation itself, if done with the double arithmetic that the evaluator uses today, would most certainly not yield an integer result for some cases that conceptually are* of integer value.

I could go even further and design an expression involving iteration that can statically be proven to conceptually result in an integer value.

Personally, I don't see a point in trying to comply with something that generally can't be complied with. I suggest we make a special note in an errata for the Eclipse MDT OCL implementation that says that values provided as Real literals will not coerce to Integer.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Jul 24, 2011 14:32

The general case is certainly intractable.

The OCL specification is pretty vague, but it seems odd that a round trip from an integer such as 3 to 3.0 (to which it conforms) and back to 3 should not get back to where one started.

For the mature code, I would suggest that we document the behaviour as that associated with Java double.

For the pivot code, I think users should get a choice of Java double or BigDecimal and perhaps Java float and perhaps any class deriving from java.lang.Number, allowing RationalNumber, HyperbolicNumber, ... according to users analytic enthusiasm.

eclipse-ocl-bot commented 1 month ago

By Axel Uhl on Jul 24, 2011 17:09

Documenting for mature sounds reasonable. Supporting more fancy implementation types in pivot sounds ok. If all Real objects are implemented with arbitrarily precise types, you can do a reliable comparison with integer types. For all other types such as double you may still have to document some glitches.

eclipse-ocl-bot commented 1 month ago

By Axel Uhl on Jul 25, 2011 04:27

Where should the documentation for mature go? As a comment in-place in 2000-ocl-standard-library.textile where the Integer type and its conformance to Real is explained? Do we still have a "Limitations" section for the mature code somewhere? In the textiles I was only able to find a Limitations section for OCLInEcore.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Jul 25, 2011 04:50

(In reply to comment #4)

Do we still have a "Limitations" section.

Nothing was intentionally removed.

This is all part of the conformance statement that we discussed at about RC3 and which I ran out of enthusiasm for.

I envisage that the OMG should provide a template table of topics on which implementations should make some compliance statement. Our filled in tables should appear at the front of the user's guide.

For now just expand the Wiki with a few paragraphs and eventually I will use it as a starter for the OMG template.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Jul 25, 2011 05:05

(In reply to comment #0)

The mature evaluator has the following test

    assertResultInvalid("(3.0).oclAsType(Integer)");

which is wrong. 3.0 is exact, so the cast is valid.

I've changed my mind on this. The OCL spec is very vague. 7.4.6, which isn't normative, says "When it is certain that the actual type of the object is the subtype, the object can be re-typed using the operation\ oclAsType(Classifier).".

3 and 3.0 have equal values, but different types, so the re-typing from Real to Integer is never valid. 3.0.round() is available when needed, and difference checking may be applied if accuracy is a concern.

This makes the code simpler by removing the need for run-time evaluation rather than static analysis.

This is now a bug against the pivot evaluator.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Aug 16, 2011 17:01

The obvious fix of overloading OclAny oclAsType just for UnlimitedNatural fixes the Real cast but breaks:

assertQueryInvalid(null, "Set{1,2}.oclAsType(Set)");\ assertQueryResults(null, "Bag{1,2}", "Set{1,2}.oclAsType(UnlimitedNatural)");

The first is a compile-time failure; an improvement.

The second seems like a regression. How did it work before?

eclipse-ocl-bot commented 1 month ago

By Axel Uhl on Aug 16, 2011 18:17

Can you push the branch on which you created the "obvious fix" so I may have a look?

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Aug 17, 2011 01:18

(In reply to comment #8)

Can you push the branch on which you created the "obvious fix" so I may have a look?

NB. Following my change of mind, this is now a pivot bug. I thought it would be a quick fix. The side effect failures are obscure issues with overloaded operations for templates and classifiers, neither of which work for the mature model.

With a bit of luck the Bug 349962 enhancement, that is almost done, will eliminate one of the major needless complexities in CS/AS synchronization and this will become much much easier.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Sep 13, 2011 01:44

Fixed in the bug 349962 branch.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on Nov 07, 2011 17:12

Pushed to master.

eclipse-ocl-bot commented 1 month ago

By Ed Willink on May 20, 2013 11:37

CLOSED after a year in the RESOLVED state.