Author: Kirstóf Umann
Date: 2020-05-20T01:36:06+02:00
New Revision: fe1a3a7e8c8be33968b9a768666489823dabab10

URL: 
https://github.com/llvm/llvm-project/commit/fe1a3a7e8c8be33968b9a768666489823dabab10
DIFF: 
https://github.com/llvm/llvm-project/commit/fe1a3a7e8c8be33968b9a768666489823dabab10.diff

LOG: [analyzer] Change the default output type to PD_TEXT_MINIMAL in the 
frontend, error if an output loc is missing for PathDiagConsumers that need it

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.

Differential Revision: https://reviews.llvm.org/D76510

Added: 
    clang/test/Analysis/output_types.cpp

Modified: 
    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

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
b/clang/include/clang/Basic/DiagnosticDriverKinds.td
index d010a7dfb2de..fdca8532ab53 100644
--- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -358,6 +358,9 @@ def err_analyzer_checker_option_invalid_input : Error<
   "invalid input for checker option '%0', that expects %1">;
 def err_analyzer_checker_incompatible_analyzer_option : Error<
   "checker cannot be enabled with analyzer option '%0' == %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">;

diff  --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h 
b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
index d2df24a6e21b..373caa30bbc9 100644
--- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
+++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.h
@@ -206,7 +206,7 @@ class AnalyzerOptions : public 
RefCountedBase<AnalyzerOptions> {
   ConfigTable Config;
   AnalysisStores AnalysisStoreOpt = RegionStoreModel;
   AnalysisConstraints AnalysisConstraintsOpt = RangeConstraintsModel;
-  AnalysisDiagClients AnalysisDiagOpt = PD_HTML;
+  AnalysisDiagClients AnalysisDiagOpt = PD_TEXT_MINIMAL;
   AnalysisPurgeMode AnalysisPurgeOpt = PurgeStmt;
 
   std::string AnalyzeSpecificFunction;

diff  --git a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
index 184fdcfb3d4b..8ce3aa2e081e 100644
--- a/clang/lib/StaticAnalyzer/Core/HTMLDiagnostics.cpp
+++ b/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 @@ void ento::createHTMLDiagnosticConsumer(
     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 @@ void ento::createHTMLSingleFileDiagnosticConsumer(
     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 @@ void ento::createPlistHTMLDiagnosticConsumer(
     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);

diff  --git a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
index 9b6369aee7a8..354091c804f9 100644
--- a/clang/lib/StaticAnalyzer/Core/PlistDiagnostics.cpp
+++ b/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 @@ static void printBugPath(llvm::raw_ostream &o, const 
FIDMap& FM,
 
//===----------------------------------------------------------------------===//
 
 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 @@ void ento::createPlistDiagnosticConsumer(
     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 @@ void ento::createPlistMultiFileDiagnosticConsumer(
     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));

diff  --git a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp 
b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
index 8c2e85601576..1ff66098bc2b 100644
--- a/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
+++ b/clang/lib/StaticAnalyzer/Core/SarifDiagnostics.cpp
@@ -15,6 +15,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"
@@ -52,9 +53,11 @@ void ento::createSarifDiagnosticConsumer(
     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);

diff  --git a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp 
b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
index 74c689730e58..988774f71620 100644
--- a/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ b/clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -23,6 +23,7 @@
 #include "clang/Analysis/PathDiagnostic.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/Rewrite/Core/Rewriter.h"

diff  --git a/clang/test/Analysis/output_types.cpp 
b/clang/test/Analysis/output_types.cpp
new file mode 100644
index 000000000000..8dbd916a467b
--- /dev/null
+++ b/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>


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

Reply via email to