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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
