Bugzilla – Full Text Bug Listing
| Summary: | Invalid access to ToStringGenFeatures for a derived feature | ||
|---|---|---|---|
| Product: | [Modeling] EMF | Reporter: | Ed Willink <ed> |
| Component: | Core | Assignee: | Ed Merks <Ed.Merks> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | eclipse |
| Version: | 2.7.0 | ||
| Target Milestone: | --- | ||
| Hardware: | PC | ||
| OS: | Windows Vista | ||
| Whiteboard: | |||
I would expect there to be a field unless the feature is marked volatile because it's computed like this in GenClassImpl
public List<GenFeature> getToStringGenFeatures()
{
return
collectGenFeatures
(getImplementedGenClasses(),
null,
new GenFeatureFilter()
{
public boolean accept(GenFeature genFeature)
{
return genFeature.isField() && !genFeature.isReferenceType();
}
});
}
where GenFeatureImpl is like this:
public boolean isField()
{
return !isContainer() && !isVolatile();
}
I guess there is some setting delegate thing involved here? Maybe we need to test for that in the isField method? We definitely don't want to call anything that's might do a computation that could fail and cause toString to have runtime exceptions so the suggested fix isn't a good idea...
I'm not entirely clear on the relative roles of GenClass/GenFeature isField since there is quite a bit of bypassing/augmentation.
But, today, you're right eSettingDelegates need to be excluded because for a derived, !volatile feature the generated code is:
public String getS() {
return (String)S__ESETTING_DELEGATE.dynamicGet(this, null, 0, true, false);
}
with no field generated that can be used by toString().
-----------
Simplest option is to extend the accept body you identified to
return genFeature.isField() && !genFeature.isReferenceType() && !genFeature.hasSettingDelegate();
Perhaps promoted to within isField().
-----------
Alternatively some control complexity could be migrated by defining that derived, !volatile represents a cached setting delegate and so the field is generated, it can feature in toString() and get is recoded vaguely as
public String getS() {
if (s == S__DEFAULT) {
s = (String)S__ESETTING_DELEGATE.dynamicGet(this, null, 0, true, false);
}
return s;
}
NB. The same outer cache/guard wrapper is applicable if/when get-body annotations are supported.
The missing support for a cached setting delegate is arguably a bug.
------------
Which way do you want to go? I'll try to align a bug 358570 patch to this so that the EMF change can go into M3 at the same time as the OCL2Java generator.
For this particular issue, updating the GenFeatureImpl.isField method seems ideal. Lack of support for caching seems potentially problematic, although how to cache a computed thing and yet know when it needs recomputation is also an open (but separate) issue (In reply to comment #3) > Lack of support for caching seems potentially problematic, although how > to cache a computed thing and yet know when it needs recomputation is also an > open (but separate) issue I think this is exactly where Axel's Impact Analyzer fits in. For OCL this is a tractable issue. Axel: Background. The OCL2Java code generator is rewriting the in-memory Ecore model during genmodel's pregenerate phase to provide Java 'body' details for EMF EAnnotations on EOperations, to replace 'EConstraints' by EOperations with similar annotations and to exploit a new (bug 358570) Java 'body' (and 'isset' and 'set' and 'unset') detail for EMF EAnnotations on EStructuralFeatures. This works nicely since it then allows all the subsequent genmodel processing top work as normal. OCL only needs an EStructuralFeature 'body' and possibly 'isset' EAnnotation. Xcore may need 'set' and 'unset' too. Java bodies on EStructuralFeatures are an enhancement so there is backward compatibility issue. A volatile EStructuralFeature is clearly uncached. A !volatile EStructuralFeature is plausibly cached but may go stale. For many applications a one shot lazy computation is perfectly adequate so it seems more helpful to provide the cache than to enforce recomputation. The user has a choice: volatile and recompute (always works), !volatile and refresh (always works for OCL, a diagnosable limitation of XCore). So yes, the refresh is a separate issue, but providing the cache seems to make !volatile more consistent. The additional guard has been committed to CVS for 2.8. The changes are available in builds. |
[I'm sure this is a duplicate, but I can't find it]. The generated toString() for a derived feature makes direct access to the non-existent derived field. Changing the final else case in Class.javajet for toString() from <%} else {%> result.append(<%=genFeature.getSafeName()%>); <%}%> to <%} else if (genFeature.isDerived()){%> result.append(get<%=genFeature.getAccessorName()%>()); <%} else {%> result.append(<%=genFeature.getSafeName()%>); <%}%> ensures that getXxx() rather than x appears in the generated code.