[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added a comment. In D128372#3991516 , @MyDeveloperDay wrote: > Ironically as an aside... looking back at the review because of the discource > article. > > The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years > ago was to c

[PATCH] D128372: [Clang-Tidy] Empty Check

Eugene.Zelenko added a comment. Please take a loon on https://github.com/llvm/llvm-project/issues/59487. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe

[PATCH] D128372: [Clang-Tidy] Empty Check

MyDeveloperDay added a comment. Ironically as an aside... looking back at the review because of the discource article. The whole reason why I added the [[nodiscard]] checker to clang-tidy 4 years ago was to cover exactly this use case...and many others like it where users have a function which

[PATCH] D128372: [Clang-Tidy] Empty Check

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGec3f8feddf81: [Clang-Tidy] Empty Check (authored by abrahamcd, committed by cjdb). Repository: rG LLVM Github Monorepo

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. In D128372#3965162 , @MaskRay wrote: > I just took a glance and the code looks reasonable. @MaskRay thanks for the review! I have updated the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://revie

[PATCH] D128372: [Clang-Tidy] Empty Check

denik updated this revision to Diff 479684. denik added a comment. Replaced deprecated [[@LINE-1]] Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTi

[PATCH] D128372: [Clang-Tidy] Empty Check

denik updated this revision to Diff 479677. denik marked 2 inline comments as done. denik added a comment. Format changes. Removed blank lines and fixed test types. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org

[PATCH] D128372: [Clang-Tidy] Empty Check

MaskRay added a comment. I just took a glance and the code looks reasonable. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:131 +bool HasClear = !Candidates.empty(); + +if (HasClear) { delete blank line Com

[PATCH] D128372: [Clang-Tidy] Empty Check

carlosgalvezp added a comment. In D128372#3957982 , @cjdb wrote: > @njames93 if you don't have any further concerns, I am going to merge this on > Friday afternoon, as it will have been four months since there has been a > maintainer's input. @cjdb I h

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added a comment. @njames93 if you don't have any further concerns, I am going to merge this on Friday afternoon, as it will have been four months since there has been a maintainer's input. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/

[PATCH] D128372: [Clang-Tidy] Empty Check

Eugene.Zelenko added a comment. In D128372#3883214 , @denik wrote: > @Eugene.Zelenko , could you please stamp the patch if you don't have any > other concerns? Please wait for developers approval. I look only for trivial issues. Repository: rG LLVM

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. @Eugene.Zelenko , could you please stamp the patch if you don't have any other concerns? @njames93 , if you don't have spare cycles could you please suggest other reviewers? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added a comment. @LegalizeAdulthood @njames93 is there anything actionable that's left for this patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. @LegalizeAdulthood, @njames93, friendly ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. @LegalizeAdulthood, @njames93, is there anything else we should address in this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. Any updates from the reviewers? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D128372: [Clang-Tidy] Empty Check

MaskRay added a comment. Drive-by comments: this patch may need a review from common reviewers like @njames93. I assume that @LegalizeAdulthood's comment (about the file hierarchy since there was a big reorganization few months ago) has been addressed. If you are concerned with unable to conta

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-b

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added a comment. This patch has been open for weeks without maintainer input, despite repeated requests, and @abrahamcd finishes his internship this Friday. We would very much appreciate direction on whether or not D128372 is ready to be merged. Reposit

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 454635. abrahamcd marked 2 inline comments as done. abrahamcd added a comment. Formatting fixes and synchronization of documentation. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.

[PATCH] D128372: [Clang-Tidy] Empty Check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:127 + + Warns when `empty()` is used on a range and the result is ignored. Suggests `clear()` as an alternative + if it is an existing member function. Please synchronize

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added a comment. @njames93, @LegalizeAdulthood, are there any further concerns regarding this CL? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-c

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd marked 2 inline comments as done. abrahamcd added a comment. Hi, checking in on this again, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 __

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 451212. abrahamcd added a comment. Minor formatting/naming fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 Files: clang-tools-extra/clang-tidy/bugprone/Bug

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 451209. abrahamcd marked 5 inline comments as done. abrahamcd added a comment. Adds check for the compatibility of qualifiers on the clear method and the object it is called on. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:177-178 + diag(NonMemberLoc, "ignoring the result of '%0', did you mean 'clear()'?") + << llvm::dyn_cast(NonMemberCall->getCalleeDecl()) + ->getQ

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:177-178 + diag(NonMemberLoc, "ignoring the result of '%0', did you mean 'clear()'?") + << llvm::dyn_cast(NonMemberCall->getCalleeDecl()) + -

[PATCH] D128372: [Clang-Tidy] Empty Check

njames93 added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:124 + return isa(ND) && + llvm::dyn_cast(ND)->getMinRequiredArguments() == + 0; `dyn_cast` isn't needed here a

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 450430. abrahamcd added a comment. Adds non-dependent base class test case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 Files: clang-tools-extra/clang-tidy/bu

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 450407. abrahamcd added a comment. Checks for `clear()` in base classes as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 Files: clang-tools-extra/clang-ti

[PATCH] D128372: [Clang-Tidy] Empty Check

rsmith added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:23 +#include "llvm/ADT/SmallVector.h" +#include + Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:149 + r

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd added a comment. Hi all, I just wanted to check in on this again and see if any more feedback or progress could be made. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 _

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd added a comment. Gentle ping! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.ll

[PATCH] D128372: [Clang-Tidy] Empty Check

denik accepted this revision. denik added a comment. Thanks Abraham! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb accepted this revision. cjdb added a comment. Thanks for working on this! I'll merge this on your behalf once there's maintainer approval. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 443056. abrahamcd marked 3 inline comments as done. abrahamcd added a comment. Adds argument 0 test case and refactoring based on feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://re

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:128 + << FixItHint::CreateReplacement(ReplacementRange, "clear"); +} else { + diag(MemberLoc, "ignoring the result of 'empty()'"); Let's elimi

[PATCH] D128372: [Clang-Tidy] Empty Check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:146 +CXXRecordDecl *ArgRecordDecl = Arg->getType()->getAsCXXRecordDecl(); +if (ArgRecordDecl == NULL) + return; `nullptr`. Repository:

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 443017. abrahamcd edited the summary of this revision. abrahamcd added a comment. Adds functionality for only warning on the case of unused return value of the call to `empty()` and removes using-directive. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd planned changes to this revision. abrahamcd added a comment. Stuck on determining unused return value, uploaded current state for @denik and @cjdb to take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 440742. abrahamcd added a comment. Progress Update Hit a roadblock with determining unused return value, uploading current state for further work. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ ht

[PATCH] D128372: [Clang-Tidy] Empty Check

cjdb added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:76 + +auto Methods = Arg->getType()->getAsCXXRecordDecl()->methods(); +auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) { denik wrote: > C

[PATCH] D128372: [Clang-Tidy] Empty Check

denik added a comment. Thanks for adding a check! Please check my comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:76 + +auto Methods = Arg->getType()->getAsCXXRecordDecl()->methods(); +auto Clear = llvm::find_if(Methods, [](const CXX

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 440346. abrahamcd marked 7 inline comments as done. abrahamcd added a comment. Addressed review edits and clarity feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:51-57 +auto Methods = MemberCall->getRecordDecl()->methods(); +auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) { + return F->getDeclName().getAs

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 440286. abrahamcd added a comment. Removed old test file. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTid

[PATCH] D128372: [Clang-Tidy] Empty Check

LegalizeAdulthood requested changes to this revision. LegalizeAdulthood added inline comments. This revision now requires changes to proceed. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp:1 +// RUN: %check_clang_tidy %s bugprone-standalone-

[PATCH] D128372: [Clang-Tidy] Empty Check

Eugene.Zelenko added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:55 + +auto Methods = MemberCall->getRecordDecl()->methods(); +auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) { Please don't us

[PATCH] D128372: [Clang-Tidy] Empty Check

abrahamcd updated this revision to Diff 439938. abrahamcd marked 7 inline comments as done. abrahamcd retitled this revision from "Clang-Tidy Empty Check" to "[Clang-Tidy] Empty Check". abrahamcd added a comment. Herald added a subscriber: xazax.hun. Added functionality to check if member functio

[PATCH] D128372: Clang-Tidy Empty Check

LegalizeAdulthood requested changes to this revision. LegalizeAdulthood added a comment. This revision now requires changes to proceed. Clang-Tidy tests and docs have moved to module subdirectories. Please rebase this onto `main:HEAD` and: - fold your changes into the appropriate subdirs, stripp

[PATCH] D128372: Clang-Tidy Empty Check

Eugene.Zelenko added a comment. Tests locations were changed recently too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list cfe-comm

[PATCH] D128372: Clang-Tidy Empty Check

Eugene.Zelenko added a comment. Please add documentation for check. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:121 +- New :doc:`bugprone-standalone-empty + ` check. + Path to documentation was changed recently. Comment at: clang-too

[PATCH] D128372: Clang-Tidy Empty Check

cjdb added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:61 + "ignoring the result of 'empty()', did you mean 'clear()'? "); + Builder << FixItHint::CreateReplacement(ReplacementRange, "clear"); +} -

[PATCH] D128372: Clang-Tidy Empty Check

cjdb added a comment. In D128372#3602881 , @tschuett wrote: > Just for reference: > https://reviews.llvm.org/D128267 Ack. I still think this CL is useful, given that not every library will have `[[nodiscard]]`, and because it can suggest appropriate alt

[PATCH] D128372: Clang-Tidy Empty Check

tschuett added a comment. Just for reference: https://reviews.llvm.org/D128267 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128372/new/ https://reviews.llvm.org/D128372 ___ cfe-commits mailing list cfe-

[PATCH] D128372: Clang-Tidy Empty Check

cjdb added a comment. A great start, thanks! Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:51-57 +auto Methods = MemberCall->getRecordDecl()->methods(); +auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) { + return F->getDe

[PATCH] D128372: Clang-Tidy Empty Check

abrahamcd created this revision. Herald added subscribers: carlosgalvezp, abrachet, phosek, mgorny. Herald added a project: All. abrahamcd requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefan1. Herald added a project: clang-tools-extr