[PATCH] D82226: Add Metadata to Transformer tooling

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 26 07:03:33 PDT 2020


gribozavr2 added a comment.

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.

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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82226





More information about the cfe-commits mailing list