[PATCH] D143751: [clang][analyzer][NFC] Refactor code of StdLibraryFunctionsChecker.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Ugh, I admit, its a little hard to follow what happened here. You moved a lot of code around (I agree with that!), but also changed code as well. Can you just summarize what is NOT just moved code and needs a more thorough look?



================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:124-125
   // requirement would render the initialization of the Summary map infeasible.
+  // A new value constraint is created furthermore by the negate functionality
+  // of the constraint and returned as pointer.
   using ValueConstraintPtr = std::shared_ptr<ValueConstraint>;
----------------
Is this what you meant?


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:218
+  /// Check if a single argument falls into a specific range.
+  /// The "range" can be built from sub-ranges that are closed intervals.
   class RangeConstraint : public ValueConstraint {
----------------
If you want a killer doc, use examples. Not saying this is not good enough, but it wouldn't hurt.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:220-222
+    /// The constraint can be specified by allowing or disallowing the range.
+    /// WithinRange indicates allowing the range, OutOfRange indicates
+    /// disallowing it.
----------------
I'd place parts of this comments above the enum values declaration (now on line 72), but even that small comment that is already there looks good enough. After that, maybe there is not much to note here.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:262
 
+  protected:
     bool checkSpecificValidity(const FunctionDecl *FD) const override {
----------------
Nice.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1031
+
+ProgramStateRef StdLibraryFunctionsChecker::NotNullConstraint::apply(ProgramStateRef State, const CallEvent &Call,
+                      const Summary &Summary,
----------------
seems like this could use [[ https://clang.llvm.org/docs/ClangFormat.html#script-for-patch-reformatting | clang-format-diff.py]].


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143751