[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

This revision was automatically updated to reflect the committed changes. Closed by commit rG8a8f77c1b849: [clang-tidy] Implement CppCoreGuideline F.54 (authored by ccotter, committed by carlosgalvezp). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp accepted this revision. carlosgalvezp added a comment. Thanks for updating the comment! Since I marked it as approved last time, I will land this in the next couple of days if no more comments come up. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revi

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter marked 2 inline comments as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:81 + Lambda->getCaptureDefaultLoc(), + "lambdas that capture 'this' should not spec

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:81 + Lambda->getCaptureDefaultLoc(), + "lambdas that capture 'this' should not specify a capture default"); + -

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter added a comment. Gentle poke for any last reviews? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 ___ cfe-commits mailing list cfe-commits@lists.llvm.o

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 492981. ccotter added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWh

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM, thanks for the contribution! Let's give a couple days for the others to have a last look Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 491171. ccotter marked an inline comment as done. ccotter added a comment. - Use nested namespace in header too; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:19 + +namespace clang { +namespace tidy { ccotter wrote: > carlosgalvezp wrote: > > We recently switched to using C++17

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.h:41-43 +} // namespace cppcoreguidelines +} // namespace tidy +} // namespace clang Forgot to comment but you'll need it he

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter marked an inline comment as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:19 + +namespace clang { +namespace tidy { carlosgalvezp wrote: > We recently switche

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 491165. ccotter added a comment. - Use nested namespace Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/Avoid

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp requested changes to this revision. carlosgalvezp added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:19 + +namespace clang { +namespace tidy

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 490701. ccotter added a comment. - cleanup comments, docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/Avo

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureDefaultWhenCapturingThisCheck.cpp:1 +//===--- AvoidCaptureDefaultWhenCapturingThisCheck.cpp - clang-tidy +//-===// Ditto. Comm

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter marked 2 inline comments as done. ccotter added a comment. Would someone with merge access mind committing this? I double checked and this diff can be applied on the latest upstream/main. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp added a comment. In D141133#4051068 , @ccotter wrote: > My latest update to the diff was the result of a rebase, then addressing the > final comments. What I've been doing is something like > > git pull upstream main --rebase > # make m

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter added a comment. My latest update to the diff was the result of a rebase, then addressing the final comments. What I've been doing is something like git pull upstream main --rebase # make more edits git commit -am arc diff --update D141133 upstream/main I'm not sure if this

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter marked 2 inline comments as done. ccotter added a comment. Thanks for pointing that out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 ___ cfe-commits

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 488943. ccotter added a comment. - Finish stengthening CHECK-FIXES Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguide

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

njames93 accepted this revision. njames93 added a comment. This revision is now accepted and ready to land. Just 2 more `CHECK-FIXES` lines need updating. FWIW the reason its good to be explicit is because FileCheck will start searching from the line where the previous CHECK-FIXES had and stop wh

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 488085. ccotter added a comment. - Rename test to match check name Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguide

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 488084. ccotter added a comment. - clang-format - Show whether 'this' is implicitly captured - fix docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clan

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

njames93 requested changes to this revision. njames93 added a comment. This revision now requires changes to proceed. Can you run this through clang format to make sure the pre merge is happy and address those nits Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/Av

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 487195. ccotter added a comment. - tidy up docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureD

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp accepted this revision. carlosgalvezp added a comment. LGTM! Minor nit. Comment at: clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-default-when-capturing-this.rst:4 +cppcoreguidelines-avoid-capture-default-when-capturing-this +==

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter marked an inline comment as done. ccotter added a comment. Thanks! I opted for 'avoid-capture-default-when-capturing-this' and updated the name and docs etc in the most recent diff. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 487193. ccotter added a comment. - rename to avoid-capture-deafult-when-capturing-this Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/cla

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 487189. ccotter added a comment. - Use const& Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThi

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp:32 + if (Capture.getCaptureKind() == LCK_ByRef) { +SourceManager &SourceMgr = Context.getSourceManager(); +SourceLoca

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp accepted this revision. carlosgalvezp added a comment. This revision is now accepted and ready to land. LGTM! Please give a few days for other reviewers to comment. If you are not 100% happy with the check name I'd suggest to change it before merging, it will be much harder to chan

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter marked 2 inline comments as done. ccotter added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp:35 +Capture.getLocation(), SourceMgr, Context.getLangOpts(), tok::amp); +llvm::errs

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 487130. ccotter added a comment. - rm trace; tidy docs Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidC

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp added inline comments. Comment at: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp:35 +Capture.getLocation(), SourceMgr, Context.getLangOpts(), tok::amp); +llvm::errs() << "FOR REF capture loc= " +

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9 +auto explicit_this_capture = [=, this]() { }; +// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this s

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9 +auto explicit_this_capture = [=, this]() { }; +// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture t

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter marked an inline comment as done and an inline comment as not done. ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9 +auto explicit_this_capture = [=, this]() { }; +

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter marked an inline comment as done. ccotter added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9 +auto explicit_this_capture = [=, this]() { }; +// CHECK-MESSAGES: :[[@LINE-1]]

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 487084. ccotter added a comment. - typo Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithC

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 487082. ccotter added a comment. oops, bad 'arc diff' again Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra/clang-tidy/cppcoreguidelines/A

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 487081. ccotter added a comment. - Use "capture default" consistently and update diagnostic message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-to

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:1 +// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-this-with-capture-default %t + -

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

carlosgalvezp added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9 +auto explicit_this_capture = [=, this]() { }; +// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture t

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter updated this revision to Diff 486873. ccotter marked an inline comment as done. ccotter added a comment. - docs feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141133/new/ https://reviews.llvm.org/D141133 Files: clang-tools-extra

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:115 + Warns when lambda specify a capture default and capture ``this``. The check also + offers FixIts. + `fix-it`. Comment at: clang-tools-extra/docs

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

ccotter created this revision. Herald added subscribers: carlosgalvezp, shchenz, kbarton, xazax.hun, nemanjai. Herald added a reviewer: njames93. Herald added a project: All. ccotter requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits