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

Looks good with a few nits.


================
Comment at: clang-tidy/cert/StrToNumCheck.cpp:182
@@ +181,3 @@
+void StrToNumCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *CE = Result.Nodes.getNodeAs<CallExpr>("expr");
+  const FunctionDecl *FD = nullptr;
----------------
Too many abbreviations for my taste. How about CE -> Call, FD -> Function or 
FuncDecl, CK -> Conversion, CFD -> ConverterFunc?

================
Comment at: clang-tidy/cert/StrToNumCheck.cpp:224
@@ +223,3 @@
+
+  std::string ConversionPerformed = ClassifyConversionType(CK),
+              AlternateFunction = ClassifyReplacement(CK);
----------------
1. s/std::string/StringRef/
2. I'd make the functions return `StringRef` instead of a `const char *`
3. One variable per declaration, please.
4. I'm not sure the variables help making the code easier to read.

================
Comment at: docs/clang-tidy/checks/cert-err34-c.rst:9
@@ +8,3 @@
+does not flag calls to ``strtol()``, or other, related conversion functions 
that
+do perform better error checking.
+
----------------
Maybe add a couple of examples?


http://reviews.llvm.org/D19590



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

Reply via email to