Prazek added inline comments. ================ 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: > > 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. > But this means that `unless(isPublic())` should work, IIUC. Oh I see. Yea I remember right now what I did: has(ignoringImpCasts(cxxConstructExpr( hasDeclaration(decl(isPublic())))))))
This ofc not gonna work, but unless(has(ignoringImpCasts(cxxConstructExpr( hasDeclaration(decl(unless(isPublic()))))))) should be totally fine https://reviews.llvm.org/D23343 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits