[PATCH] D82226: Add Metadata to Transformer tooling

This revision was automatically updated to reflect the committed changes. Closed by commit rG9945bd591163: Add Metadata to Transformer tooling (authored by asoffer, committed by ymandel). Changed prior to commit: https://reviews.llvm.org/D82226?vs=274183=274484#toc Repository: rG LLVM

[PATCH] D82226: Add Metadata to Transformer tooling

asoffer updated this revision to Diff 274183. asoffer added a comment. Signed-comparison issue in test fixed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82226/new/ https://reviews.llvm.org/D82226 Files:

[PATCH] D82226: Add Metadata to Transformer tooling

asoffer added a comment. I'm not against a map, but I don't see the need for it right now. We don't have multiple stages that write to the same AtomicChange in any tool I'm aware of. Given that these are immutable after construction, I think just the Any is simpler. So long as we're willing to

[PATCH] D82226: Add Metadata to Transformer tooling

ymandel added a comment. In D82226#2116931 , @gribozavr2 wrote: > In D82226#2115406 , @asoffer wrote: > > > I think the tradeoff here is > > Dynamic typing -- faster compile times, type safety checked at run-time

[PATCH] D82226: Add Metadata to Transformer tooling

gribozavr2 added a comment. In 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

[PATCH] D82226: Add Metadata to Transformer tooling

asoffer updated this revision to Diff 273522. asoffer added a comment. Last diff was a mistake. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82226/new/ https://reviews.llvm.org/D82226 Files:

[PATCH] D82226: Add Metadata to Transformer tooling

asoffer updated this revision to Diff 273520. asoffer marked an inline comment as done. asoffer added a comment. Updating other test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82226/new/ https://reviews.llvm.org/D82226 Files:

[PATCH] D82226: Add Metadata to Transformer tooling

asoffer updated this revision to Diff 273516. asoffer marked an inline comment as done and an inline comment as not done. asoffer added a comment. Commenting fields and simplifying test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82226/new/

[PATCH] D82226: Add Metadata to Transformer tooling

asoffer marked 2 inline comments as done. asoffer added a comment. In D82226#2114111 , @ymandel wrote: > Looks good! Only real question is one of design -- should we consider the > (deeper) change of templating the various types rather than using dynamic

[PATCH] D82226: Add Metadata to Transformer tooling

ymandel added a comment. Looks good! Only real question is one of design -- should we consider the (deeper) change of templating the various types rather than using dynamic typing? For that matter, the decision doesn't even have to be the same for both AtomicChange and the Transformer types.