Comment 1 Leo Büttiker 2006-06-23 05:00:19 EDT

Created attachment 45153 [details]
Patch for parsing comments

The attached patch contains a solution for parsing comments and attaching them (with the decorator pattern) to the most relevant node in the AST.

Comment 2 Leo Büttiker 2006-06-23 05:03:37 EDT

Created attachment 45154 [details]
Visualtest for comment parser patch

This patch show the comments in the visual DOMAST testclass.

Comment 3 Emanuel Graf 2006-07-10 12:31:59 EDT

There's a bug  in the patch. I'll fix it and then upload a new version.

Comment 4 sadik akbulut 2006-11-23 03:43:59 EST

previous versions of CDT was able to colorize comments starting with predefined keywords such as "TODO". But 4.0 nightly builds are not able to do this anymore. By the way, colorizing the entire line with TODO (intellij idea style) and showing TODO line in the gutter with a different color (just like warnings and errors) is requested.

Comment 5 Sergey Prigogin 2007-04-13 22:35:03 EDT

I'd like to use this patch as part of supporting task tags (bug 79459). Unfortunately, the patch got out of date and cannot be applied easily. Could please somebody familiar with the code update it and ideally apply to HEAD so that it doesn't get stale again.

I have a much more limited change that does comment parsing and would satisfy task tags needs, but it does not put comments in AST.

Comment 6 Emanuel Graf 2007-04-20 04:32:55 EDT

Created attachment 64394 [details]
new Comment-Handling Patch

After Markus's comment to my C++-Refactoring Report #181493 we changed our commenthandling. Adding the comments to the AST in the parser is hard to do and adds complexity to an already complex system. The new proposal just adds all the comments to a new array in the translation unit. This should meet all the needs of bug # 79459 'Tasks not working' and for the refactoring we can integrate the comments into the AST in a additional step before the refactoring starts.

Comment 7 Emanuel Graf 2007-04-20 04:38:31 EDT

Created attachment 64397 [details]
Commenthandling Unit Tests

Comment 8 Markus Schorn 2007-04-20 08:52:21 EDT

Patch looks good, I'll add some code such that you can construct the AST with comments via the recommended methods AbstractLanguage.getASTTranslationUnit(...) and TranslationUnit.getAST(...).

Comment 9 Mike Kucera 2007-04-20 11:04:23 EDT

I have taken a look at the patch and have two suggestions:

1) The parseComment() method in AbstractGNUSourceParser should be renamed to createComment(). It doesn't actually parse anything, it creates an AST node. Furthermore the convention of declaring all the create methods abstract should be followed.

2) IASTTranslationUnit.setComments() should be called in translationUnit() instead of getTranslationUnit(). This is for consistency, because translationUnit() is where the other set methods are called.

Comment 10 Emanuel Graf 2007-04-20 11:34:08 EDT

(In reply to comment #9)
> 1) The parseComment() method in AbstractGNUSourceParser should be renamed to
> createComment(). It doesn't actually parse anything, it creates an AST node.
> Furthermore the convention of declaring all the create methods abstract should
> be followed.
> 
> 2) IASTTranslationUnit.setComments() should be called in translationUnit()
> instead of getTranslationUnit(). This is for consistency, because
> translationUnit() is where the other set methods are called.

Sounds reasonable. I'll change that. I'll wait with a new patch until the discussion on the maillist has come to a result. Because maybe there will be other changes.

Comment 11 Emanuel Graf 2007-04-23 01:52:12 EDT

Created attachment 64565 [details]
patch including the changes suggested by mike kucera

Comment 12 Markus Schorn 2007-04-23 04:53:21 EDT

Thanks Emanuel for the patches, and Mike thanks for reviewing!

I have applied the patches except the changes for constructing the parser (AbstractGNUSourceCodeParser). I did not find a reason why the parser should know about the fact whether comments are provided by the scanner or not. If I am wrong about it, please reopen the bug.

Furthermore I have added the flag ITranslationUnit.AST_CREATE_COMMENT_NODES and
AbstractLanguage.OPTION_ADD_COMMENTS for conveniently creating AST with comments:
   IIndex index= ...
   ITranslationUnit tu= ...
   tu.getAST(index, ITranslationUnit.AST_CREATE_COMMENT_NODES);

Comment 13 Emanuel Graf 2007-04-23 05:09:53 EDT

Thanks Markus for all your support and work.

Comment 14 Mike Kucera 2007-04-24 11:34:57 EDT

I have a concern regarding comment nodes and the task list.

Since comment nodes are generated by the parser, after the preprocessor runs, then comment nodes will not be created for conditional compilation branches that are not followed.

Consider the following code:

#define FOO 1
#ifdef FOO
    // TODO do something
#else
    // TODO do something else
#endif

Only the first TODO will appear in array of comment nodes in the resulting AST. If the task list is built from this array, then it will be missing TODOs in the source.

Comment 15 Markus Schorn 2007-04-25 03:14:18 EDT

(In reply to comment #14)
I have created a test-case and bug 183930 for that.