Szelethus added a comment.
In general, the patch is looking alright, I'll take a second look later on.
Don't mind my inlines too much, they are more directed towards the original
code then your changes.
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:53-56
+ void Profile(llvm::FoldingSetNodeID &ID) const {
+ ID.AddInteger(Arg);
+ ID.AddInteger(TagType);
+ }
----------------
Interesting, isn't this predefined for `std::pair`?
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:474-476
+ const FunctionDecl *FDecl = C.getCalleeDecl(CE);
+ if (!FDecl || FDecl->getKind() != Decl::Function)
+ return;
----------------
When do these happen? For implicit functions in C?
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:478-480
+ StringRef Name = C.getCalleeName(FDecl);
+ if (Name.empty())
+ return;
----------------
And this? Lambdas maybe?
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:560
- for (unsigned ArgNum : TaintArgs) {
+ for (auto ArgTypePair : TaintArgs) {
+ unsigned ArgNum = ArgTypePair.Arg;
----------------
Please add the full type name, else it isn't obvious why we don't take it as a
const reference.
================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:561-562
+ for (auto ArgTypePair : TaintArgs) {
+ unsigned ArgNum = ArgTypePair.Arg;
+ TaintTagType TagType = ArgTypePair.TagType;
// Special handling for the tainted return value.
----------------
We don't use `std::pair`, so we have descriptive field names, do we need these?
================
Comment at: lib/StaticAnalyzer/Checkers/Taint.h:25-28
using TaintTagType = unsigned;
-static constexpr TaintTagType TaintTagGeneric = 0;
+static constexpr TaintTagType TaintTagNotTainted = 0;
+static constexpr TaintTagType TaintTagGeneric = 1;
----------------
Is there a **very** good reason behind us not using an `enum` instead?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59516/new/
https://reviews.llvm.org/D59516
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
