[PATCH] D64573: [Syntax] Allow to mutate syntax trees

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as not done.
ilya-biryukov added inline comments.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:57
+  return nullptr;
+}
+
----------------
gribozavr2 wrote:
> Seems like these first/last helpers should be methods on `syntax::Node`.
Good point. Done.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:66
+  syntax::Leaf *Last = lastLeaf(T);
+  return llvm::makeArrayRef(First->token(), Last->token() + 1);
+}
----------------
gribozavr2 wrote:
> The first and the last tokens are not necessarily from the same buffer...
They are for nodes with `isOriginal() == true`. I've added an assertion.
Exactly the reason why this method is not a good fit for public API, but ok to have in tests.


================
Comment at: clang/unittests/Tooling/Syntax/TreeTest.cpp:528
+void test() {
+  HALF_IF HALF_IF_2 else {}
+})cpp",
----------------
gribozavr2 wrote:
> Could you also do something like:
> 
> ```
> #define OPEN {
> #define CLOSE }
> 
> void test1() {
>   OPEN
>     1;
>   CLOSE
> }
> void test1() {
>   OPEN
>     1;
>   }
> }
> ```
Funnily enough, this causes an assertion failure, because binary-searching with `isBeforeInTranslationUnit` finds `{` expanded from `OPEN` instead of `1` when building a syntax tree.

I'll make use of a hash table for searching tokens by location and add the test in the follow-up patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64573