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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 21 07:30:50 PDT 2019


ilya-biryukov added a comment.

In D59887#1508991 <https://reviews.llvm.org/D59887#1508991>, @thakis wrote:

> Out of interest: The RecursiveASTVisitorTests are part of the ToolingTests binary while this adds a new binary TokensTest. Can you say why?
>
> (No change needed, just curious.)


The syntax library is mostly independent from the rest of tooling, so I'd rather keep everything related to it separate including the tests.
I don't think we'll get anything in terms of code reuse from merging them into the same test binary either.

In D59887#1508993 <https://reviews.llvm.org/D59887#1508993>, @thakis wrote:

> Another comment: The new binary is called TokensTest but is in a directory "Syntax". For consistency with all other unit test binaries, please either rename the binary to SyntaxTests, or rename the directory to "Tokens". (From the patch description, the former seems more appropriate.) Note the missing trailing "s" in the binary name too.


Agree with renaming the binary. In fact, the not-yet-landed revisions also use `SyntaxTests`, and I should've named it this way from the start. I'll land a patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59887





More information about the cfe-commits mailing list