Author |
Topic |
|
dewtell
Junior Member
18 Posts |
Posted - Dec 20 2013 : 8:06:29 PM
|
Using VAX version 10.7.1940.0 built 2013.05.30, with Visual C++ 2008: Extract method doesn't seem to pass variables by reference when they may be modified by a subroutine call - either by having their address taken, or by being passed to a subroutine as a reference parameter.
From my search of the forums, it looks like case=1152 might address this, but I can't seem to find any documentation on the case itself - is this available to users?
My example - starting with:
int junkRoutine(int &foo) { int bar = foo + 1;
float baz = 3.14; float rem = frexp(baz, &bar);
return bar+rem; }
When extracting the middle two lines, I get: void newSub( int bar );
int junkRoutine(int &foo) { int bar = foo + 1;
newSub(bar);
return bar+rem; }
void newSub( int bar ) { float baz = 3.14; float rem = frexp(baz, &bar); }
which is incorrect, as bar needs to be passed by reference to newSub, since it is modified by the call to frexp.
The documentation here http://docs.wholetomato.com/default.asp?W155 seems to suggest the value/reference decision is made by whether or not the variable appears on the LHS of an assignment. Would it be difficult to add "or if its address is taken, or it is passed as a non-const reference to a subroutine" to this?
We've gotten burned by this a couple of times. In the above example the problem is obvious, but in a longer stretch of code, particularly one that makes a lot of function calls, some of which use reference parameters, it can be quite difficult to see when the extraction has screwed up.
|
|
dewtell
Junior Member
18 Posts |
Posted - Dec 20 2013 : 8:11:18 PM
|
In addition to the reference/value issue, I also notice that my example also has the known issue about moving a local variable declaration inside the extracted routine when it is used later (with rem). I was too focused on getting a good example to show the former to notice this - but at least the compiler will flag the latter problem, so it is relatively easily fixed, unlike the former. |
|
|
feline
Whole Tomato Software
United Kingdom
19024 Posts |
Posted - Dec 23 2013 : 9:45:39 PM
|
Thank you for the clear bug report. The first problem, the parameter not being passed by reference is actually covered by:
case=49455
which looks at this precise problem. The difficulty here is that we need to locate and study every function call that uses the variable in the extracted code, which is taking the parsing we have to do a step or three further, so its not quite as simple as it sounds unfortunately. But we are looking at this, and are aware it is a problem.
Currently if you have selected the declaration of a local variable VA expects you to have also selected all instances of the variable. We do need to check to make sure this has really happened before doing the extract:
case=2063 |
zen is the art of being at one with the two'ness |
|
|
dewtell
Junior Member
18 Posts |
Posted - Dec 23 2013 : 10:29:31 PM
|
Thanks. Is there a way to read the documentation for a case number?
|
|
|
feline
Whole Tomato Software
United Kingdom
19024 Posts |
Posted - Dec 30 2013 : 4:21:55 PM
|
Unfortunately not currently, no. But if you ask we can check up for you. This thread should be updated when this is fixed, and the fixes are also listed in the release notes for new builds. |
zen is the art of being at one with the two'ness |
|
|
|
Topic |
|