[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

This revision was automatically updated to reflect the committed changes. Closed by commit rGbbc9f6c2ef07: [clang-tidy] Add cert-oop58-cpp check The check warns when (a member of) the… (authored by gbencze). Changed prior to commit: https://reviews.llvm.org/D70052?vs=229925=233966#toc

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

aaron.ballman accepted this revision. aaron.ballman added a comment. In D70052#1750664 , @gbencze wrote: > In D70052#1749730 , @JonasToth wrote: > > > There is a `ExprMutAnalyzer` that is able to find mutation of

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

Charusso accepted this revision. Charusso added a comment. This revision is now accepted and ready to land. I think it is fine, but please let us wait with the final thoughts from @aaron.ballman. Comment at: clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp:21 +static

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze marked 5 inline comments as done. gbencze added a comment. Mark comments as Done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70052/new/ https://reviews.llvm.org/D70052 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze added a comment. In D70052#1749730 , @JonasToth wrote: > There is a `ExprMutAnalyzer` that is able to find mutation of expressions in > general (even though it is kinda experimental still). Maybe that should be > utilized somehow? I think the

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze updated this revision to Diff 229925. gbencze added a comment. Formatting changes (curly braces, newline at EOF) Remove incorrect flag from test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70052/new/ https://reviews.llvm.org/D70052 Files:

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

aaron.ballman added inline comments. Comment at: clang-tools-extra/clang-tidy/cert/MutatingCopyCheck.cpp:73-79 + if (const auto *MemberCall = + Result.Nodes.getNodeAs(MutatingCallName)) { +diag(MemberCall->getBeginLoc(), "call mutates copied object"); + } else if

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

JonasToth added a comment. There is a `ExprMutAnalyzer` that is able to find mutation of expressions in general (even though it is kinda experimental still). Maybe that should be utilized somehow? I think the current implementation does not cover when the address is taken and mutation happens

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze updated this revision to Diff 229720. gbencze added a comment. Moved check to CERT module. Added warnings for calling mutating member functions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70052/new/ https://reviews.llvm.org/D70052 Files:

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

aaron.ballman added a comment. In D70052#1741246 , @gbencze wrote: > In D70052#1740235 , @Eugene.Zelenko > wrote: > > > If this is CERT rule, why check is not placed in relevant module? > > > To be honest I was

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

Eugene.Zelenko added a comment. In D70052#1741246 , @gbencze wrote: > In D70052#1740235 , @Eugene.Zelenko > wrote: > > > If this is CERT rule, why check is not placed in relevant module? > > > To be honest I was

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze updated this revision to Diff 228758. gbencze added a comment. Update documentation and release notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70052/new/ https://reviews.llvm.org/D70052 Files: clang-tools-extra/clang-tidy/misc/CMakeLists.txt

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze added a comment. In D70052#1740235 , @Eugene.Zelenko wrote: > If this is CERT rule, why check is not placed in relevant module? To be honest I was hoping for some feedback on this as I wasn't sure what the best place for this check would be.

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

Eugene.Zelenko added a comment. If this is CERT rule, why check is not placed in relevant module? Comment at: clang-tools-extra/docs/ReleaseNotes.rst:126 + + Finds copy operations that mutate the source object. + Please synchronize with first statement in

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze added a comment. In D70052#1740142 , @mgehre wrote: > Did you consider to warn on copy constructors/assignment operators take a > their arguments by non-`const` reference, and suggest the user to turn them > into const references? This seems

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

mgehre added a comment. Did you consider to warn on copy constructors/assignment operators take a their arguments by non-`const` reference, and suggest the user to turn them into const references? This seems like a more useful (and easier) check to me. The link above contains `Ideally, the copy

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze added a comment. In D70052#1740075 , @mgehre wrote: > Have you run your check over the LLVM/clang source base and seen true > positives/false positives? I tested it on the Xerces and Bitcoin codebases and did not get any warnings. Repository:

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

mgehre added a comment. Have you run your check over the LLVM/clang source base and seen true positives/false positives? Comment at: clang-tools-extra/docs/clang-tidy/checks/misc-mutating-copy.rst:6 + +Finds assignments to and to direct or indirect members of the copied

[PATCH] D70052: [clang-tidy] Add misc-mutating-copy check

gbencze created this revision. gbencze added reviewers: aaron.ballman, alexfh, JonasToth, Charusso. gbencze added a project: clang-tools-extra. Herald added subscribers: cfe-commits, mgehre, xazax.hun, mgorny. Herald added a project: clang. The check warns when (a member of) the copied object is