[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

Generally looks good, some nits inline. Please mark comments you already solved as done.



================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:73-75
   static const unsigned InvalidArgIndex = UINT_MAX;
   /// Denotes the return vale.
   static const unsigned ReturnValueIndex = UINT_MAX - 1;
----------------
boga95 wrote:
> boga95 wrote:
> > Szelethus wrote:
> > > boga95 wrote:
> > > > Szelethus wrote:
> > > > > We should definitely change these, not only is the large integer number impossible to remember, but this value could differ on different platforms.
> > > > I tried to use int, but I got a lot of warnings because of the `getNumArgs()` returns an unsigned value.
> > > What warnings? I thought we have `-Wsign-conversion` disabled.
> > I got `-Wsign-compare` warnings, but it compiles. I will change it in the next [[ https://reviews.llvm.org/D59637 | review ]] because that's contains the yaml file and the related tests.
> Now, this is just for internal representation. The -1 value is mapped to this.
What about the C++ way using numeric limits?


================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:52
+  using ArgVector = SmallVector<unsigned, 2>;
+  using SignedArgVector = SmallVector<int, 2>;
+
----------------
Is there a way to have only one type for argument vectors?


================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:312
+
+  if (std::error_code ec = Input.error()) {
+    return;
----------------
We tend to not write the braces in small cases like this.


================
Comment at: test/Analysis/Inputs/taint-generic-config.yaml:16
+    VarType:  Dst
+    VarIndex: 1
+
----------------
Here Var stands for variadic I guess. I would prefer to avoid abbreviations as they might be misleading.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59555