| Re: [cdt-dev] refactoring that replaces multiple nodes in the same branch of a subtree |
|
Hi Thomas, Now I think I understand. The key here is that ASTRewrite#replace returns an ASTRewrite object that can be used for further rewriting the replacement node. This is exactly what I need I think. And of course this
is a better solution. I’ll give it a try. Thanks a lot! Andrey From: cdt-dev-bounces@xxxxxxxxxxx [mailto:cdt-dev-bounces@xxxxxxxxxxx]
On Behalf Of Corbat Thomas Hi Andrey Well, then I probably had not been that clear. In your approach you mix two ways of modifying the AST: (1) Using ASTRewrite for the base case and (2) direct node replacement for the parameter. While it is one option there
are alternatives, which might be better if you have replacements that need to be recursive over several steps. What I meant: When you replace the assignment with a function call using ASTRewrite.replace(), you get a NEW ASTRewrite as a result of this operation. This new ASTRewrite is responsible to
collect modifications that are performed to the newly inserted node (i.e. your function call _expression_). Using this new ASTRewrite (which is a child of the original ASTRewrite) you can replace nodes in the new branch created by the first replace operation.
The problem is, that if you continue with your visitor in the original AST no modifications will be possible to the new branch (since the nodes are copied and you traverse the original nodes). Thus I mean you should skip the replaced part in your visitor,
set the new ASTRewrite as current rewrite and invoke the visitor on your new branch (the call _expression_). In this approach you just need to be aware that bindings cannot be resolved on the copied nodes. If this was required you need to get the original node first (getOriginalNode()
– which only works if you have copied the nodes with locations). I know this infrastructure might be tricky to use, if necessary I could provide a more specific example (perhaps with code). Regards Thomas From: cdt-dev-bounces@xxxxxxxxxxx [mailto:cdt-dev-bounces@xxxxxxxxxxx]
On Behalf Of Ayupov, Andrey Hi Thomas, It seems what you’re proposing is to do replacements in two phases. In the first one, I replace write with one ASTRewrite, then commit the change, then parse the new AST after the first rewrite and do the second
rewrite? Is that right? I think this will not be exactly what I want, because I want to see a diff window in the RefactoringWizard that correspond to the compound rewrite. What I am thinking to do now is to copy a right-hand-side subtree for the write assignment and then in the copied tree, find a copy of the node that meant to be rewritten as well and then change it to the one
that looks like I want. It will have some limitations on what kind of nodes I will be able to rewrite, but should do for the cases I have. Here is what I do: Original _expression_: a = a+1 I will find the node “=” with two children “a” and “a+1”. I will copy the _expression_ “a+1” as a right-hand side for the assignment and then will search in that copied tree if it has other nodes that I needed
to rewrite, which will be the “a”. Then I will just substitute node “a” with “a.read()” (without any ASTRewrite, because since it’s a copy - it is mutable). So in the copied and modified subtree I will have _expression_ “a.read()+1”. Finally I will do ASTRewrite.replace
node “=” with a function call that has the copied and modified subtree as an argument to the function.
Anything more elegant is welcome! Thanks, Andrey From: cdt-dev-bounces@xxxxxxxxxxx [mailto:cdt-dev-bounces@xxxxxxxxxxx]
On Behalf Of Corbat Thomas Hi Audrey Let me summarize to see if I got your intention. 1. You start with "a = a + 1;" 2. You need to replace the assignment with a write call 3. Nested in the replacement you need to replace the access to a with a read call But you are losing the nested replace because you copy the right-hand operand before assigning it as parameter of the write call. Right so far? Now, what exactly are you trying to replace in your replacement? - After your first replacement you should end up with something like a.write(a + 1);. I guess that except the statement all nodes of the _expression_ are new (or copied nodes). Therefore, you also need to replace
those nodes. So instead of replacing the original id-_expression_ "a" you would need to replace it's copy. Probably you will skip the branch in your current visitor and start over on the argument. - It is mandatory that the second replacement is created with the new ASTRewrite, which is the result of the first replace operation. Otherwise you will end up with IllegalArgumentExceptions because the synthetic
nodes do not have the original translation unit as parent. If you need to operate only on the original AST you can also create the call _expression_ with a dummy node as argument and then (on the new ASTRewrite) replace this dummy node with your right-hand operand of the
assignment _expression_. Then you will still have the original nodes for further replace operations. I hope this helps, otherwise we'll figure out another solution. Regards Thomas From:
cdt-dev-bounces@xxxxxxxxxxx
[mailto:cdt-dev-bounces@xxxxxxxxxxx]
On Behalf Of Ayupov, Andrey Hi all, It should be something frequently hit, but I can’t find an elegant solution. I need to do a refactoring that involves replacing two nodes located in one branch of a subtree. For example, I need to replace: a = a + 1; with: a.write(a.read() + 1); So I need to rewrite the assignment operator (“a = …“) as well as the subexpression (“a”) in the subtree. I cannot do it with two replace rewrites, because when I rewrite the assignment operator, I would use the “copy” method on the subtree
but it would not include any rewrites happened below the tree as far as I can tell. So what I can do is to detect if the subtree has any nodes that need rewrites, and then traverse it by recreating the tree and substituting the node that needs a rewrite with
a new node, but this solution seems to need code for recreating all types of ASTNodes! Do you see how I can do it another way that is more elegant? Something like a shallow copy for ASTNodes would help in the solution I mentioned, but I couldn’t find such
a method either. Thanks, Andrey |