[PATCH] D20964: [clang-tidy] Add modernize-use-emplace

[Stanisław Barzowski via cfe-commits]

sbarzowski added inline comments.

================
Comment at: clang-tidy/modernize/UseEmplaceCheck.cpp:40
@@ +39,3 @@
+  // passed pointer because smart pointer won't be constructed
+  // (and destructed) as in push_back case.
+  auto isCtorOfSmartPtr = hasDeclaration(cxxConstructorDecl(
----------------
Prazek wrote:
> sbarzowski wrote:
> > > Look at tests - the same thing happens when
> > 
> > Yeah. I meant looking for `new` in addition to blacklist.
> > 
> > > Not many custom classes take pointer in constructor. 
> > 
> > Obviously depends on codebase, but IMO it's quite common. However usually this classes aren't copy-constructible (or at least shouldn't be) anyway, making it impossible to use push_back, so maybe it's not such a big deal.
> > 
> > > If I will look for const pointers, then I will not be able to pass "abc" into vector<string>.
> > 
> > I wrote explicitly about only **non**-const pointers.
> It doesn't matter if it is const or not. Disabling any of them would disable some cases where it would be good.
> I have to run it on llvm code base and see what happens
Of course there would be some false negatives. It's close to impossible to know for sure if it's safe, so we need some sort of heuristic. My concern is that the current "blacklist smart pointers" solution may lead to a lot of false positives. And false positives here are quite nasty - very subtle bugs, that are unlikely to be caught by the tests or quick code review. So I'm more comfortable with false negatives.


http://reviews.llvm.org/D20964