[parser] OCL 2.1 grammar precedence rule changes

[eclipse-ocl-bot]

| --- | --- |
| Bugzilla Link | 288040 |
| Status | CLOSED FIXED |
| Importance | P3 normal |
| Reported | Aug 30, 2009 02:27 EDT |
| Modified | May 27, 2011 02:49 EDT |
| Version | 1.3.0 |
| Reporter | Ed Willink |

Description

OCL 2.1 (RTF draft) makes three changes/clarifications to the precedence rules.

let-in is clarified as lowest precedence. This is what Christian implemented in MDT-OCL 1.2.0.

and/or/xor are no longer equal precedence.

^^ and ^ are defines as very high precedence. MDT-OCL currently has then equal to . and ->.

So we have two partitionings of equal precedence.

A) We could leave the grammar unchanged, and do a CST fix-up when OCL 2.1 parsing is requested.

B) We could update the grammar, and do a CST fix-up when MDT-OCL 1.x parsing is requested.

Either way we should offer an option to create warnings when OCL 2.1 and MDT-OCL 1.x give a different parse.

I thing B is best; retaining backward compatibility when possible means we can lose it when it gets too hard.

[eclipse-ocl-bot]

By Ed Willink on Sep 01, 2009 02:53

xor/or/and\

revising the grammar gives silent changes in behaviour. I've therefore implemented a suppressable warning for 2.1 behaviour that is different to 2.0.

message\

revising the grammar breaks 6 JUnit tests where Christian did

Apple.allInstances()->any(true)^^ripen(Color::yellow)->notEmpty()

The LHS is too complicated so m ust now be

(Apple.allInstances()->any(true))^^ripen(Color::yellow)->notEmpty()

This is a safe but slightly irritating change. Who uses messages? who uses message on more than a name as LHS?

Since ^ and ^^ are not diadic, this change seems to be misguided. Christian's realisation with ^,^^,.,-> all equal seems much better.

Do we want this change? Should we reject it with as OMG issue? The resolution for Issue 6544 has no associated rationale; it was an afterthough for xor/or/and.

An issue is probably required to specify left recursion for e.g.

name1.name2.name3

A decent resolution of this generic issue should specify a grammar with precedence (just like Java).

[eclipse-ocl-bot]

By Ed Willink on Sep 02, 2009 04:16

Created attachment 146263
Fix for Xor, Or, And precedence

Attached (which excludes the auto-generated Java parsers) gives xor, or, and distinct precedence as per the imminent OCL 2.1.

If an xor/or/and expression has a different CST under OCL 2.0 a warning is generated. This may be suppressed via a ParsingOption.

I have left message precedence unchanged. I propose to raise a substantive OMG Issue revising the OCL grammar to provide a combined definition of syntax, precedence and associativity in the same style as the C++ or Java grammars. This should clarify message, if, let.

:notepad_spiral: XorAndOr.patch

[eclipse-ocl-bot]

By Adolfo Sanchez-Barbudo Herrera on Sep 21, 2009 05:56

Hi Ed,

I have a lot conflicts while trying to apply the patch, due to modifications in CVS, which come from other bug resolutions. I have tried to do the merge in a sensible way, but It's not easy when dealing with CST XMI text, specially when I don't know what you are trying to do in it. Is there any chance to do the modifications yourself, and uploading it again ?.

As it has been manifested, I (we) shouldn't delay too much patches's revision, since we make the assignee waste time. I hope to respond earlier in future bugs.

Another concern is that as far as I have understood from your comments, extra stuff has been included to "warn" previous clients of changes. I have exposed my opinion (see my last comment in Bug 285633) about this some times, which is in conflict with yours. I guess that we need Alex or even PMC's help to solve this.

Cheers,
Adolfo.

[eclipse-ocl-bot]

By Alexander Igdalov on Oct 02, 2009 07:51

(In reply to comment #3)

Another concern is that as far as I have understood from your comments, extra
stuff has been included to "warn" previous clients of changes. I have exposed
my opinion (see my last comment in Bug 285633) about this some times, which is
in conflict with yours. I guess that we need Alex or even PMC's help to solve
this.

IMO, we shouldn't concentrate too much on warnings about OCL 2.1/2.0 incompatibility. But in case like this, when we can help users avoid hidden problems without little effort I give +1 for the option. However, I would unset it by default since we shouldn't warn users when they intentionally use OCL 2.1 precedence order in MDT OCL 3.0.0.

(In reply to comment #1)\

Apple.allInstances()->any(true)^^ripen(Color::yellow)->notEmpty()

The LHS is too complicated so m ust now be

(Apple.allInstances()->any(true))^^ripen(Color::yellow)->notEmpty()

This is a safe but slightly irritating change. Who uses messages? who uses
message on more than a name as LHS?

Since ^ and ^^ are not diadic, this change seems to be misguided. Christian's
realisation with ^,^^,.,-> all equal seems much better.

Do we want this change? Should we reject it with as OMG issue? The resolution
for Issue 6544 has no associated rationale; it was an afterthough for
xor/or/and.

+1 for rejection with an OMG issue.

(In reply to comment #2)

Created an attachment (id=146263)
Fix for Xor, Or, And precedence

Looks good. A few observations:

1. Although you introduce "isParenthesized" attribute to OperationCallExpCS, you seem to never check it. I suppose you intended to check it in AbstractOCLAnalyzer.checkForXorOrAndPrecedenceHazard(). Otherwise, you will report a warning in cases such as
   a xor (b and c)
   A JUnit test checking it would be helpful.
2. As I've mentioned I would set the ParsingOptions.WARN_OF_XOR_OR_AND_PRECEDENCE_CHANGE to false by default.
3. I would omit the ParsingOption.WARN_OF_MESSAGE_PRECEDENCE_CHANGE since we do not change anything in messages. There is also a typo in its javadoc comment - should be "MDT-OCL 1.x release", not "DT-OCL 1.x release"
4. As you have proposed we should eliminate extra copyrights. This should be applied to this patch as well;-)

[eclipse-ocl-bot]

By Alexander Igdalov on Oct 02, 2009 07:53

(In reply to comment #4)

But in case like this, when we can help users avoid hidden
problems without little effort I give +1 for the option.

Oops, I meant "avoid hidden problems with little effort" =))

[eclipse-ocl-bot]

By Ed Willink on Oct 02, 2009 16:57

Committed to HEAD.

#4 Very observant.

isParenthesized was put in when I thought I needed to parse the OCL 2.0 way and fix up at analyze-time. Doing it just the 2.1 way with a warning doesn't need this so its gone. Junit test checks with/without parentheses.

WARN_OF_MESSAGE_PRECEDENCE_CHANGE was similarly put in when I thought things were changing. They didn't. Option gone.

Copyrights trimmed on all associated files.

WARN_OF_XOR_OR_AND_PRECEDENCE_CHANGE default polarity is very debatable. If everyone likes false no problem. My view is that the OCL 2.0 has significant usage, so the OCL 2.1 makes this syntax fundamentally unsafe, so it needs a warning till OCL 3.0. cf certain Java usages that must be rewritten of @SuppressWarning'd. Here the warning goes if you put in parentheses.

---

Awaiting OMG Issue to:

Define message precedence and associativity.

[eclipse-ocl-bot]

By Alexander Igdalov on Oct 03, 2009 07:02

(In reply to comment #6)

Committed to HEAD.

#4 Very observant.

isParenthesized was put in when I thought I needed to parse the OCL 2.0 way and
fix up at analyze-time. Doing it just the 2.1 way with a warning doesn't need
this so its gone. Junit test checks with/without parentheses.

I thought isParenthesized was introduced to selectively warn users only in cases when and/or/xor precedence is changed.

Now you report a warning in the case shown below:

(true or (false and false)) = true

Why? Here the precedence is the same both in OCL 2.0 and OCL 2.1.

We shouldn't warn users in the following case as well:

(true or false.and(false)) = true

But now the warning is reported.

IMO the revised isParenthesized (i.e. isParenthesizedOrNonInfix) concept would be ideal to solve the revealed problems.

WARN_OF_XOR_OR_AND_PRECEDENCE_CHANGE default polarity is very debatable. If
everyone likes false no problem. My view is that the OCL 2.0 has significant
usage, so the OCL 2.1 makes this syntax fundamentally unsafe, so it needs a
warning till OCL 3.0. cf certain Java usages that must be rewritten of
@SuppressWarning'd.

What I see in CVS now is the WARN_OF_XOR_OR_AND_PRECEDENCE_CHANGE option set to true by default. From your comment above I conclude that it is a mistake. Am I correct? IMO we are implementing OCL 2.1 in MDT OCL 3.0.0 and we shouldn't warn users by default in case they correctly use OCL 2.1. We've got this right because we are changing the major segment in the version. We can help users avoid problems if they migrate from OCL 2.0 - but IMO it shouldn't be the default option.

[eclipse-ocl-bot]

By Ed Willink on Oct 04, 2009 07:20

Observant again.

isAtomic introduced as per the earlier isParenthesised to cover explicot dot calls too. JUnit test now covers these cases.

After reviewing xor precedence in a variety of languages, use of xor without parentheses is clearly unwise. OCL 2.1 is more sensible. OCL 2.0 without any left association specification had no predictable meaning; the MDT OCL 1.3.0 meaning was useful but not defined. The current MDT OCL 3.0 behaviour is hopefully more useful, and defined for these operators

...

So since OCL 2.1 is sensible and OCL 2.0 crazy I am happy to warn only when people want to know about a crazy language.

[eclipse-ocl-bot]

By Alexander Igdalov on Oct 05, 2009 06:04

Thanks, Ed! Looks good. Closing as RESOLVED FIXED.

[eclipse-ocl-bot]

By Ed Willink on May 27, 2011 02:49

Closing after over 18 months in resolved state.