[pivot] Incorrect empty collection range

[eclipse-ocl-bot]

| --- | --- | | Bugzilla Link | 473632 | | Status | RESOLVED FIXED | | Importance | P3 normal | | Reported | Jul 27, 2015 06:58 EDT | | Modified | Aug 03, 2015 15:11 EDT | | Reporter | Ed Willink |

Description

Testing support for static collection bounds reveals that

Set{1..0) is Set{1} rather than Set{}; the first value is always included. This is clearly wrong for the idiom Sequence{1..size} to obtain all 1-based indexes, which must give Sequence{} for a zero size.

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 27, 2015 07:15

Hi,

IMO, A decreasing range should be statically prohibited and dynamically produce OclInvalid.

Specification is, as usual, not clear. But the (wrong, BTW) WFR in page 57 goes in that direction.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jul 27, 2015 07:41

Fortunately the common case of ordered collection of a single range takes a different control path that is ok. The following fail:

ocl.assertQueryEquals(null, 0, "Set{1..0}->size()");\
ocl.assertQueryEquals(null, 1, "Sequence{2, 1..0}->size()");

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #1)

IMO, A decreasing range should be statically prohibited and dynamically produce OclInvalid.

Specification should now be clear. IT used to have a WFR that was an infinite loop as the first counted up until it was equal to the last.

For a while I wondered whether 3..1 was {3,2,1} until I came across the idiom:

let s : Sequence = ... \ in Sequence{1..s->size()}\ ->forAll(index : Integer | s->at(index)....)

for which it is essential that last < first => no elements.

Specification is, as usual, not clear. But the (wrong, BTW) WFR in page 57 goes in that direction.

http://www.omg.org/issues/ocl2-rtf#Issue15836

made improvements but as you observe is still inadequate. I've raised yet another issue on an issue.

[eclipse-ocl-bot]

By Ed Willink on Jul 27, 2015 08:25

(In reply to Ed Willink from comment #2)

The following fail:

ocl.assertQueryEquals(null, 0, "Set{1..0}->size()"); ocl.assertQueryEquals(null, 1, "Sequence{2, 1..0}->size()");

Fixed on branch ewillink/473632sr

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 27, 2015 10:37

(In reply to Ed Willink from comment #2)

Fortunately the common case of ordered collection of a single range takes a different control path that is ok. The following fail:

ocl.assertQueryEquals(null, 0, "Set{1..0}->size()"); ocl.assertQueryEquals(null, 1, "Sequence{2, 1..0}->size()");

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #1)

IMO, A decreasing range should be statically prohibited and dynamically produce OclInvalid.

Specification should now be clear. IT used to have a WFR that was an infinite loop as the first counted up until it was equal to the last.

For a while I wondered whether 3..1 was {3,2,1} until I came across the idiom:

let s : Sequence = ... in Sequence{1..s->size()} ->forAll(index : Integer | s->at(index)....)

for which it is essential that last < first => no elements

That example is skipping the discussion about if the Ranges should decrease or not, because it's assuming they are allowed. The specification seems to prohibit it.

Specification is, as usual, not clear. But the (wrong, BTW) WFR in page 57 goes in that direction.

http://www.omg.org/issues/ocl2-rtf#Issue15836

made improvements but as you observe is still inadequate. I've raised yet another issue on an issue.

That issue resolution enforces the fact that decreasing ranges are prohibited. I presume that the incorrect 8.4 number comes from editorial changes on various issue resolutions, because I don't see any 8.4 and/or 8.4.1 clause in my OCL 2.4 specification copy... but the issue resolution fits the text in pages 53 and 57 with respect to CollectionRange.

The WFR is wrong, because first and last are statically OclExpression rather than IntegerLiteralExp. The static check I mentioned before, should be something like:

context CollectionRange\ inv IncreasingRange: first.oclIsKindOf(IntegerLiteralExp) and last.oclIsKindOf(IntegerLiteralExp)\ implies first <= last

At execution time, if the expressions evaluation provokes first > last, I presume that OclInvalid should propagate.

I understand the convenience of "no range" for certain cases like the one you mentioned, but if is inconsistent with what the specification tries to explain. In other words why introducing a WFR to impose an increasing range if dynamically tends to "allow" the decreasing one ?.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jul 27, 2015 10:58

The specification is not perfect. Resolving lack of clarity takes account of:

• what the specification appears to be trying to say
• what common sense says
• what custom and practice does

In this case, Sequence{1..size} is custom and practice and must be valid for zero size.

Whether it is desirable for the static equivalent Sequence{1..0} to be invalid is a separate consideration. Distinct dynamic/static validity is undesirable, so I favour no WFR violation just a compiler warning about dead code.

Whether the dynamic case is restricted to Sequence{N..N-1} or applies to any Collection{N..M} where M < N is yet another consideration. I am not fond of a Sequence-only, N-1 only special case. So I prefer the simple regular "any 'negative' range is empty".

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 27, 2015 11:21

In this case, Sequence{1..size} is custom and practice and must be valid for zero size.

Since any size can previously be tested, "must" is arguable. In any case, I'm not against it, provides you a more concise expression, but the specification clearly needs to be changed. That OMG issue resolution clearly seems to be against to what you are trying to give support in this bugzilla.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jul 27, 2015 12:22

The specification already has over 25 instances of the 1..size idiom; probably without change between 2.0/2.2/2.3/2.4. The new OMG issue that I've raised is just making the specification say what it meant all along.

Issue 15836 was very badly written, and I only have myself to blame. (You did of course review it.)

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Jul 27, 2015 12:59

(In reply to Ed Willink from comment #7)

The specification already has over 25 instances of the 1..size idiom; probably without change between 2.0/2.2/2.3/2.4. The new OMG issue that I've raised is just making the specification say what it meant all along.

I think you are stating wrong implications, regardless of how many 1..size idioms are in the spec, it doesn't mean that Sequence{2..1} or Sequence{1..x->size()} had to produce a invalid or an empty sequence. It's a matter if we want to deal with invalid or not. I understand that it's convenient if we avoid them, with little drawbacks if we don't (apart from having to restate the spec about if the ranges can increase or not)

Issue 15836 was very badly written, and I only have myself to blame. (You did of course review it.)

To me issue 15836 makes sense, Sequence{10..5} is weird. Another topic is that we now realize that we want to avoid invalids in that specific 1..size construct. Perhaps we could try to look for similar usage in another language.

Regards,\ Adolfo.

[eclipse-ocl-bot]

By Ed Willink on Jul 27, 2015 13:06

This is going nowhere.

When you have a concrete proposal please put it forward. Yes Set{10..5} may be wierd but there's nothing actually wrong with it. You observed earlier that different dynamic / static validation would be confusing.

[eclipse-ocl-bot]

By Ed Willink on Aug 03, 2015 15:11

N..M where M < N now gives an empty range.

commit fd6df360aa57e096b10a9d6ac49e2f3d0f142e0d pushed to maintenance/R6_0 for SR1

commit 9dbcdc255eaadd8b81b4ed974ed1e3dcd463a438 pushed to master for M1