[PATCH] D44143: [clang-tidy] Create properly seeded random generator check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang-tidy/cert/CERTTidyModule.cpp:40
     // DCL
-    CheckFactories.registerCheck<PostfixOperatorCheck>(
-        "cert-dcl21-cpp");
+    CheckFactories.registerCheck<PostfixOperatorCheck>("cert-dcl21-cpp");
     CheckFactories.registerCheck<VariadicFunctionDefCheck>("cert-dcl50-cpp");
----------------
This change looks unrelated to the patch.


================
Comment at: clang-tidy/cert/CERTTidyModule.cpp:44
         "cert-dcl54-cpp");
-    CheckFactories.registerCheck<DontModifyStdNamespaceCheck>(
-        "cert-dcl58-cpp");
+    CheckFactories.registerCheck<DontModifyStdNamespaceCheck>("cert-dcl58-cpp");
     CheckFactories.registerCheck<google::build::UnnamedNamespaceInHeaderCheck>(
----------------
This change looks unrelated to the patch.


================
Comment at: clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp:35-37
+      hasAnyName("linear_congruential_engine", "mersenne_twister_engine",
+                 "subtract_with_carry_engine", "discard_block_engine",
+                 "independent_bits_engine", "shuffle_order_engine"));
----------------
These should be full-qualified names.


================
Comment at: clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp:74
+      callExpr(has(implicitCastExpr(has(
+                   declRefExpr(hasDeclaration(namedDecl(hasName("srand"))))))))
+          .bind("srand"),
----------------
I think that in C mode, this is fine, but in C++ mode it should register `::std::srand`.


================
Comment at: clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp:114
+      Func->getArg(0)->IgnoreCasts()->getType().getAsString());
+  if (std::find(DisallowedSeedTypes.begin(), DisallowedSeedTypes.end(),
+                SeedType) != DisallowedSeedTypes.end()) {
----------------
You can use `llvm::find()` instead.


================
Comment at: docs/clang-tidy/checks/cert-msc51-cpp.rst:7
+This check flags all pseudo-random number engines, engine adaptor
+instantiations and srand when initialized or seeded with default argument,
+constant expression or any user-configurable type. Pseudo-random number
----------------
Backticks around `srand`


================
Comment at: docs/clang-tidy/checks/cert-msc51-cpp.rst:11
+security protocols.
+This is a CERT security rule, see MSC51-CPP.
+
----------------
Should link to the CERT rule, and also link to the C CERT rule.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44143