[PATCH] D116025: [analyzer][NFC] Refactor GenericTaintChecker to use CallDescriptionMap

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

steakhal added a comment.

Sweet stuff!



================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:55
 
 /// Check if tainted data is used as a buffer size ins strn.. functions,
 /// and allocators.
----------------
typo


================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:163
+  ArgSet(ArgVecTy &&DiscreteArgs, Optional<ArgIdxTy> VariadicIndex = None)
       : DiscreteArgs(DiscreteArgs), VariadicIndex(VariadicIndex) {}
 
----------------



================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:311
 private:
   using NamePartTy = llvm::SmallVector<SmallString<32>, 2>;
 
----------------
I would expect this in plural form.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:317
 
+  template <typename Config> static NamePartTy parseNameParts(const Config &C);
+
----------------
Definitely reads oddly. The return type is singular but the function //parses name part**s**//.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:645
   if (!Config) {
-    TaintRules = RuleLookupTy{};
+    DynamicTaintRules = RuleLookupTy{};
     return;
----------------
A comment like this would make it cleaner:
`// We don't have external taint config, no parsing required.`


================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:749-750
       [this, &C, &IsMatching, &State](ArgIdxTy I, const Expr *E, SVal) {
         IsMatching |=
             PropSrcArgs.contains(I) && isTaintedOrPointsToTainted(E, State, C);
       });
----------------
It's unfortunate that we don't shortcircuit after the patch.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:863
   StringRef DomName = C.getMacroNameOrSpelling(DomLoc);
   // White list the internal communication protocols.
   bool SafeProtocol = DomName.equals("AF_SYSTEM") ||
----------------
Use inclusive terms.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116025