[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL363803: [clangd] Collect tokens of main files when building 
the AST (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D62956?vs=205387=205579#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D62956

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/ClangdUnit.cpp
  clang-tools-extra/trunk/clangd/ClangdUnit.h
  clang-tools-extra/trunk/clangd/tool/CMakeLists.txt
  clang-tools-extra/trunk/clangd/unittests/CMakeLists.txt
  clang-tools-extra/trunk/clangd/unittests/ClangdUnitTests.cpp

Index: clang-tools-extra/trunk/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.cpp
+++ clang-tools-extra/trunk/clangd/ClangdUnit.cpp
@@ -36,6 +36,7 @@
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -418,6 +419,9 @@
   collectIWYUHeaderMaps();
   Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());
 
+  // Collect tokens of the main file.
+  syntax::TokenCollector Tokens(Clang->getPreprocessor());
+
   if (!Action->Execute())
 log("Execute() failed when building AST for {0}", MainInput.getFile());
 
@@ -446,8 +450,9 @@
   if (Preamble)
 Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-   std::move(ParsedDecls), std::move(Diags),
-   std::move(Includes), std::move(CanonIncludes));
+   std::move(Tokens).consume(), std::move(ParsedDecls),
+   std::move(Diags), std::move(Includes),
+   std::move(CanonIncludes));
 }
 
 ParsedAST::ParsedAST(ParsedAST &) = default;
@@ -540,11 +545,13 @@
 ParsedAST::ParsedAST(std::shared_ptr Preamble,
  std::unique_ptr Clang,
  std::unique_ptr Action,
+ syntax::TokenBuffer Tokens,
  std::vector LocalTopLevelDecls,
  std::vector Diags, IncludeStructure Includes,
  CanonicalIncludes CanonIncludes)
 : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
-  Action(std::move(Action)), Diags(std::move(Diags)),
+  Action(std::move(Action)), Tokens(std::move(Tokens)),
+  Diags(std::move(Diags)),
   LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
   Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {
   assert(this->Clang);
Index: clang-tools-extra/trunk/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/tool/CMakeLists.txt
@@ -26,5 +26,6 @@
   clangSema
   clangTooling
   clangToolingCore
+  clangToolingSyntax
   ${CLANGD_XPC_LIBS}
   )
Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -128,6 +128,7 @@
   clangToolingCore
   clangToolingInclusions
   clangToolingRefactoring
+  clangToolingSyntax
   ${LLVM_PTHREAD_LIB}
   ${CLANGD_ATOMIC_LIB}
   )
Index: clang-tools-extra/trunk/clangd/ClangdUnit.h
===
--- clang-tools-extra/trunk/clangd/ClangdUnit.h
+++ clang-tools-extra/trunk/clangd/ClangdUnit.h
@@ -24,6 +24,7 @@
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include 
 #include 
 #include 
@@ -115,10 +116,14 @@
   const IncludeStructure () const;
   const CanonicalIncludes () const;
 
+  /// Tokens recorded while parsing the main file.
+  /// (!) does not have tokens from the preamble.
+  const syntax::TokenBuffer () const { return Tokens; }
+
 private:
   ParsedAST(std::shared_ptr Preamble,
 std::unique_ptr Clang,
-std::unique_ptr Action,
+std::unique_ptr Action, syntax::TokenBuffer Tokens,
 std::vector LocalTopLevelDecls, std::vector Diags,
 IncludeStructure Includes, CanonicalIncludes CanonIncludes);
 
@@ -132,6 +137,11 @@
   // FrontendAction.EndSourceFile).
   std::unique_ptr Clang;
   std::unique_ptr Action;
+  /// Tokens recorded after the preamble finished.
+  ///   - Includes all spelled tokens for the main file.
+  ///   - Includes 

[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D62956#1547826 , @sammccall wrote:

> Can we add a test using TestTU that does a very basic verification of 
> expanded/spelled tokens (first after preamble, last token in file)?


Done. Will land this tomorrow, want to run it over all of LLVM first to give 
the token-building code some coverage.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62956



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 205387.
ilya-biryukov marked an inline comment as done.
ilya-biryukov added a comment.

- Rename tokens() to getTokens() for consistency with the rest of ParsedAST.
- Update comments.
- Add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62956

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/ClangdUnit.h
  clang-tools-extra/clangd/tool/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdUnitTests.cpp
@@ -10,6 +10,8 @@
 #include "ClangdUnit.h"
 #include "SourceCode.h"
 #include "TestTU.h"
+#include "clang/Basic/TokenKinds.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
@@ -81,6 +83,37 @@
   EXPECT_THAT(AST.getLocalTopLevelDecls(), ElementsAre(DeclNamed("main")));
 }
 
