On Wed, Sep 7, 2011 at 4:15 AM, Lukas Felber
<lfelber@xxxxxx> wrote:
Hi
Firstly, sorry to reply so late (was busy with other stuff and got
carried away there)...
> This is a very difficult problem to solve in the general case. I
> propose that we punt on attempts to reflect changes in the unsaved
> files that only indirectly affect the code being refactored. To
> illustrate what I mean by "indirectly" lets consider few existing
> refactorings.
I think this will be very hard to achieve in a way that ensures that
generated code will really be correct (which a refactoring should be in
my opinion). Also, this will make the development of new refactorings
very error-prone.
A much more reliable way to implements reliably generate new code would
be to have the indexer up to date with unsaved changes.
Having index reflect unsaved changes is an interesting but hard to implement idea. There is no question that such index would be good for refactoring and navigation. The question is how to implement it efficiently. Do you have a proposal on how indexing of unsaved changes should be done?
-sergey
> For Extract Local Variable all unsaved changes outside of the file
> being changed can be ignored.
I don't think that this is true. What if the type returned by the
_expression_ that is extracted changes? (which might happen in any other
code file). Such changes would have to be detected and cannot reliably
be achieved without an indexer which is up to date.
> This seems to be a better tradeoff than forcing all files to be saved
> and waiting for the indexer to reindex them. Automatic refactoring
> makes sense only when it is faster than making the same change by
> hand.
There is other helpful stuff in automated refactoring. A developer for
example does not have to know/find out about what the _expression_'s type
is of the variable he will create. But in general I agree that automated
refactoring should be lots faster than when typed by hand.
> For Generate Getters and Setters we can ignore unsaved changes in all
> files except the one containing the class definition and the one the
> generated method definitions would be added to.
Also this trade-off is not completely reliable. Suppose there is a class
A ( source A.cpp and header A.h ) containing an int field "myInt". Now I
(assumed to be a bit chaotic c++ developer) put only an implementation
of a "int A::getMyInt() const" getter into the (wrong) source file
XY.cpp (unsaved). When now choosing to add a getter for myInt (in A.h)
there will be no way to ensure that there will not be double
implementations of A::getMyInt.
I know the reasoning here is a bit unfair because this also does not
works right now with saved files.
If one had an indexer which can be adapted with unsaved changes (as
suggested above), totally new opportunities would arise. Consider this:
one could first generate the getter's declaration in A.h. Then
temporarily apply this code to the indexer (not the editor itself but
only the indexer) and try to find an implementation (the one in XY.cpp
then would have a correct binding and would thus be found). if one is
found, all is good (if it returns the correct integer myInt, otherwise
user should be warned). Otherwise, as a second step of the refactoring,
one would also generate an implementation.
> For Implement Method we can ignore changes in all files except the
> ones being changed and the ones where the methods being overridden are
> declared.
The problem here is the same as described above. What if an
implementation is added (unsaved) in any file in which CDT would not
expect the implementation to be?
> For Rename refactoring we should be able to detect references
> introduced after the last saving of files, but can ignore other
> unsaved changes, for example a typedef somewhere, that could
> potentially add more or eliminate some of the rename candidates.
This detection of "references introduced after the last saving" would
imply to generate all ASTs for all unsaved files. I have the feeling
that this will not scale well at all if many ASTs are newly created only
to see if there might be refs to the rename candidate.
Isn't the indexer exactly the tool wich was created to make such things
not necessary anymore?
Lukas
