NoQ created this revision. NoQ added reviewers: vsavchenko, xazax.hun, martong, Szelethus, baloghadamsoftware, Charusso. Herald added subscribers: steakhal, ASDenysPetrov, dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, rnkovacs, szepet, mgorny. NoQ requested review of this revision.
This `clang::DiagnosticConsumer` can take normal clang diagnostics (eg., Sema errors or Clang-Tidy warnings) and turn them into static analyzer's `PathDiagnostic`s which are then fed into `PathDiagnosticConsumer`s. It's an essential component for my clang-tidy/static analyzer cross-integration efforts. The consumer isn't magically aware of information that isn't present in Clang diagnostics but expected in path diagnostics. Such info includes checker name / bug type / category; the consumer expects an external provider to provide such information for every incoming diagnostic. Additionally, `PathDiagnostic`'s declaration-with-issue and uniqueing-declaration are currently set to `nullptr`. I'm slowly thinking about auto-detecting those based on the source location but technically all path diagnostic consumer facilities can function just fine without them (declaration-with-issue is there purely for displaying user-visible names and uniqueing declaration only makes sense for path-sensitive warnings that require exotic uniqueing which will probably never be fed into this consumer). There are also a few features missing here and there, such as fix-it hint support, which i plan to add later. Though technically possible, this consumer is never supposed to consume the output of `TextPathDiagnosticConsumer`; a lot of information is lost that way that can't be recovered. In particular, the proper way to teach clang-tidy to emit path diagnostics while keeping in mind that it already ships with static analyzer integration is to allow the integrated static analyzer to talk to path diagnostic consumers directly; static analyzer warnings should never go through this new consumer. https://reviews.llvm.org/D94476 Files: clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h clang/lib/Analysis/CMakeLists.txt clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp clang/unittests/Analysis/CMakeLists.txt clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp
Index: clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp =================================================================== --- /dev/null +++ clang/unittests/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp @@ -0,0 +1,157 @@ +//===- unittests/Analysis/CFGTest.cpp - CFG tests -------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h" +#include "clang/Frontend/CompilerInstance.h" +#include "clang/Tooling/Tooling.h" +#include "gtest/gtest.h" + +using namespace clang; +using namespace ento; +using namespace llvm; + +namespace { +class TestPathDiagnosticConsumer : public PathDiagnosticConsumer { +public: + struct ExpectedPieceTy { + SourceLocation Loc; + std::string Text; + }; + struct ExpectedDiagTy { + SourceLocation Loc; + std::string VerboseDescription; + std::string ShortDescription; + std::string CheckerName; + std::string BugType; + std::string Category; + std::vector<ExpectedPieceTy> Pieces; + }; + typedef std::vector<ExpectedDiagTy> ExpectedDiagsTy; + +private: + ExpectedDiagsTy ExpectedDiags; + +public: + TestPathDiagnosticConsumer(ExpectedDiagsTy ExpectedDiags) + : ExpectedDiags(ExpectedDiags) + {} + + virtual StringRef getName() const override { return "test"; } + + virtual void FlushDiagnosticsImpl(std::vector<const PathDiagnostic *> &Diags, + FilesMade *filesMade) override { + EXPECT_EQ(Diags.size(), ExpectedDiags.size()); + for (size_t I = 0, E = Diags.size(); I < E; ++I) { + const PathDiagnostic &Diag = *Diags[I]; + const ExpectedDiagTy &ExpectedDiag = ExpectedDiags[I]; + + EXPECT_EQ(Diag.getLocation().asLocation(), ExpectedDiag.Loc); + EXPECT_EQ(Diag.getVerboseDescription(), ExpectedDiag.VerboseDescription); + EXPECT_EQ(Diag.getShortDescription(), ExpectedDiag.ShortDescription); + EXPECT_EQ(Diag.getCheckerName(), ExpectedDiag.CheckerName); + EXPECT_EQ(Diag.getBugType(), ExpectedDiag.BugType); + EXPECT_EQ(Diag.getCategory(), ExpectedDiag.Category); + + const PathPieces &Pieces = Diag.path; + EXPECT_EQ(Pieces.size(), ExpectedDiag.Pieces.size()); + size_t PieceI = 0; + for (const auto &Piece: Pieces) { + const ExpectedPieceTy &ExpectedPiece = ExpectedDiag.Pieces[PieceI]; + EXPECT_EQ(Piece->getLocation().asLocation(), ExpectedPiece.Loc); + EXPECT_EQ(Piece->getString(), ExpectedPiece.Text); + ++PieceI; + } + } + } +}; + +PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo bugTypeInfoProvider( + const Diagnostic &) { + return PathDiagnosticConverterDiagnosticConsumer::BugTypeInfo{ + "test check name", "test bug type", "test bug category"}; +} + +class PathDiagnosticConverterDiagnosticConsumerTestConsumer + : public ASTConsumer { + SourceManager &SM; + + void performTest(const Decl *D) { + TestPathDiagnosticConsumer::ExpectedDiagsTy ExpectedDiags{ + TestPathDiagnosticConsumer::ExpectedDiagTy{ + D->getBeginLoc(), + "Test warning", // Auto-capitalization! + "Test warning", // Auto-capitalization! + "test check name", + "test bug type", + "test bug category", + {TestPathDiagnosticConsumer::ExpectedPieceTy{ + D->getBeginLoc(), + "Test warning", // Auto-capitalization! + }, + TestPathDiagnosticConsumer::ExpectedPieceTy{ + D->getEndLoc(), + "Test note" // Auto-capitalization! + }}}}; + + TestPathDiagnosticConsumer TestPDC {ExpectedDiags}; + PathDiagnosticConsumers PathConsumers = {&TestPDC}; + + llvm::IntrusiveRefCntPtr<DiagnosticsEngine> DiagEngine = + new DiagnosticsEngine(new DiagnosticIDs, new DiagnosticOptions); + PathDiagnosticConverterDiagnosticConsumer DiagConsumer( + PathConsumers, bugTypeInfoProvider, + /*ShouldDisplayNotesAsEvents=*/true); + DiagEngine->setClient(&DiagConsumer, /*ShouldOwnClient=*/false); + DiagEngine->setSourceManager(&SM); + + unsigned WarningDiagID = + DiagEngine->getCustomDiagID(DiagnosticsEngine::Warning, "test warning"); + DiagEngine->Report(D->getBeginLoc(), WarningDiagID); + + unsigned NoteDiagID = + DiagEngine->getCustomDiagID(DiagnosticsEngine::Note, "test note"); + DiagEngine->Report(D->getEndLoc(), NoteDiagID); + + // The last diagnostic must be flushed manually! + DiagConsumer.finalize(); + + PathDiagnosticConsumer::FilesMade FM; + TestPDC.FlushDiagnostics(&FM); + } + +public: + PathDiagnosticConverterDiagnosticConsumerTestConsumer(CompilerInstance &CI) + : SM(CI.getSourceManager()) {} + + bool HandleTopLevelDecl(DeclGroupRef DG) override { + for (auto *D : DG) + performTest(D); + + return true; + } +}; + +class PathDiagnosticConverterDiagnosticConsumerTestAction + : public ASTFrontendAction { +public: + std::unique_ptr<ASTConsumer> CreateASTConsumer(CompilerInstance &CI, + StringRef File) override { + return std::make_unique< + PathDiagnosticConverterDiagnosticConsumerTestConsumer>(CI); + } +}; + +// Constructing a CFG for a range-based for over a dependent type fails (but +// should not crash). +TEST(PathDiagnosticConverter, ConsumeWarning) { + EXPECT_TRUE(tooling::runToolOnCode( + std::make_unique<PathDiagnosticConverterDiagnosticConsumerTestAction>(), + "void foo() {}", "input.c")); +} + +} // end of anonymous namespace Index: clang/unittests/Analysis/CMakeLists.txt =================================================================== --- clang/unittests/Analysis/CMakeLists.txt +++ clang/unittests/Analysis/CMakeLists.txt @@ -8,6 +8,7 @@ CFGTest.cpp CloneDetectionTest.cpp ExprMutationAnalyzerTest.cpp + PathDiagnosticConverterDiagnosticConsumer.cpp ) clang_target_link_libraries(ClangAnalysisTests Index: clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp =================================================================== --- /dev/null +++ clang/lib/Analysis/PathDiagnosticConverterDiagnosticConsumer.cpp @@ -0,0 +1,79 @@ +//===--- PathDiagnosticConverterDiagnosticConsumer.cpp ----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines the PathDiagnosticConverterDiagnosticConsumer object +// which converts regular Clang diagnostics into PathDiagnostics. +// +//===----------------------------------------------------------------------===// + +#include <clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h> + +using namespace clang; +using namespace ento; + +void PathDiagnosticConverterDiagnosticConsumer::flushPartialDiagnostic() { + if (PartialPDs.empty()) + return; + + for (PathDiagnosticConsumer *Consumer : PathConsumers) { + Consumer->HandlePathDiagnostic(std::move(PartialPDs[Consumer])); + } + + PartialPDs.clear(); +} + +void PathDiagnosticConverterDiagnosticConsumer::HandleDiagnostic( + DiagnosticsEngine::Level DiagLevel, const Diagnostic &Info) { + // Count warnings/errors. + DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info); + + llvm::SmallString<128> Msg; + Info.FormatDiagnostic(Msg); + Msg[0] = toupper(Msg[0]); + std::string CleanMsg = Msg.str().substr(0, Msg.str().find('[') - 1).str(); + + SourceLocation Loc = Info.getLocation(); + const SourceManager &SM = Info.getSourceManager(); + + PathDiagnosticLocation PDLoc(Loc, SM); + + // Notes do not come in attached to their respective warnings but are fed + // to us independently. The good news is they always follow their respective + // warnings and come in in the right order so we can automatically re-attach + // them. + if (DiagLevel == DiagnosticsEngine::Note) { + for (PathDiagnosticConsumer *Consumer : PathConsumers) { + PathDiagnosticPieceRef Piece; + if (ShouldDisplayNotesAsEvents) { + Piece = std::make_shared<PathDiagnosticEventPiece>(PDLoc, Msg); + } else { + Piece = std::make_shared<PathDiagnosticNotePiece>(PDLoc, Msg); + } + PartialPDs[Consumer]->getMutablePieces().push_back(Piece); + } + } else { + // No more incoming notes. The current path diagnostic is complete. + flushPartialDiagnostic(); + + BugTypeInfo BTI = BTIProvider(Info); + + for (PathDiagnosticConsumer *Consumer : PathConsumers) { + auto PD = std::make_unique<PathDiagnostic>( + BTI.CheckName, /*DeclWithIssue=*/nullptr, BTI.BugType, + CleanMsg, CleanMsg, BTI.BugCategory, PDLoc, /*DeclToUnique=*/nullptr, + std::make_unique<FilesToLineNumsMap>()); + + PathDiagnosticPieceRef Piece = + std::make_shared<PathDiagnosticEventPiece>(PDLoc, CleanMsg); + + PD->setEndOfPath(std::move(Piece)); + + PartialPDs[Consumer] = std::move(PD); + } + } +} Index: clang/lib/Analysis/CMakeLists.txt =================================================================== --- clang/lib/Analysis/CMakeLists.txt +++ clang/lib/Analysis/CMakeLists.txt @@ -23,6 +23,7 @@ LiveVariables.cpp ObjCNoReturn.cpp PathDiagnostic.cpp + PathDiagnosticConverterDiagnosticConsumer.cpp PlistPathDiagnosticConsumer.cpp PlistHTMLPathDiagnosticConsumer.cpp PostOrderCFGView.cpp Index: clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h =================================================================== --- /dev/null +++ clang/include/clang/Analysis/PathDiagnosticConverterDiagnosticConsumer.h @@ -0,0 +1,62 @@ +//===--- PathDiagnosticConverterDiagnosticConsumer.h ------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines a Clang DiagnosticConsumer that converts Clang diagnostics +// into PathDiagnostics. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ANALYSIS_PATHDIAGNOSTICCONVERTERDIAGNOSTICCONSUMER_H +#define LLVM_CLANG_ANALYSIS_PATHDIAGNOSTICCONVERTERDIAGNOSTICCONSUMER_H + +#include "clang/Analysis/PathDiagnostic.h" +#include "clang/Analysis/PathDiagnosticConsumers.h" + +namespace clang { +namespace ento { + +class PathDiagnosticConverterDiagnosticConsumer : public DiagnosticConsumer { +public: + struct BugTypeInfo { + StringRef CheckName; + StringRef BugType; + StringRef BugCategory; + }; + + using BugTypeInfoProvider = std::function<BugTypeInfo(const Diagnostic &)>; + +private: + PathDiagnosticConsumers &PathConsumers; + BugTypeInfoProvider BTIProvider; + bool ShouldDisplayNotesAsEvents; + + std::map<PathDiagnosticConsumer *, std::unique_ptr<PathDiagnostic>> + PartialPDs; + + void flushPartialDiagnostic(); + +public: + PathDiagnosticConverterDiagnosticConsumer( + PathDiagnosticConsumers &PathConsumers, BugTypeInfoProvider &&BTIProvider, + bool ShouldDisplayNotesAsEvents) + : PathConsumers(PathConsumers), BTIProvider(std::move(BTIProvider)), + ShouldDisplayNotesAsEvents(ShouldDisplayNotesAsEvents) {} + + void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, + const Diagnostic &Info) override; + + /// Inform the consumer that the last diagnostic has been sent. This is + /// necessary because the consumer does not know whether more notes are + /// going to be attached to the last warning. + void finalize() { flushPartialDiagnostic(); } +}; + +} // end 'ento' namespace +} // end 'clang' namespace + +#endif
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
