sbarzowski added inline comments.
================
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:41
@@ +40,3 @@
+ // (and destructed) as in push_back case.
+ auto isCtorOfSmartPtr = hasDeclaration(cxxConstructorDecl(
+ ofClass(hasAnyName("std::shared_ptr", "std::unique_ptr", "std::auto_ptr",
----------------
vsk wrote:
> I agree that blacklisting some smart pointers is not a complete solution, and
> that we shouldn't introduce a check which emits false positives.
>
> ISTM it's **only** safe to perform the "push(T(...)) -> emplace(...)" change
> if: it's safe to assume that if "emplace(...)" does not successfully call
> "T(...)", it's OK for the program to fail/leak/crash. Do we get to make this
> assumption ever? Perhaps just in no-exceptions mode?
hmmm... Maybe it's not that bad.
Let's look closely at the difference in behaviour.
with push_back it works like:
1) Call the constructor
2) Allocate the memory
3) Call the copy constructor
And emplace_back is:
1) Allocate the memory
2) Call the constructor
Each of these steps in both scenarios may fail.
If we call the constructor and it fails, then it's alright. We have the same
behaviour in both cases - if it can fail it should do its cleanup. (And failure
of copy constructor obviously doesn't bother us).
So the scenario we have to worry about in only when memory allocation fails.
Then with push_back, we'll call constructor and then destructor, while
emplace_back doesn't call constructor at all.
So the real question is: "Is it safe to throw some exception and not to call
the constructor at all". And I see only one real-world case it isn't - taking
the ownership of the object.
http://reviews.llvm.org/D20964
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
