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