[PATCH] Move semantics for ValueMap

David Blaikie dblaikie at gmail.com
Mon Apr 27 10:37:17 PDT 2015


Can we just drop all the special members from this type? (Rule of Zero)

Since it has a unique_ptr member it'll be move-only anyway.

(oh, except for MSVC, which doesn't synthesize move members - I don't
think it even works with explicit "= default" (check teh codebase to
see if there are any of those already - if there are, perhaps I'm
wrong?) so you might have to write them out explicitly)

Also: I'd drop the printfs from the test case - if they values are
interesting, just have the appropriate unit test check/EXPECT/whatever
for them. Then they're self-documenting without having to print them.
I can see, statically, in the code what the values are that they will
print.

On Mon, Apr 27, 2015 at 9:59 AM, Pete Cooper <peter_cooper at apple.com> wrote:
> Hi Rafael, David
>
> In
> http://lists.cs.uiuc.edu/pipermail/llvm-commits/Week-of-Mon-20150420/273199.html,
> Rafael suggested returning a ValueMap from CloneFunction.
>
> Turns out that ValueMap has a deleted copy constructor, and no move
> constructor all.  This patch adds the move constructor (and assignment
> operator). The default implementations of both of these seem to be fine.
>
> To test this, i managed to get the Constructible class from the SmallVector
> unit test to wrap a Value*, then use that to count all the times the
> ValueMap does any of the operators.  I then went through the ValueMap and
> DenseMap code to verify each of the values i’m checking against.  I left
> some printf’s in an #if 0 for future use in case we want to easily check the
> values coming out of this code.
>
> Cheers,
> Pete
>
>
>




More information about the llvm-commits mailing list