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

[Gabor Bencze via Phabricator via cfe-commits]

gbencze added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp:82-83
+
+static bool hasPadding(const ASTContext &Ctx, const RecordDecl *RD,
+                       uint64_t ComparedBits) {
+  assert(RD && RD->isCompleteDefinition() &&
----------------
aaron.ballman wrote:
> I am surprised that we have to do this much manual work, I would have expected this to be something that `RecordLayout` would handle for us. I am a bit worried about this manual work being split in a few places because we're likely to get it wrong in at least one of them. For instance, this doesn't seem to be taking into account things like `[[no_unique_address]]` or alignment when considering the layout of fields.
> 
> I sort of wonder if we should try using the `RecordLayout` class to do this work instead, even if that requires larger changes.
Thanks, I didn't realize the [[no_unique_address]] attribute existed but I fixed it (in this check for now) and also added some tests to cover it as well as alignas. 

If you think this information should be provided by RecordLayout I'm willing to work on that as well (though I guess those changes should be in a different patch). Do you have any ideas as to how it should expose the necessary information? 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D71973/new/

https://reviews.llvm.org/D71973