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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Ok! This looks like there isn't that much code, so it should be fine to duplicate. Do you have plans to eliminate the existing switch and instead use yaml for *all* supported functions while shipping a default .yaml file with the analyzer?

I'm still in doubts on how to connect your work with the `CallDescription` effort. I'll think more about that.



================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:210-218
+  /// Defines a map between the propagation function's name and
+  /// TaintPropagationRule.
+  static NameRuleMap CustomPropagations;
+
+  /// Defines a map between the filter function's name and filtering args.
+  static NameArgMap CustomFilters;
+
----------------
I think you don't need to make them static. By the time you need them, you already have access to the instance of the checker. Static variables of non-trivial types are frowned upon (https://llvm.org/docs/CodingStandards.html#do-not-use-static-constructors).


================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:294
 
+void GenericTaintChecker::getConfiguration(StringRef ConfigFile) {
+  if (ConfigFile.trim().empty())
----------------
Let's re-use at least this function. I.e., abstract it out from `GenericTaintChecker` and put it into a header accessible by all checkers (dunno, `lib/StaticAnalyzer/Checkers/Yaml.h`).


================
Comment at: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:316
+
+  parseConfiguration(std::move(Config));
+}
----------------
Do we ever need to copy the config? Maybe make it a move-only type?


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

https://reviews.llvm.org/D59555