[PATCH] D46000: [AST] Added a helper to extract a user-friendly text of a comment.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Overall looks good. Could you add tests for the new methods?



================
Comment at: lib/AST/CommentLexer.cpp:294
 void Lexer::lexCommentText(Token &T) {
+  if (ParseCommands)
+    lexCommentTextWithCommands(T);
----------------
micro-nit: I'd probably
```
return ParseCommands ? lexWithCommands(T) : lexWithoutCommands(T);
```


================
Comment at: lib/AST/CommentLexer.cpp:471
+      case '\r':
+        TokenPtr = skipNewline(TokenPtr, CommentEnd);
+        formTokenWithChars(T, TokenPtr, tok::newline);
----------------
Can we share code with `lexCommentTextWithCommands` for these two common cases?


================
Comment at: lib/AST/RawCommentList.cpp:353
+  // MaxSkip.
+  auto SkipWs = [](llvm::StringRef S, unsigned MaxSkip) -> llvm::StringRef {
+    unsigned SkipLen = std::min(
----------------
nit: `SkipWhitespaces` for readability?


================
Comment at: lib/AST/RawCommentList.cpp:380
+        SourceMgr.getSpellingColumnNumber(Tok.getLocation(), &LocInvalid);
+    if (LocInvalid)
+      TokColumn = 0;
----------------
Explain when this would be invalid and why `TokColumn = 0` is used?


================
Comment at: lib/AST/RawCommentList.cpp:383
+    // Compute the length of whitespace we're allowed to skip.
+    unsigned MaxSkip;
+    if (IsFirstLine) {
----------------
nit: `unsigned MaxSkip = IsFirstLine ? ... : ...;`


================
Comment at: lib/AST/RawCommentList.cpp:392
+    }
+    llvm::StringRef Trimmed = SkipWs(TokText, MaxSkip);
+    Result += Trimmed;
----------------
I'd probably make `SkipWs` return the number of white spaces skipped and do the drop-front here, so that you could simplify the awkward calculation of `IndentColumn` below.


Repository:
  rC Clang

https://reviews.llvm.org/D46000