Szelethus added a comment.
Hmm, I find the revision title and summary a little vague -- I'd expect a
revision called "Refactoring" not to feature any funcitonal change, yet you
changed how variadic functions are handled. Please
- Keep purely formatting changes to your earlier revision, and rebase this
patch on that
- Edit the revision title and/or summary about what your patch does,
- If it features any change in the checker's behavior, include tests.
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:125
+ unsigned VariadicIndex;
+ /// Show when a function has variadic parameters. If it has, it mark all
+ /// of them as source or destination.
----------------
typo: marks
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:132-137
+ TaintPropagationRule(std::initializer_list<unsigned> &&Src,
+ std::initializer_list<unsigned> &&Dst,
+ VariadicType Var = VariadicType::None,
+ unsigned VarIndex = InvalidArgIndex)
+ : SrcArgs(std::move(Src)), DstArgs(std::move(Dst)),
+ VariadicIndex(VarIndex), VarType(Var) {}
----------------
I like this //a lot//! Call sites look very nice.
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:145-158
inline void addSrcArg(unsigned A) { SrcArgs.push_back(A); }
- inline void addDstArg(unsigned A) { DstArgs.push_back(A); }
+ inline void addDstArg(unsigned A) { DstArgs.push_back(A); }
- inline bool isNull() const { return SrcArgs.empty(); }
+ inline bool isNull() const {
+ return SrcArgs.empty() && DstArgs.empty() &&
+ VariadicType::None == VarType;
+ }
----------------
In-class method definitions are implicitly `inline` -- could you please remove
them while we're cleaning up anyways? :)
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55734/new/
https://reviews.llvm.org/D55734
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
