diverse-project / melange

The Melange Language Workbench
http://melange-lang.org
Eclipse Public License 1.0
28 stars 7 forks source link

K3 Sets are translated to EOperations when generating the language runtime #58

Open ebousse opened 8 years ago

ebousse commented 8 years ago

Hello :)

I have an Ecore file with an EClass Transformation, and I used K3 to define an aspect that look like this:

@Aspect(className=Transformation)
class TransformationAspect {
    public val Set<EObject> inputModel = new HashSet;
    ...

When I use Melange to generate the language runtime, the output look like that:

pb-melange-eoperation

The result is an EOperation inputModel. would it be possible instead to have a EReference with upperBound=-1, ordered=false and unique=true?

tdegueul commented 8 years ago

That would be nice indeed. Could you have a look @fcoulon?

fcoulon commented 8 years ago

For the quick explanation: You get an EOperation because inputModel is final, so K3 doesn't generated a Setter and because Melange search for both Setter and Getter in the Aspect class it deduces the Getter for inputModel is a real EOperation.

If you write public Set<EObject> inputModel = new HashSet; you will get an EAttribute typed Set because we don't support Set.

If you write public List<EObject> inputModel = new HashSet; you will get an EAttribute typed EObject but 0..* because EObject is not in your model (I guess :) ).

ebousse commented 8 years ago

For the quick explanation: You get an EOperation because inputModel is final, so K3 doesn't generated a Setter and because Melange search for both Setter and Getter in the Aspect class it deduces the Getter for inputModel is a real EOperation.

Right, but shouldn't val(or final) be ignored in the case of collections? Even if I cannot replace the valwith a new instance of HashSet, I can change the content of the collection the way I want using add, remove even clear, which is completely equivalent to what EMF does with a multi-valued feature (although in a slightly lazier way since the instance is only created on the first use of the field).

If you write public Set inputModel = new HashSet; you will get an EAttribute typed Set because we don't support Set.

Ah, but sets are awesome, you should support them :). I do everything with sets all the time.

If you write public List inputModel = new HashSet; you will get an EAttribute typed EObject but 0..* because EObject is not in your model (I guess :) ).

I'm not sure to understand?

tdegueul commented 8 years ago

One design choice we made some months ago was to discourage the use of traditional Java collections in K3 aspects (although this is not checked or enforced in any way...). The rationale behind that is that when adapters come into play, it's hard to automatically infer translation code from EMF collections to Java collections, and the other way around.

Since the public attributes of an aspect are meant to be translated to Ecore models later in the process, you could ease the work of Melange by using EMF-friendly datatypes in your aspects. So, even though that's pretty ugly, I would recommend to use the "EMF equivalent" of Set in your aspect (something like a UniqueEList?).

Otherwise, we can add some support in Melange for translating Sets to properly-configured EReferences. But you'll probably come back in a few months asking for Bags and whatnots ? :)

fcoulon commented 8 years ago

Right, but shouldn't val(or final) be ignored in the case of collections? Even if I cannot replace the valwith a new instance of HashSet, I can change the content of the collection the way I want using add, remove even clear, which is completely equivalent to what EMF does with a multi-valued feature (although in a slightly lazier way since the instance is only created on the first use of the field).

Yep you're right, the val is not enough. Using an immutable implementation of Collection should do the job.

Ah, but sets are awesome, you should support them :). I do everything with sets all the time.

I agree :D . To my mind, if we add attributs/references to metamodel, we should stay in the EMF world, so use EList subtypes for 0..* cardinality.

Maybe something like this could be OK:

@Aspect(className=Transformation)
class TransformationAspect {
    public UniqueEList<EObject> manyUnique = new UniqueEList;
    public BasicEList<EObject> manyOrdered = new BasicEList;

I'm not sure to understand?

I am just saying we have a bug :p . Melange should infer EReference when we have an attribute EObject.

ebousse commented 8 years ago

Ha, OK, using EMF collections here would indeed make sense. I would be fine with specifying my multi-valued features by using UniqueEList (for unique=true) or BasicEList (for unique=false).

So then, if possible :), I would be very happy to have:

Sidenote: however there is no collection corresponding to ordered=false, right? So we would still have no exact counterpart for Set? I'm just being curious, this is not a big urgent problem since uniqueness is the most important. I don't really care about being non-ordered, except when we have to compare collections.

fcoulon commented 8 years ago

