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

Bug 321617

Summary: ClassTypeHelper methods aren't aware of covariant return types
Product: [Tools] CDT Reporter: Tomasz Wesolowski <kosashi>
Component: cdt-parserAssignee: Markus Schorn <mschorn.eclipse>
Status: RESOLVED FIXED QA Contact: Mike Kucera <mikekucera>
Severity: normal    
Priority: P3 CC: yevshif
Version: 7.0.1Keywords: contributed
Target Milestone: 7.0.1   
Hardware: PC   
OS: Windows 7   
Whiteboard:
Attachments:
Description Flags
test case
mschorn.eclipse: iplog+
proposed fix mschorn.eclipse: iplog+

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.