Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 322163

Summary:	Silent erroneous 'EClass'.eInvoke results when genmodel 'Operation Reflection' set false
Product:	[Modeling] EMF	Reporter:	Ed Willink <ed>
Component:	Core	Assignee:	Ed Merks <Ed.Merks>
Status:	RESOLVED WONTFIX	QA Contact:
Severity:	enhancement
Priority:	P3	CC:	Kenn.Hussey
Version:	2.6.0
Target Milestone:	---
Hardware:	PC
OS:	Windows Vista
Whiteboard:
Bug Depends on:
Bug Blocks:	322159

Description Ed Willink

2010-08-09 13:28:00 EDT

genmodel suppresses the generation of the derived EClass.eInvoke method if the 'Operation Reflection' option is set false, however there is nothing to prevent an evaluation such as OCL validation invoking eInvoke, and going direct to EClass.eInvoke where for instance operationId 0 invokes eClass() and probably results in a CCE. If the return type is compatible, no error appears at all.

Since 'Operation Reflection' false is the default and users are unlikely to read the documentation, this is a trap almost guaranteed to catch out anyone who embeds an OCL operation call within an OCL validation.

In Bug 322159 MDT/OCL provides a fix by doing a Java reflection test for the derived eInvoke, falling back on getInvocationDelegate().dynamicInvoke when a NSME occurs (and also generating an Error Log warning.) Tnhis trest of course costs all users, although a configuration option allows it to be mitigated.

The space saving of the 'Operation Reflection' false may well be justified, but the broken behavior is undesirable.

Suggestion: when 'Operation Reflection' is false, an override of eInvoke be provided that just calls getInvocationDelegate().dynamicInvoke() so that the functionality is always correct. This would cost normal users nothing in speed with a small code size cost for a simple method that rescues misconfigured usage.

Comment 1 Ed Merks

2010-08-09 14:06:20 EDT

I'm not sure I understand the scenario.   Are we talking about trying to invoke operations on a generated class that defines operations but hasn't elected to generate support for reflective operation invocation?  When assertions are enabled, I'd expect that to fail which seems sufficient...

Comment 2 Ed Willink

2010-08-09 14:15:18 EDT

Yes. Invoking an operation that is not supported.

Unfortunately nearly everything just works and no assertions fail. For OCL, which assumes that EClass.eInvoke just works (since DynamicEObject redirects) the missing override means that the wrong operation is executed. Errors only occur when argument accesses fail, or more likely when the return type gets a CCE. eInvoke is available but broken for derived classes, hence the OCL workaround to test for the missing override and behave as DynamicEObject accordingly.

The OCL workaround costs execution time; not really a problem while OCL execution is interpreted and slow, but could become embarassing once OCL generates optimised Java.

Comment 3 Ed Merks

2010-08-09 14:35:28 EDT

What happens for models that extend MinimalEObjectImpl?  That's a better choice than EObjectImpl for most models...  

I'd really much rather not generate anything related to operation invocation when the client is not interested in supporting it.  Sure that means less than ideal failure behavior when others try to invoke operations, but that's the client's decision.

Comment 4 Ed Willink

2010-08-09 16:06:32 EDT

Mmm. MinimalEObjectImpl. Yes that's a different dimension. Should then work fine.

So my suggestion would have to be: override eInvoke if the root 'EObject' has an eInvoke that switches on operationId. Horrible. Derives from EObjectImpl; slightly less horrible.

The problem remains that the default, which most users are likely to use, with EObjectImpl root without Operation Reflection can give erroneous eInvoke behaviour that may arise if there is an invocationDelegate.

genmodel could issue a warning for this hazard. MDT/OCL can detect it with additional run-time overhead.

Providing genmodel with an advice/warning facility could be useful, but obviously non-trivial since it is not currently there.

Comment 5 Ed Merks

2012-10-05 01:10:32 EDT

The default for Xcore models is to support operation reflection so that should help make this issue less interesting.