[PATCH] This change-list changes clang to attach the ordinary comments if an option is specified

Dmitri Gribenko gribozavr at gmail.com
Wed Apr 3 06:26:37 PDT 2013


  I think a name like "parse all comments" would better reflect the intent of this feature.


================
Comment at: include/clang/AST/RawCommentList.h:145
@@ +144,3 @@
+  /// When true, ordinary comments starting with "//" will be considered as
+  // doc-comments.
+  bool TreatOrdinaryCommentAsDocComment;
----------------
Three slashes, please.

================
Comment at: include/clang/AST/RawCommentList.h:144
@@ -141,1 +143,3 @@
   mutable unsigned EndLine;        ///< Cached line number
+  /// When true, ordinary comments starting with "//" will be considered as
+  // doc-comments.
----------------
Only "//"?  It would be better to rename it to "parse all comments".  Is this OK with you?


================
Comment at: unittests/Frontend/FrontendActionTest.cpp:103-104
@@ +102,4 @@
+   private:
+    std::vector<std::string>* comments_;
+    ASTContext* context_;
+  };
----------------
Star goes to the right.  Member names should start with a capital letter and should not have an underscore.

================
Comment at: unittests/Frontend/FrontendActionTest.cpp:108-109
@@ +107,4 @@
+
+TEST(ASTFrontendAction, TreatOrdinaryCommentAsDoccomment) {
+  CompilerInvocation *invocation = new CompilerInvocation;
+  invocation->getPreprocessorOpts().addRemappedFile(
----------------
Why is this test so complicated, going through constructing half of Clang?

You can add a very simple test, based on c-index-test, see test/Index/annotate-comments.cpp.

================
Comment at: include/clang/AST/RawCommentList.h:92
@@ -89,3 +91,3 @@
   bool isDocumentation() const LLVM_READONLY {
-    return !isInvalid() && !isOrdinary();
+    return !isInvalid() && (!isOrdinary() || TreatOrdinaryCommentAsDocComment);
   }
----------------
It would be better to change isOrdinary instead:

  return (Kind == RCK_OrdinaryBCPL) || (Kind == RCK_OrdinaryC) || TreatOrdinaryCommentAsDocComment;


================
Comment at: include/clang/AST/RawCommentList.h:146
@@ -142,1 +145,3 @@
+  // doc-comments.
+  bool TreatOrdinaryCommentAsDocComment;
 
----------------
Please pack this at the end of the bitfield above (after IsTrailingComment, IsAlmostTrailingComment).


================
Comment at: include/clang/AST/RawCommentList.h:13-15
@@ -12,4 +12,5 @@
 
 #include "clang/Basic/SourceManager.h"
+#include "clang/Basic/CommentOptions.h"
 #include "llvm/ADT/ArrayRef.h"
 
----------------
Please sort includes.

================
Comment at: lib/AST/RawCommentList.cpp:258
@@ -255,3 +257,3 @@
                               C2.getSourceRange().getEnd());
-      *Comments.back() = RawComment(SourceMgr, MergedRange, true);
+      *Comments.back() = RawComment(SourceMgr, MergedRange, true, true);
       Merged = true;
----------------
I don't think the last 'true' is necessary here.  Why do you think it is?


================
Comment at: unittests/Frontend/FrontendActionTest.cpp:112-113
@@ +111,4 @@
+      "test.cc", MemoryBuffer::getMemBuffer(
+        "// ordinary comment\n"
+        "// for main method\n"
+        "int main() { float x; }"
----------------
Please add tests for /* ... */ comments.


http://llvm-reviews.chandlerc.com/D614



More information about the cfe-commits mailing list