[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.


================
Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:8
+//===----------------------------------------------------------------------===//
+#include "clang/Tooling/Syntax/TokenBuffer.h"
+#include "clang/Basic/Diagnostic.h"
----------------
Please separate with empty line.


================
Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:28
+}
+llvm::StringRef syntax::Token::text(const SourceManager &SM) const {
+  bool Invalid = false;
----------------
Please separate with empty line.


================
Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:323
+                                              PP.getLangOpts(), Tokens);
+  auto *CB = CBOwner.get();
+
----------------
Return type is not obvious. Actually variable is used only one, so it's not necessary.


================
Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:133
+                                      FileName};
+    auto CI = createInvocationFromCommandLine(Args, Diags, FS);
+    assert(CI);
----------------
Return type is not obvious, so auto should not be used.


================
Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:157
+    std::string NullTerminated = Text.str();
+    auto FID = SourceMgr->createFileID(llvm::MemoryBuffer::getMemBufferCopy(
+        StringRef(NullTerminated.data(), NullTerminated.size() + 1)));
----------------
Return type is not obvious, so auto should not be used.


================
Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:165
+  void checkTokens(llvm::StringRef ExpectedText) {
+    auto TokenizedCode = tokenize(ExpectedText);
+    std::vector<syntax::Token> ExpectedTokens = TokenizedCode.tokens();
----------------
Return type is not obvious, so auto should not be used.


================
Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:186
+  void checkExpansions(llvm::ArrayRef<ExpectedExpansion> Expected) {
+    auto Actual = Buffer.expansions();
+    ASSERT_EQ(Actual.size(), Expected.size());
----------------
Return type is not obvious, so auto should not be used.


================
Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:190
+    for (unsigned I = 0; I < Actual.size(); ++I) {
+      auto &A = Actual[I];
+      auto &E = Expected[I];
----------------
Return type is not obvious, so auto should not be used.


================
Comment at: clang/unittests/Tooling/Syntax/TokenBufferTest.cpp:191
+      auto &A = Actual[I];
+      auto &E = Expected[I];
+
----------------
Return type is not obvious, so auto should not be used.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59887/new/

https://reviews.llvm.org/D59887