| ||||
| Comment on GotW #6b Solution: Const-Correctness, Part 2 by Fernando Pelliccioni
Hi Herb.  Comment on GotW #90: Factories by Barry
Even though I brought it up earlier, why is using auto the preferred style in this situation? I prefer to stick with auto only when dealing with nested dependent typenames (in which cases writing out the type probably doesn’t add any clarity, if it’s even possible to do) and lambdas. Plus don’t you WANT to be made aware if somebody changed the underlying return type from a raw pointer to a smart pointer? Especially since (as other’s pointed out), you probably should have a delete somewhere… Read More »Comment on GotW #89 Solution: Smart Pointers by jlehrer
“c/c++ malloc works very deterministic and there is no intelligence. first free block that fits the size or more (requested) will be returned.” None of this is true. C++ does not dictate how the memory allocation routines should allocate their memory. This is completely implementation defined (as long as certain rules are followed regarding alignment). On my highly specialized platform we wrote a custom allocator that has zero overhead for the common allocation sizes while maintaining the normal overhead for all other allocation sizes. We also don’t have a fragmentation problem with these common sizes. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Herb Sutter
@Fernando: I just backed out that .load — whetner you transfer other’s cached area is just an optimization. And by removing it from the code, it helps to answer your other question, because it emphasizes that you don’t need to move the atomic for semantic correctness, and if you used a mutex you wouldn’t need to move that either: Just as when you move you don’t get a new atomic (it’s still the same atomic for this instance) you also don’t get a new mutex (it’s still the same as before). The only thing you have to move is the logical state, which is the vector of points, and set the cached area to a valid value or -1, then you’re consistent. So with a mutex, the move would look exactly the same as I’ve now updated it to be above — just move other.points and reset area: class polygon { public: /* ... all the other stuff ... */ polygon( polygon&& other ) : points{move(other.points)}, area{-1} { } polygon& operator=( polygon&& other ) { { points = move(other.points); area = -1; return *this; } private: vector<point> points; mutable mutex mutables; mutable double area; }; Read More » Comment on GotW #89 Solution: Smart Pointers by Herb Sutter
@celeborn2bealive: You're right, I've updated the wording to try to make these be phrased as "prefer" rather than "always." There are two parts to this: 1. unique_ptr and shared_ptr are to be preferred whenever possible, but you might have a reason to use another smart pointer type, such as legacy or for custom behavior you can't achieve with deleters and allocators on unique_ptr and shared_ptr. 2. make_unique and make_shared also should be preferred wherever possible, but you might have a reason not to use make_unique or {make|allocate}_shared if you need a custom deleter or are adopting a pointer. I've now added this to the discussion, including mentioning allocate_shared. Thanks! Read More »Comment on GotW #89 Solution: Smart Pointers by Herb Sutter
@andyprowl: Right you are. Fixed, thanks. Read More »Comment on GotW #89 Solution: Smart Pointers by Herb Sutter
@Marco, @nosenseetal: Virtually all types, including shared_ptr (and vector, and other types) are just as thread-safe as any old type including int — safe for concurrent reads (const operations) but require external synchronization if you know the object is shared and will have multiple threads trying to use it concurrently and at least one is a writer. The reason GotW #6b’s polygon needed “some” internal synchronization is the same reason std::shared_ptr needs “some” internal synchronization: to synchronize the under-the-covers shared state the caller can’t see and that isn’t covered by the usual external synchronization. The easy way to spot such *internal* shared state is that it’s shared state that can be modified in a const operation. That needs internal synchronization so that the caller’s usual external-sync duty of care is sufficient. The only types that are not like that are basically those designed to perform inter-thread communication and synchronization, like mutex and condition_variable and atomic. This is a FAQ, alas. I need to write that GotW on thread safety. Maybe later this summer… Read More »Comment on GotW #89 Solution: Smart Pointers by Herb Sutter
@Marek: Yes, unspecified. Thanks, updated. Read More »Comment on GotW #89 Solution: Smart Pointers by Herb Sutter
@Tom: I can’t easily share that, but you could try something like this (untested code follows, yours to test and fix). template<typename T, typename A1> std::unique_ptr<T> make_unique( A1&& a1 ) { return std::unique_ptr<T>( new T( std::forward<A1>(a1) ) ); } template<typename T, typename A1, typename A2> std::unique_ptr<T> make_unique( A1&& a1, A2&& a2 ) { return std::unique_ptr<T>( new T( std::forward<A1>(a1), std::forward<A2>(a2) ) ); } // etc. for as many constructor parameters as you want to support Read More » Comment on GotW #90 Solution: Factories by Herb Sutter
Repeating my comment I also made on the #89 solution: BTW, the reason I'm posting #89 to #91 in faster succession is because we already saw versions of these exactly one year ago, as then-numbered #103 to #105, so some of the comment discussion about them would be duplicated and I also wanted to get more quickly to the solution for the third which I didn't post last year. I think you'll see that all three have been considerably updated since the initial three questions and two solutions I posted a year ago — and not just because of make_unique, and its corollary don't-write-new, but also some other C++14-isms including the brand-new optional which, like make_unique, has only been in the draft standard since last month. Read More »Comment on GotW #90 Solution: Factories by esgames.org (@esgamesorg)
Why is optional::operator bool() declared explicit? Read More »Comment on GotW #89 Solution: Smart Pointers by nosenseetal
@herb btw I think you need to mention bitfields as odd(if you get how hw works not so odd) exception when talking about thread safety. Read More »Comment on GotW #90 Solution: Factories by germinolegrand
@esgames.org: to prevent things like return option + 1; which returns an int if i’m not mistaken. Read More »Comment on GotW #91: Smart Pointer Parameters by Ben Craig
(a) and (b) are likely to cause holy wars. If there is any kind of transfer of ownership (including passing off to other threads), then (a) and (b) are not appropriate. If (a) and (b) don’t change ownership, then either can be fine. Many people prefer (a) to (b), because it makes it more obvious at the call site that a level of indirection is involved. Other people will prefer (b), because it makes it very difficult to have NULL pointers. If f() is a “sink” function, then (c) is appropriate. By “sink”, I mean a function that accepts a resource, and destroys it as part of its execution. const unique_ptr ref doesn’t buy much, but if your code stores the resource as a unique_ptr, and you have lots of calls to f(), then you can avoid some syntactic overhead by passing const unique_ptr & instead of widget*. Similar arguments for (c) and (d) apply to (e) and (f), except instead of discussing “sinks”, we get to discuss extra owners. If f() needs to become an additional owner for the widget, then (e) is appropriate. Taking the shared_ptr by value here is preferable to taking a const shared_ptr ref and copy constructing a shared_ptr, as the value parameter may get to take advantage of move semantics (perhaps from a make_shared call). (f) has roughly the same semantics as (a) and (b), and like const unique_ptr &, it can make sense when you have lots of callers of your function. (f) makes a little more sense than (d) though, as you can decide several layers down that you need a copy / additional owner of your widget, and (f) permits that. With a unique_ptr, you can’t really decide several layers down that you should move from your const unique_ptr &. One of the downsides of (d) and (f) is that these signatures often overspecify your needs. (a) and (b) can accept widgets from unique_ptr and shared_ptr, but (d) can only (easily) accept widgets from unique_ptr. (f) can accept a widget from a unique_ptr, but it “steals” the widget, so it only works well with other shared_ptrs. There was a “dumb_ptr” spec at one point, but I haven’t followed standardization enough to know if it got accepted to C++14. I’m guessing not, as it would probably be mentioned by now if it had. Read More »Comment on GotW #90 Solution: Factories by tangzhnju
Why can optional::operator bool() be constexpr? Whether optional is valid or not is known at run time. Read More »Comment on GotW #90 Solution: Factories by Jon
“In C++98, programmers would often resort to returning a large object by pointer just to avoid the penalty of copying its state:” Comment on GotW #91: Smart Pointer Parameters by decourse
The difference between (c) and (d) is that (c) MUST take ownership of the object (or possibly destroy it), whereas (d) might not touch it. Use (d) if the function needs to think about whether or not to take ownership. There is a similar difference between (e) and (f). (e) says that f will own a copy, and (f) says it might or might not. Read More »Comment on GotW #91: Smart Pointer Parameters by Thanks
Maybe this should be renamed to GotD (for Guru of the Day). I can’t almost keep up with so much good material. Thanks! Read More »Comment on GotW #91: Smart Pointer Parameters by borishr
There’s extra shared count manipulation in void f( shared_ptr<widget> ); . C++98 way would be to use void f( const shared_ptr<widget>& ); . I’m still not familiar with C++11, but I suspect it will be something with two ampersands :-), like void f( shared_ptr<widget>&& ); . That would handle possible extra copy and shared count update in f( make_shared<widget>() ) I would think of void f( shared_ptr<widget> ); variants as equivalent to void f( widget* ); whereas void f( unique_ptr<widget> ); is equivalent to void f( widget*& pw ) {...; delete pw; pw=nullptr;} ; . So, for the question 3, I would use f( unique_ptr<widget> ); if f is a sink, ie we will not use that pointer again, and variant from question 1 otherwise. Also, I think that it only makes sense passing unique_ptr parameter by value. Read More »Comment on GotW #91: Smart Pointer Parameters by Ralph Tandetzky
1. Performance: There’s a little performance overhead of calling the function with shared_ptr<widget> [\code] compared to using [code] const widget & as parameter type. The function call is probably the most expensive thing for the caller here. However, passing a shared_ptr requires a thread-safe counter increment and a decrement at destruction which is definite overhead. Also, if the caller has a widget reference, but no shared_ptr to that widget, then a shared_ptr with a null deleter must be created unnecessarily. This requires another memory allocation for the deleter and reference counter, which can be expensive on the calling side. 2. Correctness: Usually a function with this signature would only need a shared_ptr , if it takes shared ownership of it or needs to pass shared ownership to someone else. This would only be useful, if the function stores a shared_ptr or a weak_ptr in some kind of global variable. If that isn't the case (and it most often isn't), the the function should prefer to take a widget & or const widget & . Even if there is a factory function like shared_ptr<widget> load_widget( widget::id id ); then a const reference would still make sense, since f( *load_widget( id ) ); would still work. The code would also still work and be correct, if the return type of the load_widget() function would be changed to unique_ptr<widget> . 3. Since the parameter widget is an input-only parameter and hence should not be modified, the type widget should be replaced by const widget in all cases, including template argument types. Now to the question what each declaration would mean: void f( const widget* ); (a) void f( const widget& ); (b) This should be prefered, since the function does not need to take ownership of the widget (at least most typically). Still (a) and (b) differ in the fact that (a) can be passed a null pointer to. Yes, it is possible to pass null references with code like this widget * p = nullptr; f( *p ); but normally people don't do that. Remember, we should write code that is easy to use correctly and hard to use incorrectly. Also (b) has the advantage, that a temporary object can be passed. For example f( create_widget() ); which wouldn't be possible with (a). Now let's consider void f( unique_ptr<const widget> ); (c) void f( unique_ptr<const widget>& ); (d) In case (c) the function f will eat the widget. This unnessesarily restricts the caller, since the caller cannot do anything with the widget afterwards. The same effect could be accomplished with the function declaration (b) by writing unique_ptr<widget> p = ...; /* ... */ f( *p ); p.reset(); The only advantage is that the callee (c) could use the unique_ptr to the const widget somehow where only a unique_ptr<const widget> can be used. However this is highly unlikely. The same applies to (d) only that it MIGHT eat the unique_ptr to the const widget. That would be even worse, since it's not clear from the declaration when this happens. The most surprising thing yet is that the callee can change the pointer so that it points to another object afterwards. Ouch! Hence we need to add another const to the declaration to prevent this: void f( const unique_ptr<const widget>& ); The unique_ptr cannot be moved then. But there's no advantage of passing a unique_ptr then over variant (a). It just makes the code a little more obscure. Last but not least, let's consider void f( shared_ptr<const widget> ); (e) void f( shared_ptr<const widget>& ); (f) I already discussed (e) in 2. In (f) incrementing the reference counter can be prevented. However, as in (d), the callee may change the pointer, hence we should add another const: void f( const shared_ptr<const widget>& ); Now this is almost the same as (e). Again, shared_ptr implies that shared ownership is being used. That is almost never necessary for simple input arguments. It's unnecessarily demanding to the caller to create a shared_ptr, if the shared ownership isn't even used. The parameter type is much more complicated compared to (a) and (b) and there's no significant advantage to (a). And (b) is probably even better than (a). Bottom line: Prefer plain pointers and references as function paramter types unless unique or shared ownership must be transferred. If ownership should be transfered pass a unique_ptr or shared_ptr by value. This clearly states the intent in the declaration and it is easy to read and efficient. I would prefer to use (b) as a function declaration, if the parameter is not optional. Otherwise, I would use (a) and pass a nullptr, if I want to omit the argument. Read More »Comment on GotW #90 Solution: Factories by Sebastian Redl
@tangzhnju: If you write optional(), you know statically that the object you created is empty. So operator bool on it is a compile time constant. Comment on GotW #90 Solution: Factories by Johannes Schaub (litb)
When the return type of the function is “optional” and you are returning a local variable of type “T”, NRVO doesn’t apply. I think that means you should explicitly move “vector” it in that case, because the implicit move-treatment is based on the possibility of being able to do NRVO, or has that rule changed for C++14? Read More »Comment on GotW #90 Solution: Factories by tangzhnju
@Sebastian Redl: Thanks for explaining constexpr to me which I did’nt quite understand before. Read More »Comment on GotW #91: Smart Pointer Parameters by David Thornley
3. (a) and (b) are the simplest ways to pass the Widget in. Since this is a required read-only parameter, the ability of (a) void f(const Widget const *) to pass nullptr in is irrelevant, and its only advantage is making it very obvious that there’s indirection going on, which is usually not all that important. It might be useful as a reminder if the programmer wants to be reminded to microoptimize memory references, but this is almost always pointless. One potential disadvantage is that somebody could put a delete widget; in the function, but if the shop uses the coding standard that a raw pointer never owns, that should be caught in code review. (b) void f(const & Widget) reads better, captures the fact that the Widget is required, and doesn’t lend itself to accidental deletion. I have been unable to imagine any use for (d) and (f). Semantically, a reference is another name for something, and (without bothering to try figuring it out) it seems to me that having two names for a unique_ptr or a shared_ptr is a bad idea. Over the years, I’ve learned to stay out of the dark corners of the C++ standard, because things lurk there that bite. (c) void f(unique_ptr<const Widget>) unequivocally transfers ownership. This is the right thing to use if and only if that’s what’s desired. For example, we may want to create a Widget and transfer it to a wrapper that is the Widget’s only link to the outside world, possibly if we’re writing a multiplatform interface or otherwise need consistent calls to inconsistent Widgets. If the idea is to make sure the Widget goes away, it might be just as useful to have the unique_ptr in the calling function, and call f using (a) or (b), since this will save the small amount of time to copy the unique_ptr. (e) void f(shared_ptr<const Widget>) gives f shared ownership. This is useful if and only if the Widget might go away while the Widget is still in use. The obvious example would be f copying the Widget to a class data member, since the class may well outlive the calling function. If the program is single-threaded, or the calling function calls f in a thread that it later necessarily joins, f’s execution is going to be over before the calling function returns, and there’s no real need to pass a unique_ptr or shared_ptr unless f transfers its ownership share to a longer-lasting variable or object. If f is called in a thread that may outlive the calling function, then it needs at least partial ownership, and so (c) or (e) is necessary. Since the Widget is an input parameter, it should not be changed, and therefore every example of Widget should be replaced by const Widget or Widget const . This means that the calling function can pass a const Widget to f, and serves as a reminder that the Widget should not be changed. In (a), the pointer itself can be a const, and that establishes the pointer itself as input-only. Read More »Comment on GotW #91: Smart Pointer Parameters by Brian M
It would be (a) or (b) for no other reason than the need for compatibility with as many c++ compilers as possible. Maybe not the correct Guru/cool answer but one that is a practical real world answer! Even just targeting latest there is still the advantage of keeping things simple and familar unless there are overriding reasons for extra complexity. The KISS principle should be stamped on every engineers forehead….. Read More »Comment on GotW #91: Smart Pointer Parameters by paercebal
This will make a copy of the shared_ptr, which will only increment the reference counter, which is, unless I’m wrong, an atomic variable (to make sure two threads could safely copy shared_ptrs pointing to the same object, as they would copy raw pointers). The thing is, we usually don’t even want to pay this cost, so, as we know the object is owned by the previous stack level, we can instead pass the shared_ptr object by const reference, which changes (almost… see below) nothing to the semantics, and makes the parameter passing costless.
The pointed widget object is owned by the stack at two levels: f, and its caller, which can be overkill in most codes. Also, as f has shared ownership, it means it is ok for this function to share it again with other parts of the code (which opens the way to ownership cycles if not monitored).
(a) and (b) are cases where f is not given ownership to the widget parameter, so it can’t, in turn, give or share it. (a) means the parameter being nullptr is a valid value, so f is expected to behave. (c) has a pure ownership transfer semantic: f will take over the resource, no negociations. The caller must know that its own unique_ptr (or resource) won’t be owned by it anymore the moment f is called. (d) has conditional ownhership transfer IF the unique_ptr is not const. That is, the function fa can decide (or not) to take ownership of the resource, and thus empty the pointer. IF the unique_ptr is const, then this is just passing a smart pointer parameter, without transfer of ownerhisp (similar to case a) (e) has ownership sharing semantics. This means that both the caller and f will share the resource… And potentially other parts of the code, as f can freely share it, too. Having this kind of prototypes means your object is out in the wild… Isn’t that where we should wonder about potential ownership cycles that will leak memory? (and where weak_ptr could help break them?) IF the shared_ptr is const, it changes nothing (I see right now the copy constructor of shared_ptr accepts “const shared_ptr &”). Unless I’m wrong, the shared_ptr could be constructed from another shared_ptr of a compatible type (derived-to-base conversion) (f) IF the shared_ptr is const, this is like e, but we avoid one counter increment. The const reference means that, the shared_ptr have again derived-to-base conversion. If the parameter is passed as non-const reference, then no conversion is possible… and we fall back into e’s case. Read More »Comment on GotW #90 Solution: Factories by monamimani
In question 4 I think you should be talking about vector. It makes it irrelevant to compare it with the polymorphic type widget. By that I mean of course you can return by value a vector because it is heap based. But the question remains to be answer, would you return a widget by value if it is not a polymorphic type. The function should probably be changed to create instead of load because there is no failure possible when you return by value. But still let take a common class, a mammoth class, not something small like a Matrix4x4, but something with bad design that does too much things and has plenty of states, bools, floats,….. I would still think twice to return that by value. It might not be call often in that case well you need to decide if you pay the price. I am just saying that this is probably more what should have been the answer to number 4 and have an other question or a note for cheap moveable type like vector. Read More »Comment on GotW #91: Smart Pointer Parameters by Róbert Dávid
@David Thornley: “If the idea is to make sure the Widget goes away, it might be just as useful to have the unique_ptr in the calling function, and call f using (a) or (b), since this will save the small amount of time to copy the unique_ptr.” If you do that, you delay the destruction of widget after the end of the calling scope, what might not be what you wanted, and also, you save nothing, as a unique_ptr is just a zero overhead wrapper around a plain old pointer, copying (or rather, moving/creating, as the whole idea is to disable copy) costs exactly the same as copying the plain pointer. You might say that “but I also need to set the source to nullptr on move!” Well, check the disassembly after an optimized build, you probably will see this being skipped. Read More » | ||||
| ||||
Friday, May 31, 2013
FeedaMail: Comments for Sutter’s Mill
Thursday, May 30, 2013
FeedaMail: Comments for Sutter’s Mill
| ||||
| Comment on GotW #89 Solution: Smart Pointers by Andrew Marshall (@planetmarshall)
What’s the advantage (if any) of weak_ptr over a non-owning raw pointer? Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Tomáš Šturm
You wrote on operator+: polygon may be a small type but it still contains std::vector as a member. And move-constructing that could make a significant difference in some use cases, couldn’t it? Read More »Comment on GotW #90: Factories by Ben Craig
1. Using a raw pointer doesn’t express the ownership semantics of the widget resource. Owning raw pointers should be avoided. 2. If widget is polymorphic, then unique_ptr<widget> is preferred. unique_ptr is preferable to a raw pointer or raw reference because of the automatic cleanup, but you still need some kind of pointer-like type for virtual functions to work. 3. If all callers were using unique_ptr already, then changing the return type from widget* to unique_ptr will not require source changes in client code. The unique_ptr constructor can take a returned unique_ptr just fine thanks to the move constructor. 4. If widget isn’t polymorphic, you should just return a widget value, and let move constructors do their thing. This lets you avoid the heap. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Herb Sutter
@Fernando: You are correct, thanks. I’ve now added the copy and move operations; fortunately I could use = default for the latter. Read More »Comment on GotW #89 Solution: Smart Pointers by Marco
One thing we were discussing about a lot: are c++ shared_ptr threadsafe? Read More »Comment on GotW #89 Solution: Smart Pointers by Tom Kirby-Green
Hi Herb, is there any chance you could post somewhere your preferred pre ‘Milan’ (variadics) implementation for VS2012 Update 2 MSVC developers who want to use a pseudo blessed implementation of make_unique today? Read More »Comment on GotW #5 Solution: Overriding Virtual Functions by nosenseetal
or else it isn't, in which case the function has to be protected (private is not allowed because the derived destructor must be able to invoke the base destructor) and would naturally also be nonvirtual (when the derived destructor invokes the base destructor, it does so nonvirtually whether declared virtual or not). why it cant be public ? Doesnt placement new/delete require that(lets pretend that some crazy person wants to put polymorphic object in char[compile_time_max(sizeof (Base), sizeof (Der1),sizeof (Der2)...] Comment on GotW #6b Solution: Const-Correctness, Part 2 by mttpd
@Herb: Fair enough. Out of curiosity, are you concerns similar to those in the SO link (interaction with uniform initialization) or are there some other reasons? // Huh, perhaps brainstorming on this could make an interesting (JG?) question on its own… Read More »Comment on GotW #90: Factories by Ahti Legonkov
I agree with most what Ben Craig said, except with answer to 3. 3. What makes me so confident is the fact that in most places callers use `auto` instead of explicit types. Read More »Comment on GotW #90: Factories by Barry
I think Ben has it right, though I wonder if for (3) Herb meant that callers would be using: auto my_widget = load_widget(42); And that’s why it wouldn’t matter. Read More »Comment on GotW #89 Solution: Smart Pointers by nosenseetal
@herb (this is not ok) (up, sp set in other thread) @marco Comment on GotW #6b Solution: Const-Correctness, Part 2 by Fernando Pelliccioni
Hi Herb, hummmm… I think that “move-ctor = default” is not going to help. I don’t see anything abount std::atomic’s move-ctor in the Standard ( n3691 – [atomics.types.generic] ) If atomic copy-ctor is explicitly deleted, the, the move-ctor is deleted too ( implicitly ). I expected to find something like … atomic(atomic&&) = default; … in the Standard, but I didn’t find it. All this makes it very difficult to create classes with std::atomic or std::mutex as members. Comment on GotW #89 Solution: Smart Pointers by Anders Dalvander
make_shared can be wasteful if you have large objects and weak_ptr to these shared objects as the memory for the large object cannot be deallocated before the last weak_ptr has been destroyed. Sometimes it is better to use new instead of make_shared. Read More »Comment on GotW #89 Solution: Smart Pointers by paercebal
@Anders Dalvander A solution is to write your own “make_my_shared” insuring it will be expection safe and the new will be done outside the standard make_shared. This way, you keep the exception safety and follow the “don’t-write-naked-new” guideline, all the while separating the memory for the reference counters from the memory for your very-large-object. Read More »Comment on GotW #89 Solution: Smart Pointers by Kobi Cohen-Arazi
@herb Comment on GotW #90: Factories by BK
For a hierarchy of widget classes, the overridden load_widget function can return the specific widget type. The requirements for a return type by overridden functions is that they return covariant types (see n3242.pdf section 10.3 #7) Read More »Comment on GotW #89 Solution: Smart Pointers by Adrian
What happens if the class being instantiated with make_shared has class-specific new and delete operators? Will make_shared use that one (and thus still make a separate allocation for the control block)? Read More »Comment on GotW #90: Factories by jlehrer
for #4 you return a shared_ptr. This is because the shared_ptr holds a pointer to the destruction function which is determined at construction time. So, if the code creates a WidgetSubclass, then when the shared_ptr reference count goes to 0, the destructor function is called, and that function is ~WidgetSubclass, even if widget* is not polymorphic. Read More »Comment on GotW #89 Solution: Smart Pointers by andyprowl
I think the first two calls to sink() (in the answer to Q3) should not compile, since the constructor of unique_ptr that accepts a raw pointer is marked as explicit. Am I overlooking something? Read More »Comment on GotW #89 Solution: Smart Pointers by celeborn2bealive
Very interesting, I love your articles :) But I have a question about make_unique() and make_shared(): If i don’t say bullshit, when you allocate memory from a dynamic library, that memory must be freed in the code of the library. So for exemple if I have a function with the following signature in my library: std::unique_ptr<Widget> createButton(); I don’t understand how the compiler do things right to release the memory for the Widget in the library code (if it does…) since unique_ptr is a template class and I think its code will be instanciated in the client code. I know I can provide a custom deleter for the pointer (std::unique_ptr createButton();) to be sure, but in that case I cannot use make_unique() to create it (the same question holds for shared_ptr and it’s Deleter functor). Thank you for your answer and correct me if I’m wrong :) Read More »Comment on GotW #89 Solution: Smart Pointers by Michael Winterberg
nosenseetal: I think that you need to use the overloads of atomic_load and atomic_store that take shared_ptr for that to be acceptable. i.e. I’m pretty sure that individual instances of shared_ptr are not thread safe, only the reference counts that may be shared between multiple instances of a shared_ptr are. Read More »Comment on GotW #89 Solution: Smart Pointers by jlehrer
Responding to celeborn2bealive “when you allocate memory from a dynamic library, that memory must be freed in the code of the library.” says who? the C++ library says nothing about that. I know that with Visual C++, given certain compiler flags, that might be true. But it certainly isn’t true on any platform I use. If that is true on your platform, then you have some choices. My preferred method is to use an object factory which returns a shared_ptr. The shared_ptr, at construction time, encodes a pointer to the destruction function. The object factory then both constructs the new object AND encodes the destructor function, for free. When the shared_ptr’s counts go to 0, the caller, even if in a different library/toolkit/translation unit, still calls back into the same TU as the object factory to free the memory. Again, this happens automatically because the destructor function is populated in the same TU as the initial construction of the referenced object in the shared_ptr. Read More »Comment on GotW #89 Solution: Smart Pointers by David Svoboda
Great article but I have one question. I always assumed that you cannot make a vector of unique pointers of objects because the vector demands that an object always be copyable. And unique pointers are definitely not copyable. Perhaps the move semantics in C++ 11 Are what enables the creation of a vector of unique pointers. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Leo
“Incidentally, notice that once we make rhs a reference-to-const parameter as noted above, we see another reason why get_point should be a const member function, returning either a const value or a reference to const.” Currently get_point returns neither. int i ... poly.get_point(i) = ... poly.get_point(i) += ... from compiling? But when assigning to a variable the top level const would be ignored and the variable could be used without constraints? Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Francisco Lopes (@pepper_chico)
@Herb: You wrote: “@Francisco: You wrote: "The answer contains good wisdom on const correctness, but got confused about the concurrency semantics meaning/correctness…" – could you elaborate on what you mean?” – First, I meant: “I got a little confused”, not the article, and, it’s nothing all special but just what one can derive from the general reaction to the article. The things related to internal/external synchronization, I think it may have not been explicit enough for the layperson, even more for the ones used to externally synchronized libraries. I wait for the synchronization wisdom from the mentioned GOTW to deal with this api contracts, etc. Read More »Comment on GotW #89 Solution: Smart Pointers by Chris Vine
“sink( new widget{}, new gadget{} ); // Q1: can you spot the problem?” The principal problem with this is that it will not compile, because unique_ptr’s constructor taking a pointer is explicit. sink(std::unique_ptr<widget>{new widget{}}, std::unique_ptr<gadget>{new gadget{}}); will compile but is not exception safe. Not only is the order of evaluation of the arguments unspecified, but the compiler is entitled to to construct gadget, and then before initializing the unique_ptr to take gadget construct widget, and only then construct the unique_ptr objects. Of course, it would be a very odd compiler which did it this way. Allowing this loose evaluation is in my view a defect in the standard, but I know that most others disagree. Read More »Comment on GotW #90: Factories by Ben Craig
After reading other people’s comments, I’ll agree with what I said, except for (3) :). Auto is the more likely answer, with unique_ptr being a runner up. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by edflanders
Regarding the guideline “Prefer passing a read-only parameter by value if you're going to make a copy of the parameter anyway…”: Does that apply to cases where need to copy to a specific location? E.g.: struct S { S(T t) : t_(t)) {} // two copies? void SetT(T t) { t_ = t; } // two copies? T t_; }; Does the guideline hold, or should you still pass T const& in these cases? Read More »Comment on GotW #90: Factories by Joe Gottman
For #4 you just return a Widget. If the class in non-polymorphic there is very little reason to go through the expense of a new and delete. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Sean Lin
In old GotW 6, there is a guideline said: no longer true? If it is still prefered, get_point should return const point, right ? const point get_point( int i ) const { return points[i]; } Read More » Comment on GotW #90: Factories by jlehrer
WRT #4, I have had plenty of cases where a type was not polymorphic but did have an inheritance tree. The object factory returns a shared_ptr to the base for the reasons I described. If there is no sublcass of Widget, then, yes, I’d probably return a simple object. Read More »Comment on GotW #90: Factories by Neil
Re #4, the recommendation of returning Widget by value assumes that (N)RVO will kick-in or Widget’s move semantics are more optimized than its copy semantics. However, if Widget is composed of non-movable members (e.g. an aggregate/POD-type) then move will be no better than copy, so if RVO does not apply then the cost of copying Widget may be greater than new-ing up a Widget* and returning it via unique_ptr. Read More »Comment on GotW #90: Factories by Mikhail Belyaev
1. It. Returns. A pointer. Nothing here says if this pointer is to some internal data, or static variable, or a new-allocated structure or (even!) to a malloc-allocated structure. What are we supposed to do with it? We are supposed to read the source code. That breaks the entire encapsulation thing factories were originally ment to bring. And with all the variants I supposed, none of them really needs a pointer returned. Too bad LLVM exploits exactly this approach a lot. If it is a real newly-allocated polymorphic pointer, use std::unique_ptr. It has minimal tradeoffs (you just actually need the new-allocation) and fully expresses your intent. Even if some guy comes buy and calls release(), it’s not your fault. If someone needs to share it, he can always create a shared_ptr manually. If you need to share the pointer (for example, you keep track of all values elsewhere or your method is supposed to return an exact same pointer for a sequence of calls), use the shared_ptr. You can also consider using this if you are stuck with a need to create a uniform interface for both new-allocated, static and need-to-be-deleted-by-other-means values (that actually happened to me) and provide a dynamic deleter for each with a lambda. unique_ptr cannot do that as its deleter is statically-bounded. 4. If it is not really a polymorphic type, just return it by value and let compiler handle the rest unless you know for sure that the type cannot be efficiently move-constructed. Then we are back at #3. Read More »Comment on GotW #89 Solution: Smart Pointers by Marco
Thanks for the answer. I was referring to the reference counter. Good to know it is thread safe. We have had our own implementation of smart and weak pointers. Now we’re about to go to VS 2012, so I’m looking forward using all these great new features! Read More »Comment on GotW #90: Factories by Thibaud
For #3 if callers are using auto, they are using delete as well and you still have to remove them all. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Arne Mertz
@edflanders: struct S { S(T t) : t_(std::move(t))) {} // at most one copy, one or two moves void SetT(T t) { t_ = std::move(t); } //at most one copy, one or two moves T t_; }; It’s in both cases one copy, one move, if the argument is an lvalue, and two moves, if it’s an rvalue. In the setter, t is move-or-copy _constructed_, while t_ is move-assigned. Read More »Comment on GotW #89 Solution: Smart Pointers by tr3w
If I’m not mistaken, the smart and weak ptr works like this: But here is the catch: If we used make_shared, the shared_ptr destructor can’t deallocate the area of the pointed object, only the weak_ptr will deallocate it (when it gets rid of the control block). Am I right? Read More »Comment on GotW #90: Factories by Elvis
I Agree with Ben except for point 3. load_widtget must return std::unique_ptr<widget> where widget is an pure abstract class. Read More »Comment on GotW #89 Solution: Smart Pointers by pip010 (@ppetrovdotnet)
well, instead: unique_ptr arg1(new widget{}); so here it goes. same exception-safety :) about: “It reduces allocation overhead, including memory fragmentation” same goes for locality? Read More »Comment on GotW #89 Solution: Smart Pointers by Marek
> The answer is no, because C++ leaves the order of evaluation of function arguments undefined… I think it is unspecified not undefined. Read More »Comment on GotW #89 Solution: Smart Pointers by Marco
@pip010: The two memory blocks will in general have very different sizes and may life in far far away areas of the memory because the system tries to minimize fragmentation. Read More »Comment on GotW #90: Factories by Sebastian Redl
I think Ben Craig’s initial instinct was more right. Comment on GotW #90: Factories by Laurent L.
Pretty good answers sofar, regarding polymorphism and ownership. I would add a question : Does the absence of “noexcept” tell the caller that failure management is performed through throwing an exception? In that case, all “widget*” solutions should be turned into “widget&” since the normal course of things guarantees that an object will be returned. And the smart pointers cannot be null, so they can be used safely without checking. IOW, any widget* solution should also say “noexcept”. Read More »Comment on GotW #89 Solution: Smart Pointers by pip010 (@ppetrovdotnet)
@Marco Comment on GotW #6b Solution: Const-Correctness, Part 2 by edflanders
@Arne Mertz: struct Matrix3x3 { double m_[3][3]; }; typedef Matrix3x3 T; Read More » Comment on GotW #89 Solution: Smart Pointers by pip010 (@ppetrovdotnet)
@Herb looking at VS2012 ans std::make_shared new _Ref_count_obj(LIST(_FORWARD_ARG)); //inside make_shared Comment on GotW #6b Solution: Const-Correctness, Part 2 by Elvis
void calc_area() const in my opinion must be double calc_area() const in this way we can avoid mutable area. Read More »Comment on GotW #89 Solution: Smart Pointers by Jeff Sullivan
Given that make_unique and make_shared can aggregate the memory allocation for the ref –counting together with that for the pointed at T, how does that play with class member operator::new() and operator::delete()? Depending on how this is managed you are either going to inherit the custom operators (which would be bad if it an allocator that assumes it only ever has to allocate sizeof(T) objects), or you are going to entirely circumvent efforts by a class designer to provide specific allocation facilities (which might lead to memory fragmentation and poor performance due to broken cache coherency, or simple failure to work if all members of a class are supposed to be in some shared memory pool). It would seem to be worthwhile pointing out that mixing classes with custom allocators together with make_shared or make_unique is a Bad Idea. To solve the problem for shared_ptr there is allocate_shared() that will take an allocator for generating shared_ptr which will get the same result as a custom operator::new()/operator ::delete() set (with a little rewriting) , However the only references I can find to "allocate_unique" are to Oracle DBMS systems…. Is this another candidate for the "Oops, we should have done that" C++14 list, or are there some subtleties I haven't considered? Read More »Comment on GotW #89 Solution: Smart Pointers by Eric Fortin
@Andrew Marshall weak_ptr gets notified when the object is deleted and will prevent you from accessing it where a raw pointer will let you do anything with the already deleted object sending you in undefined land. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Herb Sutter
@Fernando: Sigh, you’re right. You know, that was my reading too and I wrote them out, then noticed that GCC let me get away with =default for the move operations and didn’t reread the standard. Since I had already written them out before switching to =default, the fix was as easy as hitting Ctrl-Y twice. :) Fixed mo betta. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Herb Sutter
@edflanders: Yes, because you then take by value + move(param) as Arne noted. The only case where that’s not a good idea is when you have a type that’s expensive to move, such as std::array. I currently plan to cover this in a new GotW on general parameter passing, probably sometime in June or July. @Elvis: That wouldn’t allow area to be non-mutable. It’s mutable because it can be written to in const operations, which is still true if you restructure the code the way you suggest. As long as there’s a cached area that can be written to in const operations, it needs to be mutable, and synchronized as atomic or with an internal mutex. Read More »Comment on GotW #6b Solution: Const-Correctness, Part 2 by Herb Sutter
@Leo: Aha, thanks for spotting that leftover text from the original GotW. In several original GotWs I used to recommend considering returning a const value, instead of a non-const value, on the basis that it would help callers by letting them know when they accidentally modify a returned rvalue. I no longer think that’s good advice, especially since you want to move from rvalues which is a non-const operation, so I’ve been rooting it out of the GotWs as I go through, and I got rid of most of the references to that in this one but missed that one. Thanks, fixed. Read More » | ||||
| ||||