[PATCH] D131879: [clang][analyzer] Errno modeling code refactor (NFC).

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

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

Approved with nits. Thanks!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:267
 
+const char *describeErrnoCheckState(errno_modeling::ErrnoCheckState CS) {
+  assert(CS == errno_modeling::MustNotBeChecked &&
----------------
The same applies for these.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:298
+  if (!State)
+    return State;
+  return errno_modeling::setErrnoValue(State, C.getLocationContext(), ErrnoSym,
----------------
I would prefer returning `nullptr` explicitly here.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.cpp:304-307
+  std::string Message = "Assuming that function '";
+  Message += Fn;
+  Message += "' is successful, in this case the value 'errno' ";
+  Message += describeErrnoCheckState(MustNotBeChecked);
----------------
I believe you should use `Twine()` for chaining string slices.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrnoModeling.h:83
+/// others are not used by the clients.
+const char *describeErrnoCheckState(errno_modeling::ErrnoCheckState CS);
+
----------------
It seems like you are already within this namespace. You don't need to explicitly qualify this.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:405
   protected:
-    /// Many of the descendant classes use this value.
-    const errno_modeling::ErrnoCheckState CheckState;
-
-    ErrnoConstraintBase(errno_modeling::ErrnoCheckState CS) : CheckState(CS) {}
+    ErrnoConstraintBase() {}
 
----------------
Why don't you `= default` it if the body is actually empty?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:416
+  /// function call.
+  class FailureErrnoConstraint : public ErrnoConstraintBase {
   public:
----------------
I find this name much more appealing than it was before! Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131879