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

Bug 319273

Summary: Generate getters and setters can't handle function pointers
Product: [Tools] CDT Reporter: Emanuel Graf <emanuel>
Component: cdt-refactoringAssignee: Elena Laskavaia <elaskavaia.cdt>
Status: RESOLVED FIXED QA Contact: Emanuel Graf <emanuel>
Severity: normal    
Priority: P3 CC: eclipse.sprigogin, elaskavaia.cdt, yevshif
Version: 7.0   
Target Milestone: ---   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on: 319111    
Bug Blocks:    
Attachments:
Description Flags
test case
elaskavaia.cdt: iplog+
Fix for getter declaration elaskavaia.cdt: iplog+

Description Emanuel Graf CLA 2010-07-08 10:10:00 EDT 
Generate getters and setters can not handle funtion pointers. The return / parameter type is wrong.
Comment 1 Emanuel Graf CLA 2010-07-08 10:11:58 EDT 
Created attachment 173775 [details]
test case

The patch to fix this is coming soon.
Comment 2 Tomasz Wesolowski CLA 2010-07-08 11:02:35 EDT 
I've had a look on the test case.

Is it consistent behaviour to generate a typedef automatically?
I agree that it's good style but it might be more consistent to specify the return type as-it-is, e.g. typedef return type if the field's type is a typedef, or a literal return type otherwise. Even though it's good style to typedef complex types, creating a typedef every time may not always be the user's intention. 

Let's consider both options to solve this before deciding on one.

Note that the same problem (wrong return type) exists for pointers to arrays and even for simple arrays.

Also, some more issues there - see bug 319278 and bug 319279.
Comment 3 Tomasz Wesolowski CLA 2010-07-20 17:57:35 EDT 
Hah. Both your test case and my test code I've used so far used an array of function pointers - I've noticed that the refactoring doesn't even notice a simple function pointer. The reason for that is it tests the outermost IASTDeclarator of each IASTSimpleDeclaration for being an instance of IASTFunctionDeclarator, instead of the innermost one.

For the exact same reason, a function declaration like this:
int (*(*aFunction())[2]);
would be interpreted as a field and you can try to generate getter and setter for it.
Comment 4 Tomasz Wesolowski CLA 2010-07-20 18:14:48 EDT 
Created attachment 174799 [details]
Fix for getter declaration

After this patch, proper getter declarators are generated. Setter declarators seem to be OK in terms of the types (after patch from bug 319111), I guess?

There's one more problem here. Create a new class like this:

class A {
    int (*(*aFunction())[2]);
    int (*fp)();
};

Try to generate getter and setter for the field 'fp'. The generated methods seem OK, but notice what happens to the declaration of aFunction. I'll have a look on this tomorrow.
Comment 5 Tomasz Wesolowski CLA 2010-07-27 18:12:22 EDT 
This turned out to be a bit more complex than I expected. :)

First of all, the issue from comment 4 proved to be a problem with ASTRewrite, not this refactoring. See bug 321069.
Another thing I've found here is bug 321071.
I'm kind of surprised that those two went unnoticed for so long.

I'd recommend to stay with this solution (getter and setter with type declared literally as in field declaration) as it's the most straightforward one (see Principle of Least Astonishment). No objections?
Comment 6 Elena Laskavaia CLA 2010-08-01 21:56:26 EDT 
Patch applied to 8.0, thanks