[PATCH] D116329: [clang-check] Adjust argument adjusters for clang-check to strip options blocking the static analyzer

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.


================
Comment at: clang/tools/clang-check/ClangCheck.cpp:217
+    if (!AnalyzerOutput.empty()) {
+      Tool.appendArgumentsAdjuster(getInsertArgumentAdjuster("-o"));
+      Tool.appendArgumentsAdjuster(
----------------
you can pass a vector to getInsertArgumentsAdjuster


================
Comment at: clang/tools/clang-check/ClangCheck.cpp:225
+    //
+    // The syntax-only adjuster can also help us to remove other options that
+    // trigger output generation, e.g. -save-temps. Besides, to enable the
----------------
You've removed the mention of the default syntax-only adjuster, so this comment seems confusing.

"The syntax-only adjuster is installed by default.
Good: It strips options that trigger extra output, like --save-temps.
Bad: We don't want the -fsyntax-only arg itself, as it suppresses analyzer output"

Then strip -fsyntax-only, then add -analyze?


================
Comment at: clang/tools/clang-check/ClangCheck.cpp:236
+              AdjustedArgs.emplace_back(Arg);
+            } else if (!HasAnalyze) {
+              AdjustedArgs.emplace_back("--analyze");
----------------
this seems like a particularly confusing way to say "if (Arg == "-fsyntax-only && !HasAnalyze)"

The "replace first -fsyntax-only with -analyze, strip others" seems more clearly expressed as "strip -fsyntax-only" and "add -analyze" as separate steps.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116329