[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

Thanks for the quick review!
Fixed the double backtick in the release notes as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Endre Fülöp via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd33f199910fa: [clang-tidy] Extend cert-oop57-cpp to check 
non-zero memset values (authored by gamesh411).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126186

Files:
  clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,17 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison 
operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a 
non-trivially default constructible class is undefined
+}
+
+void nonLiteralSetValue(char C) {
+  NonTrivial Data;
+  // Check non-literal second argument.
+  std::memset(, C, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a 
non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -156,6 +156,9 @@
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for an arbitrary expression in the second argument of `memset`.
+
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -80,7 +80,7 @@
   auto IsRecordSizeOf =
   expr(sizeOfExpr(hasArgumentOfType(equalsBoundNode("Record";
   auto ArgChecker = [&](Matcher RecordConstraint,
-BindableMatcher SecondArg) {
+BindableMatcher SecondArg = expr()) {
 return allOf(argumentCountIs(3),
  hasArgument(0, IsStructPointer(RecordConstraint, true)),
  hasArgument(1, SecondArg), hasArgument(2, IsRecordSizeOf));
@@ -89,8 +89,7 @@
   Finder->addMatcher(
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, 
MemSetNames,
-   ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+   ArgChecker(unless(isTriviallyDefaultConstructible(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,17 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+}
+
+void nonLiteralSetValue(char C) {
+  NonTrivial Data;
+  // Check non-literal second argument.
+  std::memset(, C, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -156,6 +156,9 @@
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for an arbitrary expression in the second argument of `memset`.
+
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -80,7 +80,7 @@
   auto IsRecordSizeOf =
   

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:160
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for an arbitrary expression in the second argument of `memset`.
+

Please use double back-ticks for language constructs.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 432868.
gamesh411 added a comment.

Remove literal checking from the matcher for memset as well

There is no change in the result set on open source projects even without
restricting the matches to literals.

IMO this is more in line with the rule as it's written as @aaron.ballman 
mentioned.

Updated the ReleaseNotes to reflect this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126186

Files:
  clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,17 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison 
operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a 
non-trivially default constructible class is undefined
+}
+
+void nonLiteralSetValue(char C) {
+  NonTrivial Data;
+  // Check non-literal second argument.
+  std::memset(, C, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a 
non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -156,6 +156,9 @@
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for an arbitrary expression in the second argument of `memset`.
+
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -80,7 +80,7 @@
   auto IsRecordSizeOf =
   expr(sizeOfExpr(hasArgumentOfType(equalsBoundNode("Record";
   auto ArgChecker = [&](Matcher RecordConstraint,
-BindableMatcher SecondArg) {
+BindableMatcher SecondArg = expr()) {
 return allOf(argumentCountIs(3),
  hasArgument(0, IsStructPointer(RecordConstraint, true)),
  hasArgument(1, SecondArg), hasArgument(2, IsRecordSizeOf));
@@ -89,8 +89,7 @@
   Finder->addMatcher(
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, 
MemSetNames,
-   ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+   ArgChecker(unless(isTriviallyDefaultConstructible(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,17 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+}
+
+void nonLiteralSetValue(char C) {
+  NonTrivial Data;
+  // Check non-literal second argument.
+  std::memset(, C, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -156,6 +156,9 @@
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for an arbitrary expression in the second argument of `memset`.
+
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- 

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 432866.
gamesh411 added a comment.

fix Release Notes


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126186

Files:
  clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,10 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison 
operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a 
non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -152,6 +152,9 @@
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for non-zero integer literal `memset` arguments as well.
+
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -90,7 +90,7 @@
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, 
MemSetNames,
ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+  expr(integerLiteral(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,10 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -152,6 +152,9 @@
   ` when `sizeof(...)` is
   compared against a `__int128_t`.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for non-zero integer literal `memset` arguments as well.
+
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   ` check.
 
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -90,7 +90,7 @@
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, MemSetNames,
ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+  expr(integerLiteral(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:201
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for non-zero integer literal  memset arguments as well.

Please sort entries in section alphabetically.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:202
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for non-zero integer literal  memset arguments as well.
+

Please fix double space and enclose `memset` into double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 431348.
gamesh411 added a comment.

Add full diff with arcanist


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126186

Files:
  clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,10 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison 
operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a 
non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -198,6 +198,9 @@
   ` to simplify 
expressions
   using DeMorgan's Theorem.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for non-zero integer literal  memset arguments as well.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -90,7 +90,7 @@
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, 
MemSetNames,
ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+  expr(integerLiteral(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,10 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -198,6 +198,9 @@
   ` to simplify expressions
   using DeMorgan's Theorem.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for non-zero integer literal  memset arguments as well.
+
 Removed checks
 ^^
 
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -90,7 +90,7 @@
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, MemSetNames,
ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+  expr(integerLiteral(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D126186#3530938 , @njames93 wrote:

> As a side point I'm not sure this change really follows what the rule is 
> trying to enforce. The rule is about not using std::memset to reinitialise 
> objects that aren't trivial. Having said that limiting it to only 0 does seem 
> a little restrictive,

I think this change makes sense for the rule as it's written, per "Do not use 
std::memset() to initialize an object of nontrivial class type as it may not 
properly initialize the value representation of the object." The noncompliant 
example was using zero initialization, but there's nothing inherently special 
about `0` except for how commonly used it is as a default value.


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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

This diff looks like it's rooted on the clang-tools-extra directory which is 
why the pre-merge bot is failing to build.


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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Balázs Benics via Phabricator via cfe-commits]

steakhal added a comment.

In D126186#3530967 , @gamesh411 wrote:

> Was there a certificate change on the reviews.llmv.org site maybe?)

Yes, AFAIK.

Please redo the measurement, it doesn't look right.
F23164509: image.png 


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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 updated this revision to Diff 431319.
gamesh411 added a comment.

Added a release note
Also generated the full context (arcanist could validate the site certificate, 
that's why I had to resort to manual diff creation. Was there a certificate 
change on the reviews.llmv.org site maybe?)


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

https://reviews.llvm.org/D126186

Files:
  clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
  docs/ReleaseNotes.rst
  test/clang-tidy/checkers/cert-oop57-cpp.cpp


Index: test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,10 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison 
operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a 
non-trivially default constructible class is undefined
+}
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -198,6 +198,9 @@
   ` to simplify 
expressions
   using DeMorgan's Theorem.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for non-zero integer literal  memset arguments as well.
+
 Removed checks
 ^^
 
Index: clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -90,7 +90,7 @@
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, 
MemSetNames,
ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+  expr(integerLiteral(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(


Index: test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,10 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+}
Index: docs/ReleaseNotes.rst
===
--- docs/ReleaseNotes.rst
+++ docs/ReleaseNotes.rst
@@ -198,6 +198,9 @@
   ` to simplify expressions
   using DeMorgan's Theorem.
 
+- Made :doc:`cert-oop57-cpp ` more sensitive
+  by checking for non-zero integer literal  memset arguments as well.
+
 Removed checks
 ^^
 
Index: clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -90,7 +90,7 @@
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, MemSetNames,
ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+  expr(integerLiteral(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Please upload diff with full context.
Can you add a note about this in the release notes.

As a side point I'm not sure this change really follows what the rule is trying 
to before. The rule is about not using std::memset to reinitialise objects that 
aren't trivial. Having said that limiting it to only 0 does seem a little 
restrictive,


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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.

F23163926: CSA_20testbench_20report.zip 
There is no change in the results as far as these OS are concerned.


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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Balázs Benics via Phabricator via cfe-commits]

steakhal added reviewers: njames93, aaron.ballman, alexfh.
steakhal added a comment.

Looks reasonable to me.


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

https://reviews.llvm.org/D126186

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D126186: [clang-tidy] Extend cert-oop57-cpp to check non-zero memset values

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 created this revision.
gamesh411 added reviewers: steakhal, martong, whisperity.
gamesh411 added a project: clang-tools-extra.
Herald added subscribers: carlosgalvezp, Szelethus, dkrupp, rnkovacs, xazax.hun.
Herald added a project: All.
gamesh411 requested review of this revision.
Herald added a subscriber: cfe-commits.

Clang Tidy check cert-oop57-cpp now checks for arbitrary-valued integer
literals in memset expressions containing non-trivially
default-constructible instances. Previously it only checked 0 values.

Note that the first non-compliant example of
https://wiki.sei.cmu.edu/confluence/display/cplusplus/OOP57-CPP.+Prefer+special+member+functions+and+overloaded+operators+to+C+Standard+Library+functions
 requires this.

A comparative analysis of OS projects is currently running to see the impact of 
this change.


https://reviews.llvm.org/D126186

Files:
  clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,10 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison 
operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a 
non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -90,7 +90,7 @@
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, 
MemSetNames,
ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+  expr(integerLiteral(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(


Index: clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cert-oop57-cpp.cpp
@@ -88,3 +88,10 @@
   mymemcmp(, , sizeof(Data));
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: consider using comparison operators instead of calling 'mymemcmp'
 }
+
+void nonNullSetValue() {
+  NonTrivial Data;
+  // Check non-null-valued second argument.
+  std::memset(, 1, sizeof(Data));
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: calling 'memset' on a non-trivially default constructible class is undefined
+}
Index: clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
===
--- clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
+++ clang-tools-extra/clang-tidy/cert/NonTrivialTypesLibcMemoryCallsCheck.cpp
@@ -90,7 +90,7 @@
   callExpr(callee(namedDecl(hasAnyName(
utils::options::parseListPair(BuiltinMemSet, MemSetNames,
ArgChecker(unless(isTriviallyDefaultConstructible()),
-  expr(integerLiteral(equals(0)
+  expr(integerLiteral(
   .bind("lazyConstruct"),
   this);
   Finder->addMatcher(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits