[PATCH] Add an emplace(...) method to llvm::Optional<T>.

David Blaikie dblaikie at gmail.com
Tue Sep 30 09:42:12 PDT 2014


================
Comment at: unittests/ADT/OptionalTest.cpp:182
@@ +181,3 @@
+  int x, y;
+  MultiArgConstructor(int x, int y) : x(x), y(y) {}
+};
----------------
jordan_rose wrote:
> jordan_rose wrote:
> > dblaikie wrote:
> > > dblaikie wrote:
> > > > Maybe make this class non-copyable and non-movable, to ensure that emplace can be used on a non-movable type?
> > > > 
> > > > (or add that as a separate test - with a clear "ImmovableType" or something for testing (some of these basic feature types might eventually get pulled out into common test utility headers for use in a variety of container tests))
> > > Want to try adding an explicit ctor (or making this ctor explicit) just to demonstrate that emplace can/should be able to call explicit ctors? (I assume that's the intent - I haven't checked the standard proposal, etc)
> > Good point, will do.
> The MoveOnlyEmplace test below does test that—although it's not a non-movable type, it does track how many times the move constructor is called, and that's 0.
Yeah - in my experience it's pretty easy to introduce a compile time dependency on the existence of certain operations even if those operations are never actually executed.

(eg: clear() could be implemented as erase(begin, end) but that requires elements to be movable to move them down to fill the space (even though, dynamically, there's nothing to move down), unique_ptr(nullptr) implemented in terms of unique_ptr() (forwarding ctor) ends up meaning that unique_ptr(nullptr) needs access to the underlying object's dtor... , etc)

http://reviews.llvm.org/D5508






More information about the llvm-commits mailing list