6 ms·

Common C++ Gotchas

91 points by vickychijwani 12y ago

santaclaus 12y ago
> Option 1 is bad because: (a) it involves an unnecessary copy constructor invocation Shouldn't rvalue move semantics kick in and prevent the copy?
- vickychijwani 12y ago
  Agreed. I'll add a note. There's still the slicing though.
  - nostrademons 12y ago
    For slicing, the general rule in C++ is whenever you have a value object, polymorphism does not apply. A value object is always whatever its compile-time type is - after all, it's just a value, it can't represent some other "thing" that's off in some other memory space. Polymorphism only applies with pointers and references. Remember this rule and the throws behavior makes perfect sense, as do a lot of other C++ gotchas.
wfunction 12y ago
Why are you using delete so often in C++...? 'delete' should be rare in your code.
- robododo 12y ago
  You still need delete for some things. Smart pointers are not a panacea.
  - wfunction 12y ago
    Sorry I meant to ask why is it a habitual thing.
  - TillE 12y ago
    The only common exceptions for smart pointer usage should be for interfacing with an awkward C library or the OS, in which case you'd typically be using specific memory allocation functions, not new/delete.
- Aldo_MX 12y ago
  Why shouldn't you? You don't always have control over the code you are writing, an example that comes to my mind quickly is ffmpeg.
  - vinkelhake 12y ago
    How does ffmpeg relate to this, isn't it written in C?
    - shadowfox 12y ago
      He may be talking about interfacing with C code of that nature. (You can still localize most of your new/deletes in to the wrapper classes though)
  - nly 12y ago
    Because using new and delete bare makes writing exception safe, leak-free code about 10 times harder. And I'm pretty sure ffmpeg is written in C.
- vickychijwani 12y ago
  Someone over on Reddit asked the same thing. My reply: You're exactly right. But there are multiple complications: - unique_ptr and shared_ptr are only available in C++11 onwards, we're still to migrate to a new compiler - auto_ptr is a joke [1] - I've got to live with `delete` in our gargantuan codebase at work - I could (and should) use boost's smart pointers, and indeed I do when possible. But I admit I sometimes even forget to use those and end up with naked pointers instead. [1]: http://stackoverflow.com/a/111531/504611 http://stackoverflow.com/a/111531/504611
  - ajross 12y ago
    Promiscuous use of smart pointers is almost as bad a code smell as manually newing and deleting stuff. Ownership should be object lifetime, and it's really not hard to arrange member objects to make that work. And if you really can't, and you need a container that automatically frees its contents when destroyed, just stick it in a STL vector.
    - nostrademons 12y ago
      Promiscuous use of shared pointers is a code smell. std::unique_ptr (or its predecessor, boost:scoped_ptr if you don't have access to C++11) should be used all over the place. There are many reasons why you may not want by-value containment of objects, and std::unique_ptr gives you object-lifetime ownership of pointers.
      - wfunction 12y ago
        > There are many reasons why you may not want by-value containment of objects I'm interested in reading them actually, would you mind listing them?
        nostrademons 12y ago
        Nullability - a scoped_ptr or unique_ptr can be null, a value object can never be. Polymorphism - a value object is always exactly its static type, but you can substitute in different derived types for a unique_ptr. The previously-held object is automatically destroyed when you do this. This is essential for the State and Strategy patterns. Reassignment/swap - reassigning a unique_ptr destroys its previous value and transfers ownership of the new one. Reassigning a value object invokes operator=. The former can (sometimes, not always) be faster than the latter - think about cases where you just want a quick pointer swap inside the object, rather than having to shuffle all the bits in a large struct around. Release - unique_ptr has a member function to release ownership of the object, while this concept doesn't apply to value objects. Where might this be useful? Well, imagine trying to achieve exception safety inside a factory function with complex construction logic, and then transferring ownership to the caller. It's common to immediately assign the new object to a unique_ptr upon construction (so if an exception is ever thrown, it is destroyed properly), perform your logic, and then return ptr.release().
kazinator 12y ago
It's not only C++ that has problems with throws out of destructors or similar logic. How about Common Lisp: http://clhs.lisp.se/Issues/iss152_w.htm http://clhs.lisp.se/Issues/iss152_w.htm "The ambiguity at issue arises for the case where there are transfers of control from the cleanup clauses of an UNWIND-PROTECT. ..." It is not clear what exit points are visible when the cleanup-forms (morally similar to C++ destructor calls) are invoked. Are the exit points that are skipped torn down all the way to the control transfer's target, or is the tear-down interleaved with the unwinding.
- blt 12y ago
  The real answer, as far as I can tell, is "Don't use exceptions in a program that's too important to crash."
  - kazinator 12y ago
    Right, and to that end, don't use a CPU that traps illegal instructions, divisions by zero, pointers to unmapped pages and so on. Those are all forms of exceptions.
nly 12y ago
#3 should really say make public base class destructors virtual #6 sometimes you actually will want an A(A&) constructor... for example, if you have constructor overloaded with a greedy template argument. #8 objects returned by value will also bind to an rvalue reference #9 Remember in C++11 there's also new T{}; #10 isn't absolutely correct either, dynamic_cast can actually be used in some circumstances where static_cast or implicit casting could have been used. struct A { }; struct B: A { }; int main() { auto b = new B(); auto a = dynamic_cast<A*>(b); // fine }
- Suncho 12y ago
  #3 should really say "make polymorphic public base class destructors virtual." There are reasons why you might want to use public inheritance that have nothing to do with run-time polymorphism.
  - nly 12y ago
    Right, but unless your Base or Derived class is going to pass Base*/&s to friends and/or other instances, you typically don't want your protected destructors to be virtual just so you can get safe destruction. Of course, the thing with C++ is, the longer you sit and think about it, the more reasons you can come up with a reason to do something atypical Slicing isn't even necessarily harmful either... but I'm not sure what you had in mind.
  - jfoutz 12y ago
    I haven't touched C++ in years. Why would you ever make a non-virtual destructor? Isn't it totally up to the user of a class if they choose to inherit from the class? It seems like, if you're not providing a virtual destructor, you force users into delegation rather than inheritance, and that will catch some % of developers who aren't on the ball. They'll have leaks... As far as i know (old info) there's no such thing as a "final" class, so anybody can inherit even if you don't want them to.
    - gurkendoktor 12y ago
Suncho 12y ago
#1 should really say, "Avoid new and delete." new and delete are for experts. The average C++ programmer should go nowhere near them. If heap memory is used internally by a class that was written by someone who knew what they were doing, then fine. Use your std::vector. But new and delete are huge red flags in code.
- TillE 12y ago
  It's fine to use new as long as you're immediately sticking the result in a smart pointer. But yes, delete should be very very rare in modern C++ code.
  - hendzen 12y ago
    With std::make_shared, and the addition of std::make_unique in C++14, you shouldn't ever need to use new for this purpose either.
    - Suncho 12y ago
      It depends. The trick to using new correctly is to put it in a function whose sole purpose is to immediately store new's result in the smart handle. I.E. You're writing a function that's analogous to the make_unique function. You might need to do this if you're implementing your own smart handle or writing a factory function for a class with a private constructor (make_unique will not work). Off the top of my head, I can't think of any good reasons to use delete.
      - lomnakkus 12y ago
        If your handles are pointer types, you should still use unique_ptr, you just need to give it a custom "deleter". (Of course you should probably still encapsulate this in a function like make_xxx() just to avoid the potential mistake of not supplying the correct deleter everywhere.)
- toolslive 12y ago
  and yet, "new" and "delete" are amongst the very first things covered in every C++ tutorial book. for example in Stroustrup's "A tour of C++", it's covered in section 1.6. without any remark about shared_ptr and friends.
mpyne 12y ago
I would actually revise #1 to be to always use a smart pointer for heap-allocated objects. C++11 is 3 years old now after all.
plicense 12y ago
Selection messed up in your website: http://imgur.com/YuowMEm http://imgur.com/YuowMEm
Arelius 12y ago
#5 > If you'll tolerate my hypocrisy for a moment, here's my suggestion: try to avoid putting the const at the beginning like that Or you could not do that, and put const in the least ambiguous place, and since you rarely have a 'type* const ' it works very well just using: const type* for pointers to const types, and type* const for const pointers to types, and entirely unambiguous.
snarfy 12y ago
I wrote C++ code for about 10 years before moving over to C#. I still like C++, but posts like this make me remember why I don't miss it. It's so baroque.

91 points by vickychijwani 12y ago

> Option 1 is bad because: (a) it involves an unnecessary copy constructor invocation Shouldn't rvalue move semantics kick in and prevent the copy?

vickychijwani 12y ago
Agreed. I'll add a note. There's still the slicing though.
- nostrademons 12y ago
  For slicing, the general rule in C++ is whenever you have a value object, polymorphism does not apply. A value object is always whatever its compile-time type is - after all, it's just a value, it can't represent some other "thing" that's off in some other memory space. Polymorphism only applies with pointers and references. Remember this rule and the throws behavior makes perfect sense, as do a lot of other C++ gotchas.

wfunction 12y ago

Why are you using delete so often in C++...? 'delete' should be rare in your code.

robododo 12y ago
You still need delete for some things. Smart pointers are not a panacea.
- wfunction 12y ago
  Sorry I meant to ask why is it a habitual thing.
- TillE 12y ago
  The only common exceptions for smart pointer usage should be for interfacing with an awkward C library or the OS, in which case you'd typically be using specific memory allocation functions, not new/delete.
Aldo_MX 12y ago
Why shouldn't you? You don't always have control over the code you are writing, an example that comes to my mind quickly is ffmpeg.
- vinkelhake 12y ago
  How does ffmpeg relate to this, isn't it written in C?
  - shadowfox 12y ago
    He may be talking about interfacing with C code of that nature. (You can still localize most of your new/deletes in to the wrapper classes though)
- nly 12y ago
  Because using new and delete bare makes writing exception safe, leak-free code about 10 times harder. And I'm pretty sure ffmpeg is written in C.
vickychijwani 12y ago
Someone over on Reddit asked the same thing. My reply: You're exactly right. But there are multiple complications: - unique_ptr and shared_ptr are only available in C++11 onwards, we're still to migrate to a new compiler - auto_ptr is a joke [1] - I've got to live with `delete` in our gargantuan codebase at work - I could (and should) use boost's smart pointers, and indeed I do when possible. But I admit I sometimes even forget to use those and end up with naked pointers instead. [1]: http://stackoverflow.com/a/111531/504611 http://stackoverflow.com/a/111531/504611
- ajross 12y ago
  Promiscuous use of smart pointers is almost as bad a code smell as manually newing and deleting stuff. Ownership should be object lifetime, and it's really not hard to arrange member objects to make that work. And if you really can't, and you need a container that automatically frees its contents when destroyed, just stick it in a STL vector.
  - nostrademons 12y ago
    Promiscuous use of shared pointers is a code smell. std::unique_ptr (or its predecessor, boost:scoped_ptr if you don't have access to C++11) should be used all over the place. There are many reasons why you may not want by-value containment of objects, and std::unique_ptr gives you object-lifetime ownership of pointers.
    - wfunction 12y ago
      > There are many reasons why you may not want by-value containment of objects I'm interested in reading them actually, would you mind listing them?
      - nostrademons 12y ago
        Nullability - a scoped_ptr or unique_ptr can be null, a value object can never be. Polymorphism - a value object is always exactly its static type, but you can substitute in different derived types for a unique_ptr. The previously-held object is automatically destroyed when you do this. This is essential for the State and Strategy patterns. Reassignment/swap - reassigning a unique_ptr destroys its previous value and transfers ownership of the new one. Reassigning a value object invokes operator=. The former can (sometimes, not always) be faster than the latter - think about cases where you just want a quick pointer swap inside the object, rather than having to shuffle all the bits in a large struct around. Release - unique_ptr has a member function to release ownership of the object, while this concept doesn't apply to value objects. Where might this be useful? Well, imagine trying to achieve exception safety inside a factory function with complex construction logic, and then transferring ownership to the caller. It's common to immediately assign the new object to a unique_ptr upon construction (so if an exception is ever thrown, it is destroyed properly), perform your logic, and then return ptr.release().

kazinator 12y ago

It's not only C++ that has problems with throws out of destructors or similar logic. How about Common Lisp: http://clhs.lisp.se/Issues/iss152_w.htm http://clhs.lisp.se/Issues/iss152_w.htm "The ambiguity at issue arises for the case where there are transfers of control from the cleanup clauses of an UNWIND-PROTECT. ..." It is not clear what exit points are visible when the cleanup-forms (morally similar to C++ destructor calls) are invoked. Are the exit points that are skipped torn down all the way to the control transfer's target, or is the tear-down interleaved with the unwinding.

blt 12y ago
The real answer, as far as I can tell, is "Don't use exceptions in a program that's too important to crash."
- kazinator 12y ago
  Right, and to that end, don't use a CPU that traps illegal instructions, divisions by zero, pointers to unmapped pages and so on. Those are all forms of exceptions.

nly 12y ago

#3 should really say make public base class destructors virtual #6 sometimes you actually will want an A(A&) constructor... for example, if you have constructor overloaded with a greedy template argument. #8 objects returned by value will also bind to an rvalue reference #9 Remember in C++11 there's also new T{}; #10 isn't absolutely correct either, dynamic_cast can actually be used in some circumstances where static_cast or implicit casting could have been used. struct A { }; struct B: A { }; int main() { auto b = new B(); auto a = dynamic_cast<A*>(b); // fine }

Suncho 12y ago
#3 should really say "make polymorphic public base class destructors virtual." There are reasons why you might want to use public inheritance that have nothing to do with run-time polymorphism.
- nly 12y ago
  Right, but unless your Base or Derived class is going to pass Base*/&s to friends and/or other instances, you typically don't want your protected destructors to be virtual just so you can get safe destruction. Of course, the thing with C++ is, the longer you sit and think about it, the more reasons you can come up with a reason to do something atypical Slicing isn't even necessarily harmful either... but I'm not sure what you had in mind.
- jfoutz 12y ago
  I haven't touched C++ in years. Why would you ever make a non-virtual destructor? Isn't it totally up to the user of a class if they choose to inherit from the class? It seems like, if you're not providing a virtual destructor, you force users into delegation rather than inheritance, and that will catch some % of developers who aren't on the ball. They'll have leaks... As far as i know (old info) there's no such thing as a "final" class, so anybody can inherit even if you don't want them to.
  - gurkendoktor 12y ago

Suncho 12y ago

#1 should really say, "Avoid new and delete." new and delete are for experts. The average C++ programmer should go nowhere near them. If heap memory is used internally by a class that was written by someone who knew what they were doing, then fine. Use your std::vector. But new and delete are huge red flags in code.

TillE 12y ago
It's fine to use new as long as you're immediately sticking the result in a smart pointer. But yes, delete should be very very rare in modern C++ code.
- hendzen 12y ago
  With std::make_shared, and the addition of std::make_unique in C++14, you shouldn't ever need to use new for this purpose either.
  - Suncho 12y ago
    It depends. The trick to using new correctly is to put it in a function whose sole purpose is to immediately store new's result in the smart handle. I.E. You're writing a function that's analogous to the make_unique function. You might need to do this if you're implementing your own smart handle or writing a factory function for a class with a private constructor (make_unique will not work). Off the top of my head, I can't think of any good reasons to use delete.
    - lomnakkus 12y ago
      If your handles are pointer types, you should still use unique_ptr, you just need to give it a custom "deleter". (Of course you should probably still encapsulate this in a function like make_xxx() just to avoid the potential mistake of not supplying the correct deleter everywhere.)
toolslive 12y ago
and yet, "new" and "delete" are amongst the very first things covered in every C++ tutorial book. for example in Stroustrup's "A tour of C++", it's covered in section 1.6. without any remark about shared_ptr and friends.

mpyne 12y ago

I would actually revise #1 to be to always use a smart pointer for heap-allocated objects. C++11 is 3 years old now after all.

plicense 12y ago

Selection messed up in your website: http://imgur.com/YuowMEm http://imgur.com/YuowMEm

Arelius 12y ago

#5 > If you'll tolerate my hypocrisy for a moment, here's my suggestion: try to avoid putting the const at the beginning like that Or you could not do that, and put const in the least ambiguous place, and since you rarely have a 'type* const ' it works very well just using: const type* for pointers to const types, and type* const for const pointers to types, and entirely unambiguous.

snarfy 12y ago

I wrote C++ code for about 10 years before moving over to C#. I still like C++, but posts like this make me remember why I don't miss it. It's so baroque.