[PATCH] D28111: Misc cleanups and simplifications for NewGVN.Mostly use a bit more idiomatic C++ where we can, so we can combine some things later.

[Davide Italiano via Phabricator via llvm-commits]

davide accepted this revision.
davide added a comment.
This revision is now accepted and ready to land.

LGTM



================
Comment at: include/llvm/Transforms/Scalar/GVNExpression.h:438-453
+class int_op_inserter
+    : public std::iterator<std::output_iterator_tag, void, void, void, void> {
+private:
+  typedef AggregateValueExpression Container;
+  Container *AVE;
+
+public:
----------------
dberlin wrote:
> dberlin wrote:
> > davide wrote:
> > > `op_inserter` and `int_op_inserter` are nearly identical. Is there any chance you can templatize to reduce the duplication?
> > yeah, i'll try to fix this.
> > 
> This is pretty much beyond my C++ knowledge to fix sanely :)
> The problem is that we want both to work.
> IE we don't want op_inserter to call int_op_push_back on aggregatevalues, because aggregatevalues have both ops and int_ops.
> 
> Even sharing the base  functions is non-trivial
Don't worry then =)


https://reviews.llvm.org/D28111