[PATCH] D55044: [clang-tidy] check for Abseil make_unique

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Dec 14 06:17:46 PST 2018


hokein added a comment.

>> In D55044#1329604 <https://reviews.llvm.org/D55044#1329604>, @hokein wrote:
>>  In fact, the existing modernize-make-unique can be configured to support absl::make_unique, you'd just need to configure the check option MakeSmartPtrFunction to absl::make_unique (this is what we do inside google).
> 
> See https://reviews.llvm.org/D52670#change-eVDYJhWSHWeW (the getModuleOptions() part) on how to do that

Thanks! Yes, it is trivial to create an alias check, but if we want to support `WrapUnique`, I don't think this is a right way to go.

In D55044#1330102 <https://reviews.llvm.org/D55044#1330102>, @axzhang wrote:

> In D55044#1329604 <https://reviews.llvm.org/D55044#1329604>, @hokein wrote:
>
> > In fact, the existing `modernize-make-unique` can be configured to support `absl::make_unique`, you'd just need to configure the check option `MakeSmartPtrFunction` to `absl::make_unique` (this is what we do inside google).
> >
> > The biggest missing part of the `modernize-make-unique` is `absl::WrapUnique`, I think we should extend `MakeSmartPtrCheck` class (maybe add hooks) to support it.
>
>
> What is the best way to extend `MakeSmartPtrCheck`? The behavior I want to achieve is that `absl::WrapUnique` is suggested when brace initialization is used, but `absl::make_unique` is used in all other cases.


I think we'd need to add more interfaces to `MakeSmartPtrCheck` allowing subclasses to inject their logic to handle different initialization styles of `new` expression.

- `MakeSmartPtrCheck::replaceNew` is the interesting place
- we may split `MakeSmartPtrCheck::replaceNew` implementation into different methods (e.g. `handleCallInit`, `handleListInit`) which are corresponding to handle different `new` cases
- from my experience,  handling brace initialization is tricky, there are lots of corner cases, you can see the comments in `MakeSmartPtrCheck::replaceNew`


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55044/new/

https://reviews.llvm.org/D55044





More information about the cfe-commits mailing list