[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/lib/Tooling/Syntax/Synthesis.cpp:51
+  return createLeaf(A, tok::getKeywordSpelling(K), K);
+}
+
----------------
eduucaldas wrote:
> gribozavr2 wrote:
> > 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);
> > ```
> > 
> First, thanks for the great comments!
> 
> > Could we make a combined function that does not require the user to make a distinction between punctuation and keywords?
> We could! 
> Bui I wouldn't call it `createLeaf`, as it would work strictly for `Keyword`s or `Punctuator`s. 
> As an alternative we could unify the `createPunctuator`and `createKeyword` into `createPunctuatorOrKeyword` but I would argue that the previous two names are more readable. Effectively, we would be unifying those 2 functions because they have the same signature.
> 
> > We should also allow creating tokens that have a user-specified spelling, for example, identifiers and string literals.
> That exists already! It is `createLeaf` as is!
> but I would argue that the previous two names are more readable. Effectively, we would be unifying those 2 functions because they have the same signature.

I don't think the user cares very much about whether their token is a keyword or a punctuation. Furthermore, most callsites will have the token kind as an argument, so saying that it is a keyword or punctuation is redundant:

```
createKeyword(Arena, tok::kw_if); // Yeah, 'if' is a keyword.
```

The semantics of the combined function I'm proposing is "create a token that has one fixed spelling". The other function is "create a token where the spelling is user-provided".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D87495