Skip to main content
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [cdt-dev] ASTRewrite: Comment handling 
Hi Simon
I investigated into this since your argumentation sounds logical. The reason why there is no check, testing if the node originates from the same translation unit, is, that the comment map at this point is expected to already contain the comment assignment to the node. In cdt there is one example where this is actually happening: the "toggle function refactoring". The bad thing is, that toggle function is doing the adding of the comment-map entries manually, which means that any developer wanting to create a refactoring needs to (1) now about this and (2) manually/correctly handle this in order to not silently loose comments.
Since this is a very unsupportive way towards the refactoring-framework-user, I will create a ticket (soon) for this and try to automate this. The approach will be that when (using an ASTRewrite) a node is inserted/added, the comments originating of the node's translation unit will be merged into the current commentMap. Like this, the comment map will know of all the comments which will be involved when rewriting code. 

Do you find this reasonable Simon?

Lukas


On 29.01.2014 15:50, Simon Taddiken wrote:

Hi again,

I feel like I've found another strange behavior concerning writing
comments back. The ASTWriterVisitor has the following method:

     private List<IASTComment> getLeadingComments(IASTNode node) {
         List<IASTComment> leadingComments =
commentMap.getLeadingCommentsForNode(node);
         IASTNode originalNode = node.getOriginalNode();
         if (originalNode != node)
leadingComments.addAll(commentMap.getLeadingCommentsForNode(originalNode));
         return leadingComments;
     }

This copies comments back from the original node to its copy, but is
uses the same commentMap for looking up the comments of the original
node. So this copying can only work if the source and target translation
unit are the same. If they are not the same, original comments are not
copied because they are contained within another comment map.
If I decide to copy comments myself, they might be added twice because
of the above method, if I do not handle the comments, they might not be
copied at all. I feel like the ToggleMethodRefactoring (which does
similar things like I do) already needs to handle too much special cases
for correct handling of the comments.

It might as well be that my code is a little screwed up but I'm trying
to figure out what the correct way of moving a method including all of
its comments between different translation units would be.

Greetings
Simon

On 29.01.2014 14:38, Corbat Thomas wrote:

Hi Simon

At a glance, that indeed looks like a bug. It probably doesn't make
sense to have a comment assigned to an (AST) node after the node
enclosing the comment. Could you please file a bug, then we (Lukas
Felber and I) will have a look at it.

Thanks a lot
Thomas

-----Original Message-----
From: cdt-dev-bounces@xxxxxxxxxxx [mailto:cdt-dev-bounces@xxxxxxxxxxx]
On Behalf Of Simon Taddiken
Sent: Mittwoch, 29. Januar 2014 14:21
To: cdt-dev@xxxxxxxxxxx
Subject: [cdt-dev] ASTRewrite: Comment handling

Hi,

I'm currently working with the ASTRewrite for moving methods around
and stumbled upon the following which I consider to be a bug. Given
the following code snippet:

A.h
#ifndef A_H_
#define A_H_

class Base {
};
class Derived : Base {
public:
      void myMethod(int a) {
          // comment in body
      }
};
#endif /* A_H_ */


In this sample, the body comment of the method is considered to be a
leading comment to the #endif node by the ASTRewrite for that
TranslationUnit. This kind of makes sense because its the next node
after the comment, but I feel the comment should rather be associated
with the method it stands in. For now it takes some crazy extra effort
to move all the comments of one method definition to a copy, because
in the sample above, the comment is not associated with any node which
belongs to the definition.
_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/cdt-dev
_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/cdt-dev


_______________________________________________
cdt-dev mailing list
cdt-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/cdt-dev

Back to the top