alexfh added a comment.
> The existing google-runtime-memset-zero-length check is related. It finds
> cases when the byte count parameter is zero and offers to swap that with the
> fill value argument. Perhaps the two could be merged while maintaining
> backward compatibility through an alias.
I think, the checks should be merged, and there's no need to maintain the old
behavior under the `google-runtime-memset-zero-length` alias. I'm not even sure
we want to make this alias, as long as the new check isn't ridiculously noisy.
At this point I'd suggest that we go back to the idea of adding a separate
module for checks that target patterns that are likely to be bugs rather than
performance, readability or style issues. The name could be "bugprone" or
something like this, and we could move many misc- checks there eventually.
================
Comment at: clang-tidy/misc/SuspiciousMemsetUsageCheck.cpp:21
+void SuspiciousMemsetUsageCheck::registerMatchers(MatchFinder *Finder) {
+ const auto HasCtorOrDtor =
+ eachOf(hasMethod(cxxConstructorDecl(unless(anyOf(
----------------
rnkovacs wrote:
> 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.
This logic could be either here or in a separate check (covering 'memcpy',
'memmove' and friends), but it might even be reasonable to create a compiler
diagnostic for this eventually (maybe after a test drive of the logic in a
clang-tidy check).
https://reviews.llvm.org/D32700
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
