Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 321617 - ClassTypeHelper methods aren't aware of covariant return types
Summary: ClassTypeHelper methods aren't aware of covariant return types
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-parser (show other bugs)
Version: 7.0.1   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 7.0.1   Edit
Assignee: Markus Schorn CLA
QA Contact: Mike Kucera CLA
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2010-08-03 11:31 EDT by Tomasz Wesolowski CLA
Modified: 2010-08-04 10:23 EDT (History)
1 user (show)

See Also:


Attachments
test case (2.42 KB, patch)
2010-08-03 11:32 EDT, Tomasz Wesolowski CLA
mschorn.eclipse: iplog+
Details | Diff
proposed fix (5.08 KB, patch)
2010-08-03 11:46 EDT, Tomasz Wesolowski CLA
mschorn.eclipse: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomasz Wesolowski CLA 2010-08-03 11:31:41 EDT
Some methods assume that an overriding function will have same function type as the overridden function, which isn't the case when covariant return types are used. Thus some cases are not detected in isVirtual, isOverrider, findOverridden nad findOverrides.
Comment 1 Tomasz Wesolowski CLA 2010-08-03 11:32:14 EDT
Created attachment 175788 [details]
test case
Comment 2 Tomasz Wesolowski CLA 2010-08-03 11:46:48 EDT
Created attachment 175791 [details]
proposed fix

My proposed solution is to ignore the returned type when comparing function types.

Note that when everything besides return type matches in the functions, then it's known that the method is _intended_ to be the override, and not i.e. another overload.

After that, comparing the actual covariance of return types could be done alongside with comparing the strictness of throw() declaration to check if the code would actually compile, but both of those are more expensive checks. Neither of those is necessary to answer isOverride() and others.
We might want to have that as a codan checker.
Comment 3 Markus Schorn CLA 2010-08-04 10:05:47 EDT
(In reply to comment #2)
> Created an attachment (id=175791) [details]
> proposed fix
> My proposed solution is to ignore the returned type when comparing function
> types.
> Note that when everything besides return type matches in the functions, then
> it's known that the method is _intended_ to be the override, and not i.e.
> another overload.
> After that, comparing the actual covariance of return types could be done
> alongside with comparing the strictness of throw() declaration to check if the
> code would actually compile, but both of those are more expensive checks.
> Neither of those is necessary to answer isOverride() and others.
> We might want to have that as a codan checker.
Perfect, that's the right choice.
Comment 4 Markus Schorn CLA 2010-08-04 10:09:08 EDT
Thanks for the patches,
fixed in 7.0.1 and 8.0 > 20100804.