[PATCH] D54704: [clang-tidy] Don't generate incorrect fixes for class constructed from list-initialized arguments

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 20 06:40:34 PST 2018


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

> Currently the smart_ptr check (modernize-make-unique) generates the fixes that cannot compile for cases like below -- because brace list can not be deduced in make_unique.
> 
>   class Bar { int a, b; };
>   class Foo { Foo(Bar); };
>   auto foo = std::unique_ptr<Foo>(new Foo({1, 2}));

This code isn't legal in the first place. ;-)

Aside from some small nits, this LGTM.



================
Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:287
+  //   Foo(Bar{1, 2}) => true
+  //   Foo(1) => false
+  auto HasListIntializedArgument = [](const CXXConstructExpr *CE) {
----------------
I assume this is expected to be false? `Foo{1}`


================
Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:294
+      // std::initializer_list.
+      auto IsStdInitListInitConstructExpr = [](const Expr *E) {
+        assert(E);
----------------
No need for this lambda (then again, it was in the original code).


================
Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:295
+      auto IsStdInitListInitConstructExpr = [](const Expr *E) {
+        assert(E);
+        if (const auto *ImplicitCE = dyn_cast<CXXConstructExpr>(E)) {
----------------
No need to assert this; `dyn_cast<>` does it for you.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D54704





More information about the cfe-commits mailing list