[PATCH] D11069: Allow any comment to be a trailing comment when -fparse-all-comments is on.

Richard Smith richard at metafoo.co.uk
Tue Jul 14 14:54:23 PDT 2015


rsmith added inline comments.

================
Comment at: lib/AST/RawCommentList.cpp:77-88
@@ +76,14 @@
+  for (unsigned I = P; I != 0; --I) {
+    switch (Buffer[I - 1]) {
+    default:
+      return false;
+    case ' ':
+    case '\t':
+    case '\f':
+    case '\v':
+      break;
+    case '\r':
+    case '\n':
+      return true;
+    }
+  }
----------------
Use `clang::isHorizontalWhitespace` / `clang::isVerticalWhitespace` instead (from Basic/CharInfo.h).

================
Comment at: lib/AST/RawCommentList.cpp:300-302
@@ -243,3 +299,5 @@
   // Merge comments if they are on same or consecutive lines.
-  if (C1.isTrailingComment() == C2.isTrailingComment() &&
+  if ((C1.isTrailingComment() == C2.isTrailingComment() ||
+       (C1.isTrailingComment() && !C2.isTrailingComment() &&
+        isOrdinaryKind(C2.getKind()))) &&
       onlyWhitespaceBetween(SourceMgr, C1.getLocEnd(), C2.getLocStart(),
----------------
I find this a bit worrying...

================
Comment at: test/Index/parse-all-comments.c:42-44
@@ +41,5 @@
+
+int trdoxyB;  // Not a Doxygen trailing comment.  PART_ONE
+              // It's a multiline one too.  trdoxyB NOT_DOXYGEN
+int trdoxyC;
+
----------------
... because this case probably shouldn't get merged if the second comment is not indented. It seems that the indentation is the only way we can tell that case apart from:

    int trdoxyB;  // Not a Doxygen trailing comment.
    // This documents trdoxyC, not trdoxyB.
    ​int trdoxyC;

Perhaps we should only allow ordinary comments to merge with trailing comments if they start in the same column. I'm not too happy about making the semantics depend on the indentation, but you're already proposing that they depend on whether there are non-whitespace characters earlier on the same line, so perhaps that is OK.


http://reviews.llvm.org/D11069







More information about the cfe-commits mailing list