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

shafik closed this revision. shafik added a comment. Herald added a subscriber: StephenFan. Herald added a project: All. Closing as it look like this is now `modernize-make-unique` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044

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

hokein accepted this revision. hokein added a comment. This revision is now accepted and ready to land. In D55044#1589496 , @alexfh wrote: > Seems good now. Haojian, do you have any concerns? sorry for the looong delay, I totally miss this thread. looks

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

alexfh added a comment. Seems good now. Haojian, do you have any concerns? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

axzhang added a comment. Pinging for visibility. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

axzhang updated this revision to Diff 195791. axzhang added a comment. Simplify tests and fix issues with Options. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 Files: clang-tidy/abseil/AbseilTidyModule.cpp

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

axzhang marked 3 inline comments as done. axzhang added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreListInit(Options.get("IgnoreListInit", false)) {}

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

hintonda added inline comments. Comment at: clang-tidy/abseil/AbseilTidyModule.cpp:77 +Opts["abseil-make-unique.MakeSmartPtrFunction"] = "absl::make_unique"; +Opts["abseil-make-unique.IgnoreListInit"] = true; +return Options; This is defined as

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

hintonda added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreListInit(Options.get("IgnoreListInit", false)) {} hintonda wrote: > axzhang wrote: > >

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

hintonda added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreListInit(Options.get("IgnoreListInit", false)) {} axzhang wrote: > hintonda wrote: > >

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

axzhang marked an inline comment as done. axzhang added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreListInit(Options.get("IgnoreListInit", false)) {}

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

hintonda added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:69 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + IgnoreListInit(Options.get("IgnoreListInit", false)) {} You’re setting it false here.

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

axzhang marked an inline comment as done. axzhang added a comment. I'm having some issues with the AbseilTidyModule options. I am storing IgnoreListInit as true for the abseil-make-unique check, but when I run the check it shows IgnoreListInit as being false. Any ideas why this is happening?

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

hokein added a comment. Thanks, looks better now. I saw there are still a few undone comments, please mark them done when you address them. Comment at: docs/clang-tidy/checks/abseil-make-unique.rst:19 + +per the Abseil tips and guidelines at https://abseil.io/tips/126.

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

axzhang added a comment. If no more changes need to be made, is this okay to be merged in? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

axzhang updated this revision to Diff 194435. axzhang marked an inline comment as not done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 Files: clang-tidy/abseil/AbseilTidyModule.cpp clang-tidy/modernize/MakeSmartPtrCheck.cpp

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

lebedev.ri resigned from this revision. lebedev.ri added a comment. Hmm, i suppose this looks good to me now. It would be good to early-exit, but i'm not sure how important that is. Please see D52771 /

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

axzhang marked 2 inline comments as done and an inline comment as not done. axzhang added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:94 equalsBoundNode(PointerType),

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

axzhang marked an inline comment as done. axzhang added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:94 equalsBoundNode(PointerType), CanCallCtor)

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

lebedev.ri added a comment. It is best to error-out early. Could you please try this instead: Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:39 } } // namespace ``` AST_MATCHER_P(CXXNewExpr, hasInitializationStyle,

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

axzhang updated this revision to Diff 192394. axzhang added a comment. Add documentation about the added option for `modernize-make-unique`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 Files: clang-tidy/abseil/AbseilTidyModule.cpp

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

lebedev.ri added a comment. Starting to look good i think. Comment at: docs/ReleaseNotes.rst:88 +- New alias :doc:`abseil-make-unique + ` to :doc:`modernize-make-unique Also add a new entry about the new option for `modernize-make-unique`. CHANGES SINCE

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

axzhang updated this revision to Diff 190775. axzhang added a comment. Make fixes to the `UseLegacyFunction` option, renaming to `IgnoreListInit`. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 Files: clang-tidy/abseil/AbseilTidyModule.cpp

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

axzhang updated this revision to Diff 190676. axzhang added a comment. Updated diff with full context. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 Files: clang-tidy/abseil/AbseilTidyModule.cpp clang-tidy/modernize/MakeSmartPtrCheck.cpp

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

MyDeveloperDay added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:163 + // Conservatively disable for list initializations + if (UseLegacyFunction && New->getInitializationStyle() == CXXNewExpr::ListInit) { +return; elide the

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

lebedev.ri added inline comments. Comment at: clang-tidy/modernize/MakeSmartPtrCheck.cpp:55 + IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)), + UseLegacyFunction(Options.getLocalOrGlobal("UseLegacyFunction", false)) {} I'm not sure it

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

lebedev.ri added a comment. Please always upload all patches with full context (`=U9`) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

axzhang updated this revision to Diff 190559. axzhang added a comment. Apologies for the extended hiatus. I have changed the implementation to an alias to modernize-make-unique, and have added an extra option to modernize-make-unique that ignores C++11 features such as list initializations, to

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

hokein added a comment. >> In 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

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

axzhang added a comment. In 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`

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

lebedev.ri added a comment. In 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

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

hokein added a comment. 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

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

lebedev.ri requested changes to this revision. lebedev.ri added inline comments. This revision now requires changes to proceed. Comment at: clang-tidy/abseil/MakeUniqueCheck.h:29 +/// \endcode +class MakeUniqueCheck : public modernize::MakeSmartPtrCheck { +public:

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

EricWF marked an inline comment as done. EricWF added a comment. I'm not sure how you're building this, but it doesn't link for me. We need to add a dependency on `clangTidyModernizeModule` since we're deriving from its `MakeSmartPtrCheck`. Comment at:

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

Eugene.Zelenko added inline comments. Comment at: clang-tidy/abseil/MakeUniqueCheck.cpp:28 + recordType(hasDeclaration(classTemplateSpecializationDecl( + hasName("::std::unique_ptr"), templateArgumentCountIs(2), + hasTemplateArgument(

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

EricWF added inline comments. Comment at: clang-tidy/abseil/MakeUniqueCheck.cpp:28 + recordType(hasDeclaration(classTemplateSpecializationDecl( + hasName("::std::unique_ptr"), templateArgumentCountIs(2), + hasTemplateArgument( Does this

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

Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:93 + + Checks for instances of initializing a `unique_ptr` with a direct call to + `new` and suggests using `absl::make_unique` instead. Please use double `, not single ones to hightlight

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

axzhang updated this revision to Diff 177972. axzhang added a comment. Per the suggestions of reviewers, change the check so that it uses modernize-make-smart-pointer instead. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044 Files:

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

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Marking both of these as "changes requested" to highlight the similarity of issues in them. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION

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

JonasToth added a comment. In D55044#1312438 , @lebedev.ri wrote: > Thanks for working on this. > Semi-duplicate of D50852 Please see > discussion there. > It should not be an abseil-specific check, the prefix

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

Eugene.Zelenko added inline comments. Comment at: docs/ReleaseNotes.rst:70 +- New :doc:`abseil-make-unique + ` check. Please use alphabetical order for new checks. Comment at: docs/clang-tidy/checks/abseil-make-unique.rst:6 + +Replaces