+TEST(ClangdUnitTest, TokensAfterPreamble) {
+  TestTU TU;
+  TU.AdditionalFiles["foo.h"] = R"(
+int foo();
+  )";
+  TU.Code = R"cpp(
+  #include "foo.h"
+  first_token;
+  void test() {
+  }
+  last_token
+)cpp";
+  auto AST = TU.build();
+  const syntax::TokenBuffer  = AST.getTokens();
+  const auto  = AST.getSourceManager();
+
+  ASSERT_GT(T.expandedTokens().size(), 2u);
+  // First token after preamble is 'void'.
+  EXPECT_EQ(T.expandedTokens().front().text(SM), "first_token");
+  // Last token is always 'eof'.
+  EXPECT_EQ(T.expandedTokens().back().kind(), tok::eof);
+  // Check the token right before 'eof'.
+  EXPECT_EQ(T.expandedTokens().drop_back().back().text(SM), "last_token");
+
+  // The spelled tokens for the main file should have everything.
+  auto Spelled = T.spelledTokens(SM.getMainFileID());
+  ASSERT_FALSE(Spelled.empty());
+  EXPECT_EQ(Spelled.front().kind(), tok::hash);
+  EXPECT_EQ(Spelled.back().text(SM), "last_token");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -87,6 +87,7 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingSyntax
   LLVMSupport
   LLVMTestingSupport
   )
Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -26,5 +26,6 @@
   clangSema
   clangTooling
   clangToolingCore
+  clangToolingSyntax
   ${CLANGD_XPC_LIBS}
   )
Index: clang-tools-extra/clangd/ClangdUnit.h
===
--- clang-tools-extra/clangd/ClangdUnit.h
+++ clang-tools-extra/clangd/ClangdUnit.h
@@ -24,6 +24,7 @@
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include 
 #include 
 #include 
@@ -115,10 +116,14 @@
   const IncludeStructure () const;
   const CanonicalIncludes () const;
 
+  /// Tokens recorded while parsing the main file.
+  /// (!) does not have tokens from the preamble.
+  const syntax::TokenBuffer () const { return Tokens; }
+
 private:
   ParsedAST(std::shared_ptr Preamble,
 std::unique_ptr Clang,
-std::unique_ptr Action,
+std::unique_ptr Action, syntax::TokenBuffer Tokens,
 std::vector LocalTopLevelDecls, std::vector Diags,
 IncludeStructure Includes, CanonicalIncludes CanonIncludes);
 
@@ -132,6 +137,11 @@
   // FrontendAction.EndSourceFile).
   std::unique_ptr Clang;
   std::unique_ptr Action;
+  /// Tokens recorded after the preamble finished.
+  ///   - Includes all spelled tokens for the main file.
+  ///   - Includes expanded tokens produced **after** preabmle.
+  ///   - Does not have spelled or expanded tokens for files from preamble.
+  syntax::TokenBuffer Tokens;
 
   // Data, stored after parsing.
   std::vector Diags;
Index: clang-tools-extra/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -36,6 +36,7 @@
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include 

[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Can we add a test using TestTU that does a very basic verification of 
expanded/spelled tokens (first after preamble, last token in file)?




Comment at: clang-tools-extra/clangd/ClangdUnit.h:140
   std::unique_ptr Action;
+  /// Expanded tokens for the main file. Does not contain tokens for the file
+  /// preamble.

Not sure "expanded" is accurate here - it has both.
(And it'd be worth spelling out if expanded tokens or spelled tokens or both 
are missing for the preamble)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62956



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62956: [clangd] Collect tokens of main files when building the AST

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, mgorny.
Herald added a project: clang.
ilya-biryukov added a child revision: D61681: [clangd] A code tweak to expand a 
macro.

The first use of this is a code tweak to expand macro calls.
Will later be used to build syntax trees.

The memory overhead is small as we only store token of the main file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D62956

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/ClangdUnit.cpp
  clang-tools-extra/clangd/ClangdUnit.h
  clang-tools-extra/clangd/tool/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CMakeLists.txt

Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -86,6 +86,7 @@
   clangTooling
   clangToolingCore
   clangToolingInclusions
+  clangToolingSyntax
   LLVMSupport
   LLVMTestingSupport
   )
Index: clang-tools-extra/clangd/tool/CMakeLists.txt
===
--- clang-tools-extra/clangd/tool/CMakeLists.txt
+++ clang-tools-extra/clangd/tool/CMakeLists.txt
@@ -26,5 +26,6 @@
   clangSema
   clangTooling
   clangToolingCore
+  clangToolingSyntax
   ${CLANGD_XPC_LIBS}
   )
Index: clang-tools-extra/clangd/ClangdUnit.h
===
--- clang-tools-extra/clangd/ClangdUnit.h
+++ clang-tools-extra/clangd/ClangdUnit.h
@@ -24,6 +24,7 @@
 #include "clang/Serialization/ASTBitCodes.h"
 #include "clang/Tooling/CompilationDatabase.h"
 #include "clang/Tooling/Core/Replacement.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include 
 #include 
 #include 
@@ -115,10 +116,14 @@
   const IncludeStructure () const;
   const CanonicalIncludes () const;
 
+  /// Tokens recorded while parsing the main file.
+  /// (!) does not have tokens from the preamble.
+  const syntax::TokenBuffer () const { return Tokens; }
+
 private:
   ParsedAST(std::shared_ptr Preamble,
 std::unique_ptr Clang,
-std::unique_ptr Action,
+std::unique_ptr Action, syntax::TokenBuffer Tokens,
 std::vector LocalTopLevelDecls, std::vector Diags,
 IncludeStructure Includes, CanonicalIncludes CanonIncludes);
 
@@ -132,6 +137,9 @@
   // FrontendAction.EndSourceFile).
   std::unique_ptr Clang;
   std::unique_ptr Action;
+  /// Expanded tokens for the main file. Does not contain tokens for the file
+  /// preamble.
+  syntax::TokenBuffer Tokens;
 
   // Data, stored after parsing.
   std::vector Diags;
Index: clang-tools-extra/clangd/ClangdUnit.cpp
===
--- clang-tools-extra/clangd/ClangdUnit.cpp
+++ clang-tools-extra/clangd/ClangdUnit.cpp
@@ -36,6 +36,7 @@
 #include "clang/Serialization/ASTWriter.h"
 #include "clang/Serialization/PCHContainerOperations.h"
 #include "clang/Tooling/CompilationDatabase.h"
+#include "clang/Tooling/Syntax/Tokens.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
@@ -418,6 +419,9 @@
   collectIWYUHeaderMaps();
   Clang->getPreprocessor().addCommentHandler(IWYUHandler.get());
 
+  // Collect tokens of the main file.
+  syntax::TokenCollector Tokens(Clang->getPreprocessor());
+
   if (!Action->Execute())
 log("Execute() failed when building AST for {0}", MainInput.getFile());
 
@@ -446,8 +450,9 @@
   if (Preamble)
 Diags.insert(Diags.begin(), Preamble->Diags.begin(), Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),
-   std::move(ParsedDecls), std::move(Diags),
-   std::move(Includes), std::move(CanonIncludes));
+   std::move(Tokens).consume(), std::move(ParsedDecls),
+   std::move(Diags), std::move(Includes),
+   std::move(CanonIncludes));
 }
 
 ParsedAST::ParsedAST(ParsedAST &) = default;
@@ -540,11 +545,13 @@
 ParsedAST::ParsedAST(std::shared_ptr Preamble,
  std::unique_ptr Clang,
  std::unique_ptr Action,
+ syntax::TokenBuffer Tokens,
  std::vector LocalTopLevelDecls,
  std::vector Diags, IncludeStructure Includes,
  CanonicalIncludes CanonIncludes)
 : Preamble(std::move(Preamble)), Clang(std::move(Clang)),
-  Action(std::move(Action)), Diags(std::move(Diags)),
+  Action(std::move(Action)), Tokens(std::move(Tokens)),
+  Diags(std::move(Diags)),
   LocalTopLevelDecls(std::move(LocalTopLevelDecls)),
   Includes(std::move(Includes)), CanonIncludes(std::move(CanonIncludes)) {
   assert(this->Clang);
Index: clang-tools-extra/clangd/CMakeLists.txt