I add it to my TODO list. ;)

Since ordered lists are particular cases of unordered lists maybe its a fake feature of EMF :D

ebousse commented 8 years ago

Since ordered lists are particular cases of unordered lists maybe its a fake feature of EMF :D

That's in fact true, because EMF will always generate an ordered structure I think!

But the ordered boolean is still relevant and meaningful in an Ecore model. For instance, if you write a tool to compare models, then at runtime you might have to look at the EStructuralFeature of a collection to check whether it's ordered or not, in order to compare correctly (ie with or without ordering) two collections conforming to this feature.

And I am actually doing this in the generated code of my trace generators.... but so far I was not aware that it was useless because of Melange that always generates dynamic features with ordered=true :) (at least if I understoof correctlty).

fcoulon commented 8 years ago

Aspect attributes declared final and typed BasicEList & UniqueEList should work now.

ebousse commented 8 years ago

Great, many thanks, I'll test it as soon as I can!

Quick question: does melange look for the concrete type, or the declared type?

In other words, can I write:

public val List<EObject> manyUnique = new UniqueEList
fcoulon commented 8 years ago

We look the concrete type because the right part of an affectation is an expression and it is difficult to know its type at compile time.

So you have to write:

public val UniqueEList<EObject> manyUnique = new UniqueEList
ebousse commented 8 years ago

We look the concrete type because the right part of an affectation is an expression and it is difficult to know its type at compile time.

Actuallt I was calling the left part the "declared" type and the right part the "concrete" type. But OK it's clear with your example, I'll try with your snippet.

fcoulon commented 8 years ago

Actuallt I was calling the left part the "declared" type and the right part the "concrete" type. But OK it's clear with your example, I'll try with your snippet.

Sorry I didn't reread myself :D

So yeah, Melange looks the static type.

fcoulon commented 8 years ago

We change to @Unique annotation on field. Field declared UniqueEList and BasicEList are not supported anymore.

See 9412eaac6c034d4a0c819ca86b06f93a755a74cf for details

ebousse commented 8 years ago

Even better :). A few questions though:

  1. Is it only used as metadata (for melange and others), or is k3 generating some behavior to ensure/check uniqueness? (this question is a bit out of scope since it is for k3, but well...)
  2. Is @Ordered planned? or maybe already available/handled?
  3. Which of these snippets are valid solutions for managing the situation of the original issue:

(a)

@Unique
public val List<EObject> manyUnique = new ArrayList

(b)

@Unique
public val List<EObject> manyUnique = new UniqueEList

(c)

@Unique
public val List<EObject> manyUnique = new BasicEList

(d)

@Unique
public val UniqueEList<EObject> manyUnique = new UniqueEList

(e)

@Unique
public val BasicEList<EObject> manyUnique = new BasicEList

(f)

@Unique
public val Set<EObject> manyUnique = new HashSet
fcoulon commented 8 years ago

Is it only used as metadata (for melange and others), or is k3 generating some behavior to ensure/check uniqueness? (this question is a bit out of scope since it is for k3, but well...)

It is used as a metadata. @Unique is not an active annotation, just a simple one.

Is @Ordered planned? or maybe already available/handled?

Since EReferences are ordered by default it seems not important to me to have this feature.

Which of these snippets are valid solutions for managing the situation of the original issue:

Only field typed EList are infered to EReference so for your case it is:

@Unique
public val EList<EObject> manyUnique = new ArrayList

Note that = new ArrayList is not useful if you have an EReference manyUnique in your metamodel (K3 delegate to the EMF getter, so the list is always initialised)

ebousse commented 8 years ago

OK, thanks a lot!

Although it is not crucial/urgent at all, I still think that @Ordered would be useful as a way to control more precisely the resulting Ecore model. As I said earlier in the discussion, you might want to specify in your syntax that the order is not important, meaning for instance that two references with ordered=false can be considered equal if they contain the same elements but not in the same order (or course it is the responsibility of tools to take this piece of metadata into account).

For instance, in the code I generate for for execution trace construction, I will have to check for changes between different versions of a collection, and I make use of ordered to decide how the comparison must be done.

And an example from the language perspective: when I am extending an FSM-like language with dynamic concepts, I may want to have a dynamic feature called activeNodes for which I would explicitly want to specify that the order of the nodes in this collection are of no importance.

In any case: I'll test this new @Unique as soon as I can, I promise :)