[PATCH] D87495: [SyntaxTree][Synthesis] Add support for simple Leafs and test based on tree dump

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.


================
Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:30-31
+syntax::Leaf *createKeyword(Arena &A, tok::TokenKind K);
+syntax::Leaf *createLeaf(syntax::Arena &A, const char *spelling,
+                         tok::TokenKind K);
+
----------------



================
Comment at: clang/include/clang/Tooling/Syntax/BuildTree.h:30-31
+syntax::Leaf *createKeyword(Arena &A, tok::TokenKind K);
+syntax::Leaf *createLeaf(syntax::Arena &A, const char *spelling,
+                         tok::TokenKind K);
+
----------------
gribozavr2 wrote:
> 
I think putting token kind first might be more readable at the call site -- going top-down, first the kind (general), then spelling (specific).


================
Comment at: clang/lib/Tooling/Syntax/Synthesis.cpp:26
+namespace {
+syntax::Leaf *createLeafLowLevel(syntax::Arena &A, const char *spelling) {
+  auto Tokens = A.lexBuffer(llvm::MemoryBuffer::getMemBuffer(spelling)).second;
----------------
+`assert(spelling != nullptr);` ?


================
Comment at: clang/lib/Tooling/Syntax/Synthesis.cpp:51
+  return createLeaf(A, tok::getKeywordSpelling(K), K);
+}
+
----------------
Could we make a combined function that does not require the user to make a distinction between punctuation and keywords?

We should also allow creating tokens that have a user-specified spelling, for example, identifiers and string literals.

So maybe define two functions:

```
// Uses the provided spelling.
syntax::Leaf *createLeaf(syntax::Arena &A, tok::TokenKind K, StringRef Spelling);

// Infers spelling if possible.
syntax::Leaf *createLeaf(syntax::Arena &A, tok::TokenKind K);
```



================
Comment at: clang/unittests/Tooling/Syntax/SynthesisTest.cpp:30-37
+  ASSERT_NE(C, nullptr);
+
+  auto Dump = trim(C->dump(Arena->sourceManager()));
+  auto Expected = trim(R"txt(
+',' Detached synthesized
+  )txt");
+
----------------
Could you extract the part that I highlighted into a function, and deduplicate it across all tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87495