[PATCH] D32700: [clang-tidy] Add misc-suspicious-memset-usage check.

Reka Kovacs via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon May 8 14:48:27 PDT 2017


rnkovacs added a comment.

The current proposition could be that we only keep the first two cases, possibly merging in the google check for a third case (with its old name evoking original functionality). Separately, another check could be written that warns when the below mentioned memory management functions are used on not `TriviallyCopyable` objects.

I wonder if any of the commenters/reviewers/subscribers have any thoughts about this :)



================
Comment at: clang-tidy/misc/SuspiciousMemsetUsageCheck.cpp:21
+void SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder *Finder) {
+  const auto HasCtorOrDtor =
+      eachOf(hasMethod(cxxConstructorDecl(unless(anyOf(
----------------
xazax.hun wrote:
> I think this might not be the best approach.
> 
> For example, if the constructor is compiler generated, but there is a member of the class with non-trivial constructor, we still want to warn. 
> 
> E.g.:
> 
> ```
> struct X { X() { /* something nontrivial */ } };
> 
> struct Y { X x; };
> ```
> 
> Maybe we should check instead whether the class is a POD? Other alternative might be something like 
> `CXXRecordDecl::hasNonTrivialDefaultConstructor`.
So, we had a discussion yesterday that I'll try to sum up here. The root of the problem is not exactly about constructors or the object being a POD, but rather about calling `memset()` on an object that is not `TriviallyCopyable`. But as you suggested, this holds for `memcpy()` and `memmove()` as well, and might be better placed in another check.


https://reviews.llvm.org/D32700





More information about the cfe-commits mailing list