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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 25 05:17:41 PDT 2018


aaron.ballman added inline comments.


================
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>(
----------------
boga95 wrote:
> aaron.ballman wrote:
> > This change looks unrelated to the patch.
> Clang format did it when I apply it to the whole file. 
You should clang-format the patch, not the entire file. See https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting for details.


================
Comment at: clang-tidy/cert/ProperlySeededRandomGeneratorCheck.cpp:74
+      callExpr(has(implicitCastExpr(has(
+                   declRefExpr(hasDeclaration(namedDecl(hasName("srand"))))))))
+          .bind("srand"),
----------------
boga95 wrote:
> aaron.ballman wrote:
> > I think that in C mode, this is fine, but in C++ mode it should register `::std::srand`.
> It is not match for ##::std::srand##, just for ##::srand##. I found some examples, but I think they don't work neither.
> 
> 
> 
As it stands, I'm not certain whether this will match code like `std::srand(0);`. Please add a test case to the C++ tests for it as it should be handled.


================
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
----------------
Eugene.Zelenko wrote:
> aaron.ballman wrote:
> > Backticks around `srand`
> Two of them and please add (). Will be good idea to make first statement same as in Release Notes.
This comment still has not been handled. Also, please remove the space between `srand` and the parens.


https://reviews.llvm.org/D44143





More information about the cfe-commits mailing list