Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 325196 - [core] DefaultEcoreElementFactory casts to BasicEList<EObject>
Summary: [core] DefaultEcoreElementFactory casts to BasicEList<EObject>
Status: CLOSED FIXED
Alias: None
Product: TMF
Classification: Modeling
Component: Xtext (show other bugs)
Version: 1.0.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: M2   Edit
Assignee: Project Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 325984 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-09-14 03:16 EDT by Knut Wannheden CLA
Modified: 2017-09-19 17:55 EDT (History)
4 users (show)

See Also:
sebastian.zarnekow: helios+
sebastian.zarnekow: indigo+

Attachments
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Knut Wannheden CLA 2010-09-14 03:16:45 EDT 
With bug 307665 (see comment 10) a performance change was introduced which casts the structural feature to BasicEList<EObject> and invokes the addUnique() method instead of simply add() on EList.

This works fine for normal lists, but not for EMaps. The code should instead cast to InternalEList<EObject> which all many values structural features can be expected to implement. This interface also declares the addUnique() method.
Comment 1 Sebastian Zarnekow CLA 2010-09-14 06:00:15 EDT 
Please check for other ocurrences of BasicEList, too.
Comment 2 Knut Wannheden CLA 2010-09-14 09:13:51 EDT 
fixed
Comment 3 Sebastian Zarnekow CLA 2010-09-22 17:50:33 EDT 
*** Bug 325984 has been marked as a duplicate of this bug. ***
Comment 4 Julien Bigot CLA 2010-09-23 15:59:18 EDT 
Could you please tell us in what revision this has been solved and in what release we can expect it.

Please also note that nothing prevents list exposed by models to not be based on InternalEList at all, for example in the case of a derived reference implemented by the user. I didn't see how you solved the problem but it might be safer to check if the object is actually an instance of BasicEList and use the simple "add" method otherwise as a fallback.
Comment 5 Sebastian Zarnekow CLA 2010-09-23 16:16:24 EDT 
see comment #4
Comment 6 Sebastian Zarnekow CLA 2010-09-23 16:25:01 EDT 
We should discuss this as the JavaDoc InternalEList states:

An interface that the value of multi-valued feature
is expected to implement.


I'd like to close this again as the current impl meets the expectations of implementators and clients.
Comment 7 Julien Bigot CLA 2010-09-23 16:32:20 EDT 
You're right, it seems you're supposed to implement InternalEList even if that's not required by the generated interface ... I don't really get the rationale behind that but that's not the matter here.

You're right, you can leave the bug closed.

Could you please answer my other question regarding the revision and release where the fix can be found.

Thank you.
Comment 8 Sebastian Zarnekow CLA 2010-09-23 16:48:15 EDT 
The fix is part of the Indigo Stream of Xtext. A first milestone is likely to appear next Tuesday.
Comment 9 Sven Efftinge CLA 2010-09-23 16:53:12 EDT 
Note that such early milestones are not intended to be used in production.

You should instead fix this by subclass the org.eclipse.xtext.parser.DefaultEcoreElementFactory
Comment 10 Jan Koehnlein CLA 2010-11-05 11:40:10 EDT 
Fix backported to 1.0.2
Comment 11 Karsten Thoms CLA 2017-09-19 17:44:32 EDT 
Closing all bugs that were set to RESOLVED before Neon.0
Comment 12 Karsten Thoms CLA 2017-09-19 17:55:49 EDT 
Closing all bugs that were set to RESOLVED before Neon.0