Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 354799 - Add support for EMap
Summary: Add support for EMap
Status: CLOSED WORKSFORME
Alias: None
Product: TMF
Classification: Modeling
Component: Xtext (show other bugs)
Version: 2.0.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-08-16 04:47 EDT by Aaron Digulla CLA
Modified: 2017-10-31 11:27 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Aaron Digulla CLA 2011-08-16 04:47:56 EDT
Please add support for EMaps in the model to allow fast lookup of child elements by name. Example:

options:  @Map Option*

Option: name=ID '=' Value;

Value: ...

In my code, I'd like to be able to get Value instances by using options.get(String).
Comment 1 Moritz Eysholdt CLA 2011-08-16 05:06:21 EDT
With our grammar language we have the strategy so far, that only a very basic Ecore model can be derived. For more advanced things like specifying default values, lower/upper-bounds, and fine-tuning the inheritance hierarchy one has to edit the Ecore model manually. I'd consider EMaps to be such an advanced feature, too. So we wont add any special features to our grammar language for this. 

If you create your Ecore model manually, you should be able to use EMaps, since EMaps are just EList<Map.Entry<K, V>>. And we have support for lists and generics.

Did you try this, and if so, did you run into any problems with this approach?
Comment 2 Aaron Digulla CLA 2011-08-16 06:37:01 EDT
(In reply to comment #1)
> If you create your Ecore model manually, you should be able to use EMaps, since
> EMaps are just EList<Map.Entry<K, V>>. And we have support for lists and
> generics.

Does that mean the element look up would be O(N) even though it's a map?

> Did you try this, and if so, did you run into any problems with this approach?

No, because I wouldn't know where to start. Do you have a working example?
Comment 3 Moritz Eysholdt CLA 2011-08-16 07:30:20 EDT
(In reply to comment #2)
> (In reply to comment #1)
> > If you create your Ecore model manually, you should be able to use EMaps, since
> > EMaps are just EList<Map.Entry<K, V>>. And we have support for lists and
> > generics.
> 
> Does that mean the element look up would be O(N) even though it's a map?

look at org.eclipse.emf.common.util.BasicEMap.get(Object) and make up your mind...

> > Did you try this, and if so, did you run into any problems with this approach?
> 
> No, because I wouldn't know where to start. Do you have a working example?

yes, you might want to look at the Ecore models of 
org.eclipse.emf.ecore.EAnnotation.getDetails()
org.eclipse.emf.ecore.change.ChangeDescription.getObjectChanges()
Comment 4 Aaron Digulla CLA 2011-08-16 07:33:02 EDT
(In reply to comment #3)

> > No, because I wouldn't know where to start. Do you have a working example?
> 
> yes, you might want to look at the Ecore models of 
> org.eclipse.emf.ecore.EAnnotation.getDetails()
> org.eclipse.emf.ecore.change.ChangeDescription.getObjectChanges()

I'm referring to "create your Ecore model manually". How would I do that? And doesn't that mean I have to keep my model and the grammar in sync whenever I make a change?

Wouldn't it make more sense to allow annotations in the grammar which a generator step could pick up to modify the generated files? There are probably thousands of applications for something like this.
Comment 5 Moritz Eysholdt CLA 2011-08-16 07:43:45 EDT
(In reply to comment #4)
> (In reply to comment #3)
> 
> > > No, because I wouldn't know where to start. Do you have a working example?
> > 
> > yes, you might want to look at the Ecore models of 
> > org.eclipse.emf.ecore.EAnnotation.getDetails()
> > org.eclipse.emf.ecore.change.ChangeDescription.getObjectChanges()
> 
> I'm referring to "create your Ecore model manually". How would I do that?

http://www.eclipse.org/Xtext/documentation/2_0_0/020-grammar-language.php#package_declarations

> And doesn't that mean I have to keep my model and the grammar in sync whenever I
> make a change?

yes, it does. I usually switch from "generated Ecore model" to "imported Ecore model" once my grammar has become pretty stable. 

> Wouldn't it make more sense to allow annotations in the grammar which a
> generator step could pick up to modify the generated files?

No, because we would have to re-implement and maintain all language features of Ecore. Using annotations would also mean to have an ugly generic syntax for it and to loose comfort with regards to content assist to static validation.

> There are probably thousands of applications for something like this.

That is exactly the problem. The grammar would eventually contain more annotations than syntax definitions. This would render the grammar as readable as XMI. (XMI is not readable).

But nevertheless, the situation will improve: http://ed-merks.blogspot.com/2011/08/xcore-coolness-reborn.html
Comment 6 Sebastian Zarnekow CLA 2011-08-16 17:05:18 EDT
(In reply to comment #4)
> Wouldn't it make more sense to allow annotations in the grammar which a
> generator step could pick up to modify the generated files? There are probably
> thousands of applications for something like this.

Please feel free to add the next 990 use cases for annotations to bug 287230 ;-)

Meanwhile I'll close this one as works for me since importing a manually maintained EPackage is recommended and we do not have any plans to change the grammar syntax to allow to infer EMaps from a rule.
Comment 7 Aaron Digulla CLA 2012-05-16 04:21:55 EDT
I still think that having a "by name" access would be the most useful missing EMF feature in Xtext (well, apart from being able to override the generated code). How about this grammar syntax?

Options: options*=Option*

i.e. "*=" instead of "+=" to say "make this an EMap instead of an EList"
Comment 8 Moritz Eysholdt CLA 2012-05-16 04:44:04 EDT
(In reply to comment #7)
> I still think that having a "by name" access would be the most useful missing
> EMF feature in Xtext (well, apart from being able to override the generated
> code). How about this grammar syntax?
> 
> Options: options*=Option*
> 
> i.e. "*=" instead of "+=" to say "make this an EMap instead of an EList"

what would be the "name" in that example?

The example that I keep on having in mind is when you want to preserver the ordering multiple elements in the semantic model when the elements are not of the same type and when you don't want to (or can't) use polymorphism.

Imagine the following EClass

class Config {
  EReference Options1 options1
  EReference Options2 options2
  EReference Options3 options3
  EMap<EStructuralFeature, Object> options
}

Now you could use a grammar such as:

Config:
  (options[option1]=Option1 | options[option2]=Option2 | options[option3]=Option3);

This would parse the different kind of Options in any order. The benefit of using the map is:
- The oder in which the "Option" objects are parsed is preserved in the semantic model. I.e. if you serialize the model, the objects would still have the same order.
- The types of the "Options" objects don't need to be in the same inheritance hierarchy. 

However, it might not be necessary to extend our grammar language at all. As stated earlier in this discussion, you can always customize the Ecore model manually. This has just gotten much easier thanks to Xcore. Therefore, you can always create your EMaps manually.

Then, the grammar would look as follows:

Config:
  (option1=Option1 | option2=Option2 | option3=Option3);

Since EMF would use an EMap internally, the EStructuralFeatures option1..3 delegate to the EMap "options". Therefore, the semantic model that an Xtext parser would instantiate would look the same as with my first example.

However, it would be helpful if Xtext's serializer would take the order of EMaps into account if there are some.
Comment 9 Moritz Eysholdt CLA 2012-05-16 04:57:14 EDT
I've created a new ticket for the serializer issue: bug 379651
Comment 10 Aaron Digulla CLA 2012-05-16 14:55:44 EDT
(In reply to comment #8)

> > Options: options*=Option*
> > 
> > i.e. "*=" instead of "+=" to say "make this an EMap instead of an EList"
> 
> what would be the "name" in that example?

The name of the child. As in my first comment:

  Option: name=ID '=' value=Value;

Almost all my grammars are more Java like; I almost never do "dumb" grammars.

>[...]
> However, it might not be necessary to extend our grammar language at all. As
> stated earlier in this discussion, you can always customize the Ecore model
> manually. This has just gotten much easier thanks to Xcore. Therefore, you can
> always create your EMaps manually.

I stubbornly refuse to risk getting the Ecore model and the grammar out of sync. I might be game for a solution which generates the xtext/grammar file from a new  model or maybe something that patches the Xtext grammar code generator at runtime (like a config file which overrides defaults).

But I don't want two models in my projects that can get out of sync. I'm one of those guys who can never get this right. I lost many hours of work with the stupid EMF "@Autogenerated NOT" kind of development approach and I now accept that this isn't for me.
Comment 11 Sven Efftinge CLA 2012-05-16 18:52:25 EDT
If you want to get an element from a list by name, not for performance reasons but for convenience, you could also use an extension method:

def <T extends NamedElement> T get(List<T> list, String key) {
  list.findFirst[ name == key ]
}
Comment 12 Aaron Digulla CLA 2012-05-17 08:50:34 EDT
That's a nice workaround but in my case, performance matters. In my grammar, I have nodes which are looked several million times during the code generation step (to see which options are active, what their values are, checks during validation).

My current workaround is create my own model and then copy the values from the EMF model with a tree visitor. It works but it means that I have to maintain the source for a copy of the original model with all lists replaced with maps.

I use this approach because the compiler will tell me when the models get out of sync.
Comment 13 Knut Wannheden CLA 2012-05-18 00:55:50 EDT
As already pointed out you can manually modify the Ecore model to support EMaps. If you don't want to maintain your Ecore model by hand you could also use a post processor for this purpose. I wrote a blog post about how to do this for EMaps some time ago: http://20000frames.blogspot.com/2010/09/working-with-emaps-in-xtext.html.
Comment 14 Aaron Digulla CLA 2012-05-18 08:00:57 EDT
(In reply to comment #13)

> If you don't want to maintain your Ecore model by hand you could also
> use a post processor for this purpose. I wrote a blog post about how to do this
> for EMaps some time ago:
> http://20000frames.blogspot.com/2010/09/working-with-emaps-in-xtext.html.

This sounds exactly like what I'm looking for. How did you find out about ConfigPostProcessor.ext? Google couldn't find it, it's not the Xtext docs and nothing in the Eclipse Community Forums.

Is this an official feature of Xtext? If so, why is there no documentation about it?
Comment 15 Sebastian Zarnekow CLA 2012-05-18 08:02:56 EDT
We discourage the postprocessing of the model inference due to the risk of producing invalid results. I can only repeat the strong advise to use an imported EPackage if you want to specialize it.
Comment 16 Aaron Digulla CLA 2012-05-18 12:01:19 EDT
(In reply to comment #15)

> We discourage the postprocessing of the model inference due to the risk of
> producing invalid results. I can only repeat the strong advise to use an
> imported EPackage if you want to specialize it.

How is maintaining the grammar twice (once as Ecode model and once as Xtext file) less risky?

Also, looking at the example in the blog post, adding EMap support would be three lines of code in the Xtext code generator. How about you simply add that in the core, so we don't need post processing hacks?
Comment 17 Sebastian Zarnekow CLA 2012-05-18 12:10:03 EDT
Aaron,

Note that we do not have any plans nor resources to evaluate EMap support. Given the fact that EMF itself does not use EMaps for e.g. EPackage.getEClassifier(String), I doubt that they are of much value, especially when it comes to model changes / updating maps if the name of the contained instance changes etc.

However, feel free to do that and provide a high quality patch.
Comment 18 Sebastian Zarnekow CLA 2012-05-18 12:10:39 EDT
(In reply to comment #16)
> How is maintaining the grammar twice (once as Ecode model and once as Xtext
> file) less risky?
> 

The imported EPackage is validated against the grammar.
Comment 19 Sebastian Zarnekow CLA 2012-05-18 12:13:19 EDT
(In reply to comment #18)
> The imported EPackage is validated against the grammar.

Sorry that was to fast.

Changes that you do in the post processing step are applied by the editor itself and in the code generator. Loading code into the active editor is error prone since e.g. an endless loop in your postprocessor will simply freeze the grammar editor. Furthermore you could - by accident - remove an EStructuralFeature in your post processor which was already linked to the grammar. No validation rule will detect that. There are more things that users could do by accident thus we do not encourage them using that feature in any way.
Comment 20 Sven Efftinge CLA 2012-05-19 14:58:20 EDT
(In reply to comment #12)
> That's a nice workaround but in my case, performance matters. In my grammar, I
> have nodes which are looked several million times during the code generation
> step (to see which options are active, what their values are, checks during
> validation).

You might want to use a cache (e.g. OnChangeEvictingCache)
Comment 21 Moritz Eysholdt CLA 2012-05-21 09:07:19 EDT
(In reply to comment #16)
> (In reply to comment #15)
> 
> > We discourage the postprocessing of the model inference due to the risk of
> > producing invalid results. I can only repeat the strong advise to use an
> > imported EPackage if you want to specialize it.
> 
> How is maintaining the grammar twice (once as Ecode model and once as Xtext
> file) less risky?

I wouldn't say it's risky since the Xtext editor provides your error markers right away if something gets out-of-sync.

IMHO a good Xtext language also comes with a well-designed Ecore model. After all, this Ecore model defines the API that the majority of your hand-written Java-code will use. And creating a well-designed Ecore model just isn't possible when deriving it from an Xtext grammar. By well-designed I mean proper use of EClass inheritance, markin EClasses abstract/interface such as creating useful EOperations and derived features.

To tweak EClass inheritance you may create never-called parser rules. However, this again pollutes your grammar.
Comment 22 Aaron Digulla CLA 2012-05-21 09:44:27 EDT
After reading some articles on the 'net, I'm starting to see what you're talking about. Some comments:

http://blog.efftinge.de/2009/11/xtext-using-existing-ecore-models.html makes it sound like this is really simple.

But the wiki page http://wiki.eclipse.org/Xtext/FAQ#How_can_I_get_rid_of_the_ecore_model_generation_.28and_use_only_imported_EMF_models.29.3F leaves me confused. Information that I'm missing:

1. Where do I have to put these files?

2. What do you mean by "Make sure you have imported the packages that contain the EMF model in the MANIFEST.MF"??

3. The workflow fragments contain some complex URLs but there is no explanation how I can build them ("platform:/resource/org.xtext.example.mydsl/src/org/other/model/OtherModel.genmodel").

4. "Import the ecore model(s) and use it in your grammar." - How do I import an Ecode model into an existing grammar (see the first step of the FAQ)?

For me, it looks as if the FAQ item tries to answer two questions at once (how do I do that with an existing grammar and how do I do this when I already have an Ecore model but no grammar).

Re comment #21:

> And creating a well-designed Ecore model just isn't
> possible when deriving it from an Xtext grammar.

That may be true but I hard some traumatic memories of my first contact with EMF 2.x a few years ago where I had to edit the model using a tree and a property editor - without any useful documentation (I just had the blue "EMF" book which is a nice ad for EMF but doesn't answer many important questions).

For me, the minimum requirement for such a step would be a detailed explanation how I can convert the generated Ecore model to Emfatic or something that a sane person can understand without having 15 years of modelling experience ... not sure which smiley would be appropriate here...
Comment 23 Sven Efftinge CLA 2012-05-21 09:51:28 EDT
If it's just the indexed access to members, I would really go with an extension method and a cache.
That's not too complicated, is it?
Comment 24 Aaron Digulla CLA 2012-05-23 04:22:20 EDT
(In reply to comment #23)
> If it's just the indexed access to members, I would really go with an extension
> method and a cache.
> That's not too complicated, is it?

The point isn't about complicated. It's just about 1500 lines of boiler plate code that I'd have to write because I need that feature really often ... really almost for every rule in my grammar ... in every grammar that I wrote so far.

My DSL is more like Java and not some simple config language. I can see that this feature would be overkill for simple DSLs where the user can't define their own config options, fields and methods in the DSL. But as soon as you write your own programming language, you start to wonder why Xtext doesn't support this basic feature.

Why do you refuse to add these two lines to your code generator?

process(xtext::GeneratedMetamodel this):
    ePackage.getEClassifier('StringToTypedValueMapEntry').setInstanceClassName('java.util.Map$Entry')
;
Comment 25 Sven Efftinge CLA 2012-05-23 04:49:39 EDT
Emaps cause several problems
1) the key is not getting updated when the model changes
2) it's not possible to have duplicate entries and come up with proper validation messages.
3) relying on just one attribute for the key is not flexible enough.

Also the cached lookup can be implemented generically. So it's more like 5 lines of code.
Comment 26 Eclipse Webmaster CLA 2017-10-31 11:27:06 EDT
Requested via bug 522520.

-M.