[PATCH] D134072: [Support] Provide access to the full mapping in llvm::Annotations

[Sam McCall via llvm-commits]

Yes, this makes sense. I think exposing just the names is a bit "clever".

If we changed the internal structure to be an array of struct {
  StringRef Name;
  StringRef Payload;
  size_t Start;
  size_t End; // -1 for a point
}
Then how do you feel if we expose that array and drop the current maps?

On Thu, Nov 17, 2022 at 2:16 PM Eric Li via Phabricator <
reviews at reviews.llvm.org> wrote:

> li.zhe.hua added a comment.
>
> So the two functions I added were because previously using `Annotations`
> required knowledge of the annotation names a priori. If you are
> constructing one and using it directly yourself in a test, great. If you
> handed the `Annotations` object to a helper function, you'd also need to
> hand the two sets of names with it (one for points and one for ranges),
> otherwise the helper function doesn't know what annotations exist. In our
> case, the helper function is a googletest matcher verifying matches on AST
> nodes are bound to IDs matching the annotations listed. However, if no
> nodes are matched, we can't verify that all annotated nodes were matched
> without a lot of extra (possibly error-prone) boilerplate.
>
> Ultimately, we only really need the names. Returning the full map was
> mostly for simplicity (if you want all the names, you probably will query
> all the values eventually), but really the key (... heh) piece of data we
> need is the names from the maps.
>
>
> Repository:
>   rG LLVM Github Monorepo
>
> CHANGES SINCE LAST ACTION
>   https://reviews.llvm.org/D134072/new/
>
> https://reviews.llvm.org/D134072
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221117/ab726c3c/attachment.html>