Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 319273 - Generate getters and setters can't handle function pointers
Summary: Generate getters and setters can't handle function pointers
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-refactoring (show other bugs)
Version: 7.0   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Elena Laskavaia CLA
QA Contact: Emanuel Graf CLA
URL:
Whiteboard:
Keywords:
Depends on: 319111
Blocks:
  Show dependency tree
 
Reported: 2010-07-08 10:10 EDT by Emanuel Graf CLA
Modified: 2010-08-01 22:23 EDT (History)
3 users (show)

See Also:

Attachments
test case (1.24 KB, patch)
2010-07-08 10:11 EDT, Emanuel Graf CLA 		elaskavaia.cdt: iplog+
Details | Diff
Fix for getter declaration (5.30 KB, patch)
2010-07-20 18:14 EDT, Tomasz Wesolowski CLA 		elaskavaia.cdt: iplog+
Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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