Bugzilla – Full Text Bug Listing
| Summary: | Provide a way to suppress operation visibility | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Kenn Hussey <Kenn.Hussey> | ||||||||||||||||
| Component: | Tools | Assignee: | Dave Steinberg <davidms> | ||||||||||||||||
| Status: | VERIFIED FIXED | QA Contact: | |||||||||||||||||
| Severity: | enhancement | ||||||||||||||||||
| Priority: | P3 | CC: | Ed.Merks | ||||||||||||||||
| Version: | 2.4.0 | ||||||||||||||||||
| Target Milestone: | --- | ||||||||||||||||||
| Hardware: | PC | ||||||||||||||||||
| OS: | Windows XP | ||||||||||||||||||
| Whiteboard: | |||||||||||||||||||
| Attachments: |
|
||||||||||||||||||
|
Description
Kenn Hussey
I'll try to get to this tomorrow. Created attachment 87109 [details]
Preliminary patches
It's not quite ready for prime time, but is mostly working.
If an operation collides with a volatile feature being implemented then the body replaces the feature's body. If it collides with a normal feature being implemented, the feature's accessor is redirected to a "Gen" version of it.
I'm just not sure I have the operation filtering working quite right...
Created attachment 87149 [details]
Okay, it's working properly now and is ready for review
I've been thinking about one more addition. I can imagine it might be very handy to introduce fields in the Impl as part of generating an operation with a body.
Created attachment 87150 [details]
Example model
I wonder if "Gen" is really the best thing to use. I suppose we could use "_" or pretty much anything. The argument in favor is that renaming to Gen is exactly what'd you'd do for writing this type of pattern by hand.
It seems your patch doesn't include your changes to EcoreUtil. Created attachment 87163 [details]
This includes the EcoreUtil and EcoreValidator changes
Hi Ed, I've still just been looking at the incomplete patch so far, so I'll look at EcoreUtil and EcoreValidator next, but I thought I'd point out something that looks fishy first... The new stuff you've added to GenClassImpl seems to assume that a volatile feature always results in no getter implementation being implemented. But that's not really true. Of course, there's the case of delegation to a feature map, and there's also a case where a proxy-resolving single-valued volatile feature gets delegated to a basic getter. I haven't looked closely at the set, isSet, and unset. Anyhow, we never get to check getGetAccessorOperation() in the getter pattern for these cases, since we've already rightly handled them. But, no "Gen" suffix is included on the getter name and no corresponding operation is generated (because the volatile flag causes hasCollidingGetAccessorOperation() to return false and causes it to be filtered from getImplementedGenOperations()). So, the specified operation body is lost. If this is to work properly, then every time we figure out whether there's a colliding accessor we really need to take into account all of the cases where we generate something more useful than a TODO comment in all three places. Yuck. Who thought that using operations to specify feature accessor implementations was a good idea again? Created attachment 87171 [details]
Example of the volatile problem
Demonstrates mistreatment of a volatile feature that still has a generated implementation. Compare getPriorityOrders() to getPriorityOrders2().
Created attachment 87183 [details]
Fixes for the described issue
There weren't so many cases and the getter was the worst one. Thanks for looking so closely!
It looks right, and my example works now. Though, I will admit, I didn't examine it quite as closely as I did last time. Hope you were careful. ;) I'm still not sure about "Gen", either. I think I'd rather do something like defaultGetFoo(), since this really is just another generator pattern, much like basicGetFoo(). It may be sort of analogous to the renaming by hand, but it's an entirely different mechanism. It seems very wrong to use "Gen" to distinguish between two generated things. Yes, I'm not so thrilled with Gen either, but I'm not fond of the idea of prefixing rather that postfixing something. The later will help to group them in the outline view and is an easier pattern to generate as well. I think Kenn suggested using '_' which I kind of like. That's what we do when we want to generated getClass but it collides with Object.getClass, or when the getAccessor collides with something in the root implements interface like for getType for SDO which is in DataObject. So I'd prefer to use '_'. What do you think? Yup, I'm okay with that. Created attachment 87264 [details]
Change to use _ instead of Gen
Kenn,
I'll wait for you to verify that this causes no further problems for UML before I commit it.
I made the corresponding changes in UML2 and everything seems to work OK; the validation errors are gone now. Surprisingly, I didn't have to make any changes to the UML2 code generator... The changes are committed to CVS for 2.4. Fix in CVS. Fix available in HEAD: 2.4.0.I200801221930. |