[PATCH] Move semantics for ValueMap

Pete Cooper peter_cooper at apple.com
Mon Apr 27 10:41:27 PDT 2015


> On Apr 27, 2015, at 10:37 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> Can we just drop all the special members from this type? (Rule of Zero)
That would be even better.
> 
> 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)
I did a check for ‘)= default’ and found some hits so figured MSVC was ok.  But now looking again I see those are only for C++03 constructors, destructors, and assignment operators.  I bet you’re right that MSVC will still struggle with this.

I’ll prepare another patch which should be MSVC safe.
> 
> 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.
Thats fine.  You’re right about the values being self-documenting anyway.

Cheers,
Pete
> 
> 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