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

Bug 211270

Summary:

error in generated code for two level override, if return type changes

Product:

[Modeling] EMF

Reporter:

Philipp W. Kutter <kutter>

Component:

Tools

Assignee:

Dave Steinberg <davidms>

Status:

VERIFIED FIXED

QA Contact:

Severity:

normal

Priority:

CC:

Ed.Merks

Version:

2.4.0

Target Milestone:

---

Hardware:

OS:

Windows XP

Whiteboard:

Attachments:

Description	Flags
Example Projects	none
Example Screenshot	none
Patches to address the issue.	none
I'll mark it as a patch this type.	none

Description Philipp W. Kutter

2007-11-28 11:43:02 EST

Created attachment 83991 [details]
Example Projects

Build ID: I20071101-2000

Steps To Reproduce:
1. Install the included example projects
2. Reload/Regenerate code
3. Look at the error in Super C



More information:
Given the following class diagram (attachment Override.jpg) everything works well, until the SuperSuperC class is added.

The generator by error thinks that the two getRef operations are the same, and only generates the signature for SuperSuperC.

By simply adding "public SuperD getRef() {return null;};"
to SuperCImpl, the problem is fixed. 

Ed Merks answered, 20071128:
>We did nothing to take into account changes to the return >type in terms of whether to generate a method body or not.  >In this case I suppose it would be nice to generate:
>
>        @Override
>        public SuperD getRef()
>        {
>          return (SuperD)super.getRef();
>        }
>That would be a new pattern in the template.
>Please open a bugzilla with this example.

Comment 1 Philipp W. Kutter

2007-11-28 11:43:27 EST

Created attachment 83992 [details]
Example Screenshot

Comment 2 Ed Merks

2007-12-26 07:52:56 EST

Created attachment 85836 [details]
Patches to address the issue.

The idea is to check before discarding a method if the return type is different.  Unfortunately we can't in general know how to deal with proper subtype checking because with EDataTypes we're just dealing with class names.  So I could imagine complex situations where we ought to be finding the "overload" is the most specific type.

Comment 3 Ed Merks

2007-12-26 07:58:16 EST

Created attachment 85838 [details]
I'll mark it as a patch this type.

Comment 4 Ed Merks

2008-01-03 09:53:52 EST

Dave,

Does this look okay?

Comment 5 Dave Steinberg

2008-01-03 13:49:53 EST

It seems fine as a minimal solution. Of course, this still allows invalid code to be generated in the case where it's not type compatible. It would be nice to (you guessed it!) have a constraint that does the type analysis for classes and perhaps issues a warning for differing data types, since they could possibly result in generating invalid code.

Also, the bug report quoted you suggesting a pattern that calls super and casts, but I don't see that in the patch. Did you intend to add it?

Comment 6 Ed Merks

2008-01-03 14:00:39 EST

No, I'm too lazy to take the approach of trying to generate a method body in some fancy way.  It's not clear that such a result will always be exactly what's intended so a TODO seems reasonable.

The constraint on the type is difficult to enforce because in general I can't know the relationship between two different EDataTypes.  There are quite  few possible Java types of problems, such of overloaded signatures that are actually collisions rather than proper overloads that I just can't do in general...

Comment 7 Dave Steinberg

2008-01-03 14:34:10 EST

1. Cool, and I agree.  I was just checking to make sure you had done what you intended, since your previous comment suggested otherwise.

2. My suggestion was to only enforce the constraint in the case of classes, and just to issue a warning any time data types do not match exactly. My motivation is that some people might have invalid cases in their models already, which were just being ignored by the generator previously and will now generate code with errors. It might help them figure out what's wrong if we present an error (for classes) or warning (for data types). Again, it's just a suggestion; this definitely seems a corner case.

Comment 8 Ed Merks

2008-01-05 08:49:42 EST

The changes are committed to CVS for 2.4.

Comment 9 Nick Boldt

2008-01-09 10:28:59 EST

Fixed in I200801090807.

Comment 10 Nick Boldt

2008-01-28 16:41:59 EST

Move to verified as per bug 206558.