This is an archive of the discontinued LLVM Phabricator instance.

Add some extra checks to the MoveOnly test class to ensure it is not constructed or assigned from in a moved-from state.
ClosedPublic

Authored by dblaikie on Jun 10 2014, 1:22 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

dblaikie updated this revision to Diff 10300.Jun 10 2014, 1:22 PM
dblaikie retitled this revision from to Add some extra checks to the MoveOnly test class to ensure it is not constructed or assigned from in a moved-from state..
dblaikie added a reviewer: mclow.lists.
dblaikie updated this object.
dblaikie added a subscriber: Unknown Object (MLST).
mclow.lists edited edge metadata.Jul 24 2014, 7:57 AM

What's the general motivation for these changes?
What errors do you expect these asserts to find?

What's the general motivation for these changes?

Honestly it's been so long since I sent this I've lost most of my context, but it was mostly motivated by bugs I found in SmallVector that, while not existent in libc++'s std::vector, weren't tested either.

What errors do you expect these asserts to find?

This helps cover cases where container resizing is performed in an order that invalidates the to-be-added elements before they're inserted into the sequence. I don't know of any bugs in std::vector for this case, but I've seen SmallVector bugs (which I haven't got around to fixing) that this would catch.

Somewhat related to http://reviews.llvm.org/D4088 (similar test coverage, but in D4095 the tests were already there, it's just that the move-only type wasn't verifying certain behavior so it could've silently passed with the same sort of bugs)

I can go back & try to reproduce the particular valid cases that are buggy in SmallVector and non-buggy but untested in std::vector, if you'd like.

I can go back & try to reproduce the particular valid cases that are buggy in SmallVector and non-buggy but untested in std::vector, if you'd like.

I would appreciate it; thanks.

And I apologize for taking so long on this that you forgot the context.

dblaikie closed this revision.Aug 9 2014, 3:44 PM
dblaikie updated this revision to Diff 12328.

Closed by commit rL215301 (authored by @dblaikie).