[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

Hey, I'm really sorry, I missed this, too but I just landed the patch for you, 
apologies for such a huge delay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97265

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGda168dd875bf: [clang] Allow clang-check to customize 
analyzer output file or dir name (authored by OikawaKirie, committed by 
kbobyrev).
Herald added a subscriber: manas.

Changed prior to commit:
  https://reviews.llvm.org/D97265?vs=328927=387266#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97265

Files:
  clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
  clang/tools/clang-check/ClangCheck.cpp


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,6 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
@@ -206,7 +210,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo 
-ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: echo '// CHECK: {{qwerty}}' > %t/cclog-check
+// RUN: clang-check -p "%t" "%t/test.cpp" -analyze 
-analyzer-output-path=%t/qwerty -extra-arg=-v -extra-arg=-Xclang 
-extra-arg=-verify 2>&1 | FileCheck %t/cclog-check
+// RUN: FileCheck %s --input-file=%t/qwerty
+
+// CHECK: DOCTYPE plist
+// CHECK: Division by zero
+int f() {
+  return 1 / 0; // expected-warning {{Division by zero}}
+}


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,6 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
@@ -206,7 +210,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo -ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: echo '// CHECK: {{qwerty}}' > 

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

Ping @sammccall again for landing this patch on my behalf.
Thanks.


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

https://reviews.llvm.org/D97265

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

If this patch is OK, could you please commit it on my behalf (Ella Ma 
)?
Thx.
And I will continue working on the `-Xanalyzer` option for clang-check, as it 
is too complex when adding analyzer options via the `-extra-arg` option.


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

https://reviews.llvm.org/D97265

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Really sorry about the long delays. Working on clearing my backlog...
This seems reasonable to me!


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

https://reviews.llvm.org/D97265

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

ping @sammccall again

Do I need to add other reviewers if you are busy doing your job?


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

https://reviews.llvm.org/D97265

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

@sammccall ping.

Updated as you mentioned in D97265#2581653 



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

https://reviews.llvm.org/D97265

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie updated this revision to Diff 328927.
OikawaKirie added a comment.

Replace concatenating the output from clang-check and plist with separated 
checks to workaround the test failure on Windows.


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

https://reviews.llvm.org/D97265

Files:
  clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
  clang/tools/clang-check/ClangCheck.cpp


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,7 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
-
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
   cl::cat(ClangCheckCategory));
@@ -206,7 +209,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo 
-ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: echo '// CHECK: {{qwerty}}' > %t/cclog-check
+// RUN: clang-check -p "%t" "%t/test.cpp" -analyze 
-analyzer-output-path=%t/qwerty -extra-arg=-v -extra-arg=-Xclang 
-extra-arg=-verify 2>&1 | FileCheck %t/cclog-check
+// RUN: FileCheck %s --input-file=%t/qwerty
+
+// CHECK: DOCTYPE plist
+// CHECK: Division by zero
+int f() {
+  return 1 / 0; // expected-warning {{Division by zero}}
+}


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,7 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
-
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
   cl::cat(ClangCheckCategory));
@@ -206,7 +209,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo -ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: echo '// CHECK: {{qwerty}}' > %t/cclog-check
+// RUN: clang-check -p "%t" "%t/test.cpp" -analyze -analyzer-output-path=%t/qwerty -extra-arg=-v -extra-arg=-Xclang -extra-arg=-verify 2>&1 | 

[PATCH] D97265: [clang] Allow clang-check to customize analyzer output file or dir name

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie updated this revision to Diff 328396.
OikawaKirie retitled this revision from "[clang] Allow clang-check to customize 
output file name" to "[clang] Allow clang-check to customize analyzer output 
file or dir name".
OikawaKirie edited the summary of this revision.
OikawaKirie added a comment.
Herald added subscribers: steakhal, ASDenysPetrov, dkrupp, donat.nagy, 
Szelethus, a.sidorin, baloghadamsoftware.

1. Update option name from `-output` to `-analyzer-output-path`. As it can 
either be a file name or a directory name, the word `path` may not be accurate. 
Please give me your suggestions on this option name.
2. Add more verifications on the output files, rather than verifying the value 
of the `-o` option. For simplicity, the regression test case only checks the 
plist format.


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

https://reviews.llvm.org/D97265

Files:
  clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
  clang/tools/clang-check/ClangCheck.cpp


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,7 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
-
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
   cl::cat(ClangCheckCategory));
@@ -206,7 +209,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
===
--- /dev/null
+++ clang/test/Tooling/clang-check-set-analyzer-output-path.cpp
@@ -0,0 +1,15 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cd %t
+// RUN: echo '[{"directory":".","command":"clang++ -c %t/test.cpp -o foo 
-ofoo","file":"%t/test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: cp "%s" "%t/test.cpp"
+// RUN: clang-check -p "%t" "%t/test.cpp" -analyze 
-analyzer-output-path=qwerty -extra-arg=-v -extra-arg=-Xclang 
-extra-arg=-verify 2>&1 | cat - qwerty | FileCheck %s
+// FIXME: Make the above easier.
+
+// CHECK: Invocation
+// CHECK: {{qwerty}}
+// CHECK: DOCTYPE plist
+// CHECK: Division by zero
+int f() {
+  return 1 / 0; // expected-warning {{Division by zero}}
+}


Index: clang/tools/clang-check/ClangCheck.cpp
===
--- clang/tools/clang-check/ClangCheck.cpp
+++ clang/tools/clang-check/ClangCheck.cpp
@@ -76,7 +76,10 @@
 Analyze("analyze",
 cl::desc(Options.getOptionHelpText(options::OPT_analyze)),
 cl::cat(ClangCheckCategory));
-
+static cl::opt
+AnalyzerOutput("analyzer-output-path",
+   cl::desc(Options.getOptionHelpText(options::OPT_o)),
+   cl::cat(ClangCheckCategory));
 static cl::opt
 Fixit("fixit", cl::desc(Options.getOptionHelpText(options::OPT_fixit)),
   cl::cat(ClangCheckCategory));
@@ -206,7 +209,19 @@
 
   // Clear adjusters because -fsyntax-only is inserted by the default chain.
   Tool.clearArgumentsAdjusters();
-  Tool.appendArgumentsAdjuster(getClangStripOutputAdjuster());
+
+  // Reset output path if is provided by user.
+  Tool.appendArgumentsAdjuster(
+  Analyze ? [&](const CommandLineArguments , StringRef File) {
+  auto Ret = getClangStripOutputAdjuster()(Args, File);
+  if (!AnalyzerOutput.empty()) {
+Ret.emplace_back("-o");
+Ret.emplace_back(AnalyzerOutput);
+  }
+  return Ret;
+}
+  : getClangStripOutputAdjuster());
+
   Tool.appendArgumentsAdjuster(getClangStripDependencyFileAdjuster());
 
   // Running the analyzer requires --analyze. Other modes can work with the
Index: clang/test/Tooling/clang-check-set-analyzer-output-path.cpp