[PATCH] D82226: Add Metadata to Transformer tooling

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D82226#2116931 <https://reviews.llvm.org/D82226#2116931>, @gribozavr2 wrote:

> In D82226#2115406 <https://reviews.llvm.org/D82226#2115406>, @asoffer wrote:
>
> > I think the tradeoff here is 
> >  Dynamic typing -- faster compile times, type safety checked at run-time (in tests), lower maintenance cost
> >  Templates -- Faster runtime, type safety checked at compile-time, better user expereience
>
>
> For me, the more important part of the tradeoff is whether we will have one type or multiple. If we use Any, then all `AtomicChange` regardless of what produced them, will have the same type. If we use templates, then `AtomicChange<T>` is a different type from `AtomicChange<U>`. So based on that I think using Any is a better choice, since infrastructure code would want to handle with `AtomicChange` objects and not have to be implemented as a template over arbitrary metadata type.


I'm convinced (both yours and Andy's arguments). Andy, please consider whether any of this belongs in comments. (I don't have much opinion).

> Regarding the metadata itself, WDYT about using a map from string to any instead of just one any? This way multiple layers of the system would be able to attach metadata without interfering.

Sounds good, but I'm fine either way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82226