[PATCH] D23343: [clang-tidy] modernize-make-{smart_ptr} private ctor bugfix

[Piotr Padlewski via cfe-commits]

Prazek marked 8 inline comments as done.

================
Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:35
@@ +34,3 @@
+  auto CanCallCtor = unless(has(ignoringImpCasts(cxxConstructExpr(
+      hasDeclaration(decl(anyOf(isPrivate(), isProtected())))))));
+
----------------
alexfh wrote:
> Prazek wrote:
> > aaron.ballman wrote:
> > > Prazek wrote:
> > > > aaron.ballman wrote:
> > > > > Perhaps: `unless(isPublic())` instead of `anyOf(isPrivate(), isProtected())`?
> > > > POD types doesn't have public constructors so it will fail :)
> > > Don't they have an implicit one for the purposes of a CXXConstructExpr? Looking at an AST dump for:
> > > ```
> > > struct S {
> > >   int i;
> > > };
> > > ```
> > > yields:
> > > ```
> > > |-CXXRecordDecl 0x26d74ae5348 <line:25:1, line:27:1> line:25:8 referenced struct S definition
> > > | |-CXXRecordDecl 0x26d74ae5460 <col:1, col:8> col:8 implicit struct S
> > > | |-FieldDecl 0x26d74ae7880 <line:26:3, col:7> col:7 i 'int'
> > > | |-CXXConstructorDecl 0x26d74ae87b8 <line:25:8> col:8 implicit used S 'void (void) noexcept' inline
> > > | | `-CompoundStmt 0x26d74ae3850 <col:8>
> > > | |-CXXConstructorDecl 0x26d74ae34a8 <col:8> col:8 implicit constexpr S 'void (const struct S &)' inline noexcept-unevaluated 0x26d74ae34a8
> > > | | `-ParmVarDecl 0x26d74ae35e0 <col:8> col:8 'const struct S &'
> > > | `-CXXConstructorDecl 0x26d74ae3678 <col:8> col:8 implicit constexpr S 'void (struct S &&)' inline noexcept-unevaluated 0x26d74ae3678
> > > |   `-ParmVarDecl 0x26d74ae37b0 <col:8> col:8 'struct S &&'
> > > ```
> > what about std::shared_ptr<int> x = std::shared_ptr(new int); ?
> > If I recall correctly, it didn't work on this test.
> There's no `CXXConstructExpr` for primitive types:
> ```
> $ cat /tmp/qq.cc 
> #include <memory>
> 
> struct S { S(); };
> void ffff() {
>   std::shared_ptr<int> p1 = std::shared_ptr<int>(new int);
>   std::shared_ptr<S> p2 = std::shared_ptr<S>(new S);
> }
> $ clang-check -ast-dump -ast-dump-filter=ffff /tmp/qq.cc -- -std=c++11
> Dumping ffff:
> FunctionDecl 0x7f48270fb6e0 </tmp/qq.cc:4:1, line:7:1> line:4:6 ffff 'void (void)'
> `-CompoundStmt 0x7f48271582a8 <col:13, line:7:1>
>   |-DeclStmt 0x7f4827131000 <line:5:3, col:58>
>   | `-VarDecl 0x7f48270fb9c8 <col:3, col:57> col:24 p1 'std::shared_ptr<int>':'class std::shared_ptr<int>' cinit
>   |   `-ExprWithCleanups 0x7f4827130fe8 <col:29, col:57> 'std::shared_ptr<int>':'class std::shared_ptr<int>'
>   |     `-CXXConstructExpr 0x7f4827130fb0 <col:29, col:57> 'std::shared_ptr<int>':'class std::shared_ptr<int>' 'void (class std::shared_ptr<int> &&) noexcept' elidable
>   |       `-MaterializeTemporaryExpr 0x7f4827130f98 <col:29, col:57> 'class std::shared_ptr<int>' xvalue
>   |         `-CXXFunctionalCastExpr 0x7f48271303c8 <col:29, col:57> 'std::shared_ptr<int>':'class std::shared_ptr<int>' functional cast to std::shared_ptr<int> <ConstructorConversion>
>   |           `-CXXBindTemporaryExpr 0x7f48271303a8 <col:29, col:57> 'std::shared_ptr<int>':'class std::shared_ptr<int>' (CXXTemporary 0x7f48271303a0)
>   |             `-CXXConstructExpr 0x7f4827130338 <col:29, col:57> 'std::shared_ptr<int>':'class std::shared_ptr<int>' 'void (int *)'
>   |               `-CXXNewExpr 0x7f48270fbb58 <col:50, col:54> 'int *' Function 0x7f4826a1c000 'operator new' 'void *(std::size_t)'
>   `-DeclStmt 0x7f4827158290 <line:6:3, col:52>
>     `-VarDecl 0x7f4827131238 <col:3, col:51> col:22 p2 'std::shared_ptr<S>':'class std::shared_ptr<struct S>' cinit
>       `-ExprWithCleanups 0x7f4827158278 <col:27, col:51> 'std::shared_ptr<S>':'class std::shared_ptr<struct S>'
>         `-CXXConstructExpr 0x7f4827158240 <col:27, col:51> 'std::shared_ptr<S>':'class std::shared_ptr<struct S>' 'void (class std::shared_ptr<struct S> &&) noexcept' elidable
>           `-MaterializeTemporaryExpr 0x7f4827158228 <col:27, col:51> 'class std::shared_ptr<struct S>' xvalue
>             `-CXXFunctionalCastExpr 0x7f4827157658 <col:27, col:51> 'std::shared_ptr<S>':'class std::shared_ptr<struct S>' functional cast to std::shared_ptr<S> <ConstructorConversion>
>               `-CXXBindTemporaryExpr 0x7f4827157638 <col:27, col:51> 'std::shared_ptr<S>':'class std::shared_ptr<struct S>' (CXXTemporary 0x7f4827157630)
>                 `-CXXConstructExpr 0x7f48271575c8 <col:27, col:51> 'std::shared_ptr<S>':'class std::shared_ptr<struct S>' 'void (struct S *)'
>                   `-CXXNewExpr 0x7f4827131838 <col:46, col:50> 'struct S *' Function 0x7f4826a1c000 'operator new' 'void *(std::size_t)'
>                     `-CXXConstructExpr 0x7f4827131808 <col:50> 'struct S' 'void (void)'
> ```
exactly.


https://reviews.llvm.org/D23343