[PATCH] D137909: [Support] Allow complex names for annotation points and ranges via $()

Tom Praschan via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 14 05:16:05 PST 2022


tom-anders added a comment.

In D137909#3924587 <https://reviews.llvm.org/D137909#3924587>, @sammccall wrote:

> This seems pretty nice (I do find `foo$long_word^bar` a bit hard to read). I think I'm in favor, my concerns would be that for a small benefit we're:
>
> - adding some extra complexity people have to understand
> - providing more ways to write something, so more bikeshedding opportunities
> - encouraging people to embed structured information in the annotations, in a way that won't be very readable *and* isn't well-supported by the API
>
> Can you give examples of where you want to use this?
> (I think we should *probably* accept this regardless, but tests want to iterate over payload-name pairs or things like that we should think a couple of steps ahead)

Yeah sorry, should've included this example right away:

This clangd test <https://searchfox.org/llvm/source/clang-tools-extra/clangd/unittests/XRefsTests.cpp#1933> (and some others in this file) use the names of annotation for encoding the expected `Attribute` field of the `Reference`struct.
For testing https://github.com/clangd/clangd/issues/177 (https://reviews.llvm.org/D137894) I wanted to extend those tests instead of writing new one, so the idea was to encode both the expected `Attribute` and the expected `Context` inside the annotation name (e.g. via comma-separated list).
Or maybe there's a better way for extending the test without abusing annotations? I couldn't really find a solution that wouldn't involve a major rewrite of the test logic here.



================
Comment at: llvm/include/llvm/Testing/Support/Annotations.h:27
+///       $definition^class Foo{};           // points can be named: "definition"
+///       $(very,complex::name)^class Foo{}; // names inside $() can contain any characters
+///       $fail[[static_assert(false, "")]]  // ranges can be named too: "fail"
----------------
sammccall wrote:
> How do you feel about `${...}` rather than `$(...)`?
> 
> `$foo` looks like shell to me, and so `${foo}` seems like a quoting version while `$(foo)` reminds me more of some kind of embedded expression.
> 
> Small difference but maybe others would have similar associations.
Ah yes I agree, this is definitely more intuitive, I'll change it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137909



More information about the llvm-commits mailing list