Szelethus created this revision.
Szelethus added reviewers: NoQ, xazax.hun, baloghadamsoftware, rnkovacs, 
martong, balazske, dcoughlin.
Szelethus added a project: clang.
Herald added subscribers: cfe-commits, ASDenysPetrov, steakhal, Charusso, 
gamesh411, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, szepet, whisperity.
Szelethus added a parent revision: D76509: [analyzer][NFC] Move the text output 
type to its own file, move code to PathDiagnosticConsumer creator functions.

The title and the included test file sums everything up -- the only thing I'm 
mildly afraid of is whether anyone actually depends on the weird behavior of 
`HTMLDiagnostics` pretending to be `TextDiagnostics` if an output directory is 
not supplied. If it is, I guess we would need to resort to tiptoeing around the 
compatibility flag.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76510

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
  clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
  clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/output_types.cpp

Index: clang/test/Analysis/output_types.cpp
===================================================================
--- /dev/null
+++ clang/test/Analysis/output_types.cpp
@@ -0,0 +1,49 @@
+// RUN: not %clang_analyze_cc1 %s \
+// RUN:   -analyzer-output=plist \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PLIST
+
+// CHECK-PLIST: error: analyzer output type 'plist' requires an output file to
+// CHECK-PLIST-SAME: be specified with -o </path/to/output_file>
+
+
+// RUN: not %clang_analyze_cc1 %s \
+// RUN:   -analyzer-output=plist-multi-file \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PLIST-MULTI
+
+// CHECK-PLIST-MULTI: error: analyzer output type 'plist-multi-file' requires
+// CHECK-PLIST-MULTI-SAME: an output file to be specified with
+// CHECK-PLIST-MULTI-SAME: -o </path/to/output_file>
+
+
+// RUN: not %clang_analyze_cc1 %s \
+// RUN:   -analyzer-output=plist-html \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-PLIST-HTML
+
+// CHECK-PLIST-HTML: error: analyzer output type 'plist-html' requires an output
+// CHECK-PLIST-HTML-SAME: directory to be specified with
+// CHECK-PLIST-HTML-SAME: -o </path/to/output_directory>
+
+
+// RUN: not %clang_analyze_cc1 %s \
+// RUN:   -analyzer-output=sarif \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-SARIF
+
+// CHECK-SARIF: error: analyzer output type 'sarif' requires an output file to
+// CHECK-SARIF-SAME: be specified with -o </path/to/output_file>
+
+
+// RUN: not %clang_analyze_cc1 %s \
+// RUN:   -analyzer-output=html \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-HTML
+
+// CHECK-HTML: error: analyzer output type 'html' requires an output directory
+// CHECK-HTML-SAME: to be specified with -o </path/to/output_directory>
+
+
+// RUN: not %clang_analyze_cc1 %s \
+// RUN:   -analyzer-output=html-single-file \
+// RUN:   2>&1 | FileCheck %s -check-prefix=CHECK-HTML-SINGLE
+
+// CHECK-HTML-SINGLE: error: analyzer output type 'html-single-file' requires
+// CHECK-HTML-SINGLE-SAME: an output directory to be specified with
+// CHECK-HTML-SINGLE-SAME: -o </path/to/output_directory>
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -23,6 +23,7 @@
 #include "clang/Analysis/CodeInjector.h"
 #include "clang/Basic/SourceManager.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/StaticAnalyzer/Checkers/LocalCheckers.h"
Index: clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -14,6 +14,7 @@
 #include "clang/Basic/Version.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/StaticAnalyzer/Core/PathDiagnosticConsumers.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringMap.h"
@@ -51,9 +52,11 @@
     const std::string &Output, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
 
-  // TODO: Emit an error here.
-  if (Output.empty())
+  if (Output.empty()) {
+    PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
+      << "sarif" << "file";
     return;
+  }
 
   C.push_back(new SarifDiagnostics(AnalyzerOpts, Output, PP.getLangOpts()));
   createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, Output, PP, CTU);
Index: clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
@@ -16,6 +16,7 @@
 #include "clang/Basic/SourceManager.h"
 #include "clang/Basic/Version.h"
 #include "clang/CrossTU/CrossTranslationUnit.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Frontend/ASTUnit.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/TokenConcatenation.h"
@@ -571,10 +572,10 @@
 //===----------------------------------------------------------------------===//
 
 PlistDiagnostics::PlistDiagnostics(
-    AnalyzerOptions &AnalyzerOpts, const std::string &output,
+    AnalyzerOptions &AnalyzerOpts, const std::string &OutputFile,
     const Preprocessor &PP, const cross_tu::CrossTranslationUnitContext &CTU,
     bool supportsMultipleFiles)
-    : OutputFile(output), PP(PP), CTU(CTU), AnOpts(AnalyzerOpts),
+    : OutputFile(OutputFile), PP(PP), CTU(CTU), AnOpts(AnalyzerOpts),
       SupportsCrossFileDiagnostics(supportsMultipleFiles) {
   // FIXME: Will be used by a later planned change.
   (void)this->CTU;
@@ -585,9 +586,11 @@
     const std::string &OutputFile, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
 
-  // TODO: Emit an error here.
-  if (OutputFile.empty())
+  if (OutputFile.empty()) {
+    PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
+      << "plist" << "file";
     return;
+  }
 
   C.push_back(new PlistDiagnostics(AnalyzerOpts, OutputFile, PP, CTU,
                                    /*supportsMultipleFiles*/ false));
@@ -599,9 +602,11 @@
     const std::string &OutputFile, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
 
-  // TODO: Emit an error here.
-  if (OutputFile.empty())
+  if (OutputFile.empty()) {
+    PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
+      << "plist-multi-file" << "file";
     return;
+  }
 
   C.push_back(new PlistDiagnostics(AnalyzerOpts, OutputFile, PP, CTU,
                                    /*supportsMultipleFiles*/ true));
Index: clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
@@ -21,6 +21,7 @@
 #include "clang/Lex/Lexer.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/Token.h"
+#include "clang/Driver/DriverDiagnostic.h"
 #include "clang/Rewrite/Core/HTMLRewrite.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/StaticAnalyzer/Core/AnalyzerOptions.h"
@@ -137,18 +138,14 @@
     const std::string &OutputDir, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
 
-  // FIXME: HTML is currently our default output type, but if the output
-  // directory isn't specified, it acts like if it was in the minimal text
-  // output mode. This doesn't make much sense, we should have the minimal text
-  // as our default. In the case of backward compatibility concerns, this could
-  // be preserved with -analyzer-config-compatibility-mode=true.
-  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
-
-  // TODO: Emit an error here.
-  if (OutputDir.empty())
+  if (OutputDir.empty()) {
+    PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
+      << "html" << "directory";
     return;
+  }
 
   C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, true));
+  createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
 }
 
 void ento::createHTMLSingleFileDiagnosticConsumer(
@@ -156,9 +153,11 @@
     const std::string &OutputDir, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
 
-  // TODO: Emit an error here.
-  if (OutputDir.empty())
+  if (OutputDir.empty()) {
+    PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
+      << "html-single-file" << "directory";
     return;
+  }
 
   C.push_back(new HTMLDiagnostics(AnalyzerOpts, OutputDir, PP, false));
   createTextMinimalPathDiagnosticConsumer(AnalyzerOpts, C, OutputDir, PP, CTU);
@@ -168,6 +167,13 @@
     AnalyzerOptions &AnalyzerOpts, PathDiagnosticConsumers &C,
     const std::string &prefix, const Preprocessor &PP,
     const cross_tu::CrossTranslationUnitContext &CTU) {
+
+  if (prefix.empty()) {
+    PP.getDiagnostics().Report(diag::err_analyzer_missing_output_loc)
+      << "plist-html" << "directory";
+    return;
+  }
+
   createHTMLDiagnosticConsumer(
       AnalyzerOpts, C, std::string(llvm::sys::path::parent_path(prefix)), PP,
       CTU);
Index: clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
===================================================================
--- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -206,7 +206,7 @@
   ConfigTable Config;
   AnalysisStores AnalysisStoreOpt = RegionStoreModel;
   AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel;
-  AnalysisDiagClients AnalysisDiagOpt = PD_HTML;
+  AnalysisDiagClients AnalysisDiagOpt = PD_TEXT_MINIMAL;
   AnalysisPurgeMode AnalysisPurgeOpt = PurgeStmt;
 
   std::string AnalyzeSpecificFunction;
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -341,6 +341,9 @@
   "checker '%0' has no option called '%1'">;
 def err_analyzer_checker_option_invalid_input : Error<
   "invalid input for checker option '%0', that expects %1">;
+def err_analyzer_missing_output_loc : Error<
+  "analyzer output type '%0' requires an output %1 to be specified with "
+  "-o </path/to/output_%1>">;
 
 def err_drv_invalid_hvx_length : Error<
   "-mhvx-length is not supported without a -mhvx/-mhvx= flag">;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to