Bug 464927

Comment 1 Ed Willink 2015-04-18 09:52:08 EDT

If you hover over the definition of lookupNsmespace you see

Operation target::Visitable::lookupNamespace(pathSeq : OrderedSet(source::PathElementCS)) : target::Namespace

chnage the declaration to [?] and you see

Operation target::Visitable::lookupNamespace(pathSeq : OrderedSet(source::PathElementCS)) : target::Namespace[?]

change the decvlaration to [1] and you see

Operation target::Visitable::lookupNamespace(pathSeq : OrderedSet(source::PathElementCS)) : target::Namespace

again. So you have declared a function that never returns null allowing the subsequent comparison to null to be optimized away.

Unfortunately there is no validation warning. Introduction of safe navigation makes better warnings even more important. Bug 464931.

Comment 2 Ed Willink 2015-04-18 18:10:51 EDT

(In reply to Ed Willink from comment #1)

> change the decvlaration to [1] and you see
> 
> Operation target::Visitable::lookupNamespace(pathSeq :
> OrderedSet(source::PathElementCS)) : target::Namespace

Omission of "[1]" is pretty but no longer seems helpful. I'll see how annoying displaying it always in the PrettyPrinter is.

Comment 3 Adolfo Sanchez-Barbudo Herrera 2015-04-19 06:53:14 EDT

Hi 

Interesting, I didn't know the bounds in the op return type had any effect (I never use them). In this case just for the CGed code and apart from the surprise (since the interpreter doesn't get affected) I think it makes sense. As you comment just the tool needs to help user.

I've tried different combinations of op/defs and op calls, and the CGed code is what I expected. The lookup calls look prettier with the safe navigator.

As additional note , I also tried using [*] for the op return types . The operation type is a Set of the corresponding element. In my opinion, the type Collection should be more appropriate.

Comment 4 Ed Willink 2015-04-19 07:42:10 EDT

(In reply to Ed Willink from comment #2)
> Omission of "[1]" is pretty but no longer seems helpful. I'll see how
> annoying displaying it always in the PrettyPrinter is.

Pushed to master for M7.

Comment 5 Ed Willink 2015-05-02 07:51:38 EDT

(In reply to Adolfo Sanchez-Barbudo Herrera from comment #3)
> Interesting, I didn't know the bounds in the op return type had any effect
> (I never use them).

On further investigation, while a [1..1] default is consistent with UML, it is inconsistent with Ecore, but much more importantly it is inconsistent with OCL legacy.

    let temp : Temp = ... in ...
    temps->forAll(temp : Temp | ... )

The above examples impose no limitation that temp cannot be null, therefore the default for non-Collection declarations must be [0..1]. [1..1] is new functionality that must therefore be explicit.

(In reply to Ed Willink from comment #2)
> Omission of "[1]" is pretty but no longer seems helpful. I'll see how
> annoying displaying it always in the PrettyPrinter is.

Seems ok and given the different expectations of OCL/UML/Ecore users, displaying it is clearly important.

Comment 6 Ed Willink 2015-05-02 13:40:28 EDT

(In reply to Ed Willink from comment #5)
> The above examples impose no limitation that temp cannot be null, therefore
> the default for non-Collection declarations must be [0..1]. 

This applies to both T[*] and Set(T).

Also representations of EDataTypes whose instance class is a  Java primitive. This reveals some bad models that erroneously declare an EInt[?]. null is not a possible value and leads to a CG error.

Comment 7 Ed Willink 2015-05-23 12:11:45 EDT

(In reply to Ed Willink from comment #2)
> Omission of "[1]" is pretty but no longer seems helpful. I'll see how
> annoying displaying it always in the PrettyPrinter is.

Null-safe collections extend the multiplicity syntax to collection-multiplicity|element-multiplicity to e.g. [*|1] so showing multiplicities is increasingly valuable.

Fixed in M7/RC1/RC2.