[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

gbencze added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2582-2584 +int64_t BitfieldSize = Field->getBitWidthValue(Context); +if (BitfieldSize > FieldSizeInBits) + return llvm::None; steakhal wrote: > Why do you return `None` here?

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

gbencze added a comment. Ping @rsmith CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89649/new/ https://reviews.llvm.org/D89649 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze added a comment. Thanks for the review! I'll push this as soon as the updated version of D89649 is also accepted. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89651/new/ https://reviews.llvm.org/D89651

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

gbencze added a comment. Pinging @rsmith - could you please check the updates to this patch? Thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89649/new/ https://reviews.llvm.org/D89649 ___ cfe-commits mailing list

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze updated this revision to Diff 306679. gbencze added a comment. Herald added a subscriber: jfb. Address comments: - add new test case for `_Atomic` (with a FIXME comment) - fix string literal formatting - update docs CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89651/new/

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze marked an inline comment as done. gbencze added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:45 + + for (unsigned int i = 0; i < 2; ++i) { +const Expr *ArgExpr = CE->getArg(i); aaron.ballman

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze updated this revision to Diff 303643. gbencze added a comment. - Added some new test cases - Fixed assertion in templates - Changed loop variable name from `i` to `ArgIndex` - Changed wording of warnings - Changed CHECK-MESSAGES to be in a single line: turns out that only the first line

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

gbencze added inline comments. Comment at: clang/test/SemaCXX/has_unique_object_reps_no_unique_addr.cpp:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -verify -std=c++2a %s +// expected-no-diagnostics Just to be sure: is the specifying the

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

gbencze updated this revision to Diff 303082. gbencze added a comment. Herald added a subscriber: mgrang. Sorry for the slow update on this. I fixed the behavior when reusing tail padding as mentioned by @rsmith and took a shot at unifying the code paths for base classes and fields. Let me know

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze added a comment. In D89651#2338266 , @njames93 wrote: > Should point out there is already a check for cert-oop57-cpp, added in D72488 > . Do these play nice with each other or > should they perhaps be merged or

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

gbencze added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2580-2581 if (!isStructEmpty(Base.getType())) { llvm::Optional Size = structHasUniqueObjectRepresentations( Context, Base.getType()->castAs()->getDecl()); if (!Size)

[PATCH] D89651: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze created this revision. gbencze added reviewers: aaron.ballman, JonasToth, Charusso. gbencze added a project: clang-tools-extra. Herald added subscribers: cfe-commits, xazax.hun, mgorny. Herald added a project: clang. gbencze requested review of this revision. The check warns on suspicious

[PATCH] D89649: Fix __has_unique_object_representations with no_unique_address

gbencze created this revision. gbencze added reviewers: rsmith, aaron.ballman. Herald added a project: clang. Herald added a subscriber: cfe-commits. gbencze requested review of this revision. Fix incorrect behavior of __has_unique_object_representations when using the no_unique_address

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze added a comment. ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71973/new/ https://reviews.llvm.org/D71973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze added a comment. Another option that came to my mind is using a BitVector to (recursively) flag bits that are occupied by the fields. This solution would be slightly slower and uses more memory but is probably a lot easier to understand, maintain and more robust than the currently

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:82-83 + +static bool hasPadding(const ASTContext , const RecordDecl *RD, + uint64_t ComparedBits) { + assert(RD &&

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze updated this revision to Diff 237529. gbencze added a comment. Address (most of the) comments by @aaron.ballman - remove top-level `const` on locals - move declaration into `if` - pass `TagDecl` to diag - added test for `operator void *` - fixed `[[no_unique_address]]` - remove

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze added a comment. ping @aaron.ballman - any thoughts on the patch? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71973/new/ https://reviews.llvm.org/D71973 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze marked 4 inline comments as done. gbencze added a comment. Thanks for all the feedback @JonasToth :) Comment at: clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memory-comparison.rst:10 +This check corresponds to the CERT C Coding Standard rule

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze updated this revision to Diff 235965. gbencze added a comment. Punctuation in comment CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71973/new/ https://reviews.llvm.org/D71973 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze updated this revision to Diff 235915. gbencze added a comment. Tests: Split C/C++ tests and add 32/64bit specific test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71973/new/ https://reviews.llvm.org/D71973 Files:

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze marked 18 inline comments as done. gbencze added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/bugprone-suspicious-memory-comparison.cpp:1 +// RUN: %check_clang_tidy %s bugprone-suspicious-memory-comparison %t + JonasToth

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze updated this revision to Diff 235900. gbencze added a comment. Coding guide and better diagnostic message for padding comparison CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71973/new/ https://reviews.llvm.org/D71973 Files:

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze updated this revision to Diff 235564. gbencze added a comment. ReleaseNotes: move alias after new checks CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71973/new/ https://reviews.llvm.org/D71973 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze marked 5 inline comments as done. gbencze added inline comments. Comment at: clang-tools-extra/docs/ReleaseNotes.rst:103 + +- New alias :doc:`cert-exp42-c + ` to Eugene.Zelenko wrote: > Please move into aliases section (in alphabetical order) I'm not

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze updated this revision to Diff 235526. gbencze added a comment. Update style based on comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71973/new/ https://reviews.llvm.org/D71973 Files: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

[PATCH] D71973: [clang-tidy] Add bugprone-suspicious-memory-comparison check

gbencze created this revision. gbencze added reviewers: aaron.ballman, alexfh, JonasToth, Charusso. gbencze added projects: clang-tools-extra, clang. Herald added subscribers: cfe-commits, xazax.hun, whisperity, mgorny. The check warns on suspicious calls to memcmp. It currently checks for

[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

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

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

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

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

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

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