Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 298201 - [releng] OCL models cannot be regenmodeled under 3.6M4
Summary: [releng] OCL models cannot be regenmodeled under 3.6M4
Status: CLOSED FIXED
Alias: None
Product: OCL
Classification: Modeling
Component: Core (show other bugs)
Version: unspecified   Edit
Hardware: PC Windows Vista
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: OCL Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 290103
  Show dependency tree
 
Reported: 2009-12-18 12:05 EST by Ed Willink CLA
Modified: 2013-05-22 14:16 EDT (History)
3 users (show)

See Also:


Attachments
Possibly complete patch (1.18 MB, patch)
2009-12-21 06:00 EST, Ed Willink CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Willink CLA 2009-12-18 12:05:12 EST
Models are genmodeled with custom dynamic templates.

These templates have not been updated to track UML2 and so they have an include path failure for CodeGenUtil.

Generating the CST model with standard templates results in a number of missing @since, @noreference, @noimplement of which only the former are errors for us.

These attributes could all be embedded within the

	 * <!-- begin-user-doc -->
	 * <!-- end-user-doc -->

comment region and so be preserved automatically.

Do we need custom templates just for this nicety?

Should the minor improvement be contributed back so that we don't get broken again?

Detailed investigation of the custom template deviations required.
Comment 1 Adolfo Sanchez-Barbudo Herrera CLA 2009-12-18 12:32:18 EST
Ed, I think that the bug Bug 290103 should track this an similar issues.

I believe that the current used templates (org.eclipse.ocl/templates) should be modified to accommodate any change in EMF and/or UML one.

Regards,
Adolfo.

*** This bug has been marked as a duplicate of bug 290103 ***
Comment 2 Ed Willink CLA 2009-12-18 17:11:53 EST
The existing templates indeed do no more than provide @noimplement and
@noreference comments automatically.

Unfortunately they do this for o.e.ocl and o.e.ocl.uml but not o.e.ocl.ecore.
This is inconsistent and wrong.

In the case of int values, we want to exclude ints from the API because they
change with Class addition/re-ordering, so o.e.ocl.ecore should be @noreference
too.

In the case of interfaces, we may wish to exclude o.e.ocl from the API with
@noimplement, but there is no reason why o.e.ocl.uml should be @noimplement,
particularly when o.e.ocl.ecore is not.

So the templates didn't even work as intended.

At best the templates are VERY slow (ten times slower) compared to the standard
ones. Perhaps they have a leakage bug in them. At worst they don't work now
anyway. Fixing the changed include path sometimes helps. I don't have time to
debug this VERY slow and 'sometimes'.

Philosophically good templates can be an asset, but bad templates in a vintage
language that have consistently failed to track the changes in UML and EMF
templates means that we are getting much more pain than benefit.

The simple expedient of placing @noreference and/or @noimplement within the
user documentation section means we do not need them at all and so don't need
their bugs and don't need to maintain them. We have to use this technique for
@since anyway.

I'll submit a patch to delete the templates and migrate the annotations.

If someone has time to work on the templates, the standard templates could be
enhanced to at least @noreference the int values. @noimplement perhaps could be
a genmodel option.
Comment 3 Ed Willink CLA 2009-12-19 09:50:14 EST
I am totally unable to prepare a patch without getting clobbered by timeouts (Bug 293355). With a couple of hundred trivial annotation migrations, preparing a patch piecemeal and metging is also not practical.

Can we therefore review the principle?

a) Delete the custom templates and so revert to the standard ones
b) Migrate the @nereference and @noimplement lines inside the user doicumentation section, so that regeneration preserves them
c) Making the Ecore-binding int properties @noreference like the UML-binding
d) Removing @noimplement from the UML-binding interfaces to match the Ecore-binding.

The above involve no code changes at all. The change in annotations appears to have no impact on QVTd or QVTo.

After migration of the annotations, the only problem with regeneration is the usual spurious import of xxx.* from xxx.util.XxxSwitch.java.
Comment 4 Laurent Goubet CLA 2009-12-21 03:46:53 EST
As far as I'm concerned, I am all for reverting to the standard JET template, especially so if the custom templates only provide annotations.

a) +1
b) Doesn't this require changes in the models? Adding "documentation" annotation for the API tags doesn't bother me though. +1
c) These are dangerous as API (any client referencing them will eventually see their code fail without real warning), +1
d) +1
Comment 5 Ed Willink CLA 2009-12-21 05:08:29 EST
Thanks for the +1's Laurent. I'm not sure Afolfo is happy, so I'll wait a couple of days to give him (and Alex) a chance to comment.

>> b) Migrate the @nereference and @noimplement lines inside the user
doicumentation section, so that regeneration preserves them

> b) Doesn't this require changes in the models? Adding "documentation"
annotation for the API tags doesn't bother me though. +1

A change within the comments only. Every interface file gets a 2 line comment migration. The Package interface also gets a comment migration/revision on every int returning feature method. Editing with regular expression replace matches is a great help here.
Comment 6 Adolfo Sanchez-Barbudo Herrera CLA 2009-12-21 05:51:39 EST
(In reply to comment #5)
> Thanks for the +1's Laurent. I'm not sure Afolfo is happy, so I'll wait a
> couple of days to give him (and Alex) a chance to comment.
> 
> >> b) Migrate the @nereference and @noimplement lines inside the user
> doicumentation section, so that regeneration preserves them
> 
> > b) Doesn't this require changes in the models? Adding "documentation"
> annotation for the API tags doesn't bother me though. +1
> 
> A change within the comments only. Every interface file gets a 2 line comment
> migration. The Package interface also gets a comment migration/revision on
> every int returning feature method. Editing with regular expression replace
> matches is a great help here.

Hi all,

I'm afraid I'm not sure about which is the best solution.

Using templates is good, since a decision taken when generating, will always be preserved every time we generate. I haven't ever annoyed when generating OCL or OCLCST code, I don't really care if the generation takes 10 secs isntead of 1, or a minute instead of six secs....as said, I haven't been worried about that. What I don't really like about using templates, is that we always have to be stearing at any change in EMF/UML.

So, I'm not unhappy with the proposed change, if just API tooling related are acomplished by the templates, and you preserve them in documentation section.

I also agree with the annotations-related conclusions.

Regards,
Adolfo.
Comment 7 Ed Willink CLA 2009-12-21 06:00:30 EST
Created attachment 154868 [details]
Possibly complete patch

Timeouts seem a bit better today.

Attached might demonstrate the changes.
Comment 8 Alexander Igdalov CLA 2009-12-21 07:22:28 EST
Hi All,

(In reply to comment #3)

> Can we therefore review the principle?
> 
> a) Delete the custom templates and so revert to the standard ones
> b) Migrate the @nereference and @noimplement lines inside the user
> doicumentation section, so that regeneration preserves them
> c) Making the Ecore-binding int properties @noreference like the UML-binding
> d) Removing @noimplement from the UML-binding interfaces to match the
> Ecore-binding.
> 

My +1 to a, b, c, and d.

As regards the patch, I would omit @noextend and @noimplement annotations in CST interfaces. QVTO, for instance, makes use of some the OCL interfaces such as CSTNode and OCLExpressionCS. Since OCL grammar is extensible, why should not the CST interfaces be extensible too?
In general, any CST interface might be extended (though I can hardly imagine a three-state boolean or heirs of InvalidLiteralExpCS). Thus, I would also get rid of @noextend and @noimplement in the CST model.

Regards,
- Alex.
Comment 9 Alexander Igdalov CLA 2009-12-21 07:40:15 EST
(In reply to comment #6)

Adolfo,

I am not sure that having to align the templates for the sake of API tooling only is reasonable. In this case I vote for the workaround.

However, we can consider raising a bugzilla against EMF and creating a corresponding patch for the EMF templates. In this patch we could provide JET extensions for optional .javajetinc files. If these files are included then the corresponding customizations take place (API tooling annotation insertions in our case).

If such patch is approved in EMF we could re-introduce the templates in OCL.

Cheers,
- Alex.
Comment 10 Adolfo Sanchez-Barbudo Herrera CLA 2009-12-21 09:18:28 EST
(In reply to comment #9)
> (In reply to comment #6)
> 
> Adolfo,
> 
> I am not sure that having to align the templates for the sake of API tooling
> only is reasonable. In this case I vote for the workaround.

Alex, let me disagree (partly). API tooling has been adopted as convention tool to facilitate the work and avoid bad practices to MDT-OCL developers and MDT-OCL clients. That said, if we want to follow the API tooling conventions, and take advantages from its facilities for our clients, we have two choices:
1. Using some templates, so that when generating the code we can include some API tooling wonders for free and, preserving them along the time, and saving us of having to be worried about them when regenerating after any metamodel change.
2. Doing that work by hand in every already generated class.

So, I would definitely do the point 1, regardless I had to align templates or I hadn't. Now, we have a third choice:
3. Let's use some regexp facilities, so that all API tooling annotations, are included in the documentation section, so that the templates are not needed anymore.

This decision has advantages and disadvantages:
+ We don't have to align to the EMF/UML template's changes, since we don't use templates anymore.
- We have to remember to include the annotations (by hand) after each regeneration when new classes, features are included/modified (error-prone).

That's the reason why I'm not completely pleased with the proposed change. Undoubtedly, if we don't need to worry about changes of the EMF/UML templates , the proposal has some added value (Although the proposal goes against some MDE and generation principles).

> 
> However, we can consider raising a bugzilla against EMF and creating a
> corresponding patch for the EMF templates. In this patch we could provide JET
> extensions for optional .javajetinc files. If these files are included then the
> corresponding customizations take place (API tooling annotation insertions in
> our case).
> 
> If such patch is approved in EMF we could re-introduce the templates in OCL.

I'm not sure what you refer. Are you suggesting to have a parameter in the genmodel, so that when it's enabled it automatically applies the changes (include the annotations) in the same fashion MDT-OCL is doing with the current templates ?. This sounds good, but we must convince Ed (Merks), that this a general purpose change, which seems to be sensible, in my opinion. Actually, Ecore itself shouldn't be extended, as Ed Merks usually says, so this annotations would be helpful for Ecore itself.

The idea of having a metamodel conceived to be extended, and not extended, and hence, annotated so that API tooling complains when extending. We could refine the idea, but it sounds good. I also guess the we would need a patch. I'm not a JET expert, and honestly I don't think this is prioritary. I think that we could at least throw an enhancement request.

What do you think ?

Regards,
Adolfo.

> 
> Cheers,
> - Alex.
Comment 11 Ed Willink CLA 2009-12-21 10:06:36 EST
Adolfo. I agree that it is error prone to have to add the annotations manually to new classes. Furtunately this is not a common occurrence. Unfortunately the existing templates had a number of errors that we are now correcting (manually).

Alex. I agree with removing the constraints on CST altogether. I think Christian added these to provide some API change flexibility when the 'internal' was removed from the package to support re-use by QVTd. 

I am now unconvinced that these annotations are correct at all. I now think they should all be removed.

@noreference on int features is not really the correct annotation. The int values are provided by EMF to provide efficient run-time behaviours and provided these values are not persisted, there is no reason why clients should not use them, so while @noreference avoids maintaining API filters when values change, it imposes an over-strict constraint on clients. We need an @nopersist annotation to accurately document the API. This is perhaps why Ed Merks will very reasonably decline to update EMF to supply an @noreference on int features. Perhaps we should put a 'This value is provided to facilitate efficient client usage. Its value may change when Eclipse is restarted, therefore a URI should be used to persist this value.' warning instead.

@noimplement on the abstract-binding is also not accurate. Why should a client encounter numerous warnings if they attempt to provide, for instance, a CMOF-binding? Ecore and UML bindings should not be privileged.

The latter problem may vanish if my plans for a generic OCLruntime binding mature to support run-time reflection via OCLClassifier, OCLOperation etc. See bug 283052.

Therefore compared to:

a) Delete the custom templates and so revert to the standard ones
b) Migrate the @nereference and @noimplement lines inside the user
doicumentation section, so that regeneration preserves them
c) Making the Ecore-binding int properties @noreference like the UML-binding
d) Removing @noimplement from the UML-binding interfaces to match the
Ecore-binding.

I vote

a) +1
b) -1
c) -1
d) +1

I now propose:

a) Delete the custom templates and so revert to the standard ones
e) Removing @noimplement and @noreference from all CST, OCL, OCL.Ecore, OCL.UML interfaces. (just requires a regeneration).
f) Add a 'This value is provided to facilitate efficient client usage. The value may change when Eclipse is restarted, therefore a URI should be used to persist this value.' comment on all integer features.

We will need API filters whenever integer values change.
Comment 12 Alexander Igdalov CLA 2009-12-21 10:11:45 EST
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #6)
> > 
> > Adolfo,
> > 
> > I am not sure that having to align the templates for the sake of API tooling
> > only is reasonable. In this case I vote for the workaround.
> 
> Alex, let me disagree (partly). API tooling has been adopted as convention tool
> to facilitate the work and avoid bad practices to MDT-OCL developers and
> MDT-OCL clients. That said, if we want to follow the API tooling conventions,
> and take advantages from its facilities for our clients, we have two choices:
> 1. Using some templates, so that when generating the code we can include some
> API tooling wonders for free and, preserving them along the time, and saving us
> of having to be worried about them when regenerating after any metamodel
> change.
> 2. Doing that work by hand in every already generated class.
> 

EMF Merge ensures that the user documentation is preserved. IOW, if we regenerate the model code over the exisiting sources the annotations do not vanish. I agree that it would be nice to have a chance to delete all generated files and make a "clean" regeneration. However, we do have custom code already (see @generated NOT tags in OCL AST), thus, such a clean regeneration won't work there for other reasons.

> > 
> > However, we can consider raising a bugzilla against EMF and creating a
> > corresponding patch for the EMF templates. In this patch we could provide JET
> > extensions for optional .javajetinc files. If these files are included then the
> > corresponding customizations take place (API tooling annotation insertions in
> > our case).
> > 
> > If such patch is approved in EMF we could re-introduce the templates in OCL.
> 
> I'm not sure what you refer. Are you suggesting to have a parameter in the
> genmodel, so that when it's enabled it automatically applies the changes
> (include the annotations) in the same fashion MDT-OCL is doing with the 
> current templates ?.

I was thinking of a single-line modification to EMF JET templates. Something like:

<%@ include file="Class/someThoughtfulName.javajetinc" fail="silent" %>

This instruction tells the JET compiler to look for a custom template file called someThoughtfulName.javajetinc and if it exists include its content into the invoking JET template. Such a modification will not change the previous EMF behaviour - it will just provide yet another extensibility point (you can see there are many .javajetinc referred to in the templates). Thus, if we put someThoughtfulName.javajetinc into the Class directory of the templates folder we could add our custom functionality without having to keep a slightly modified copy of Class.javajet and others.

> I'm not a
> JET expert, and honestly I don't think this is prioritary. I think that we
> could at least throw an enhancement request.
> 
> What do you think ?
>

I agree it's not a P1=)) Moreover, before we raise this request we should find out whether we actually need these annotations.

In addition to my comment #8:
I have noticed that not only CST but AST interfaces are also generated with @noextend and @noimplement tags. QVTO extends AST interfaces too. So these tags should be removed as well.

Best,
- Alex.
Comment 13 Alexander Igdalov CLA 2009-12-21 11:10:30 EST
(In reply to comment #11)

Hi Ed,

Seems we are going in the same direction (I have had a mid-air collision with your comment :).

I have implicitly agreed with the most parts of your new proposal in my previous comment. I just don't understand

> f) Add a 'This value is provided to facilitate efficient client usage. The
> value may change when Eclipse is restarted, therefore a URI should be used to
> persist this value.' comment on all integer features.

As I see it, these values are hardcoded. Thus, they shouldn't change when Eclipse is restarted. They can change iff an OCL model is modified and regenerated. 

I agree with you that these constants could be made available to clients since they also provide reflective access to OCL models. Thus, my +1 to the new proposal with a small refinement to (f):

f) Add a 'This value may change when the model code is regenerated. It is subject to change without notice.' comment on all integer features.

Cheers,
- Alex.
Comment 14 Ed Willink CLA 2009-12-21 11:32:37 EST
Regenerating will lose the annotation that we want to lose; those that the templates used to preserve. All @generated NOT's get standard correct treatment.

I have no problem with someone (not me) suggesting an extension flexibility to the EMF templates, which we might consider using.

>> 'This value may change when the model code is regenerated. It is
subject to change without notice.'

Your comment is shorter and more accurate. I was deliberately over-stating the hazard to allow a future version of a model implementation that allocated the integers dynamically at load-time following a package merge.

The 'It is subject to change without notice' covers dynamic allocation too.
Comment 15 Alexander Igdalov CLA 2009-12-21 14:08:42 EST
(In reply to comment #14)

Ed,

> I have no problem with someone (not me) suggesting an extension flexibility to
> the EMF templates, which we might consider using.
> 

I raised bug 298337 for the user-doc comment for int-fields. AFAIU, we do not need any other annotations yet.

- Alex.
Comment 16 Ed Willink CLA 2009-12-22 14:04:01 EST
Committed to CVS HEAD.
Comment 17 Ed Willink CLA 2009-12-22 15:04:49 EST
Verified in build #79.

[It turned out that the illegally implements were nothing to do with Xx extending Xx<Y>, but Ecore and UML extending @noimplement. So these filters all went. Ecore has no filters at all now.]
Comment 18 Ed Willink CLA 2011-05-27 02:48:34 EDT
Closing after over 18 months in resolved state.