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



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

Reply via email to