samestep updated this revision to Diff 441117. samestep added a comment. - Merge branch 'diagnose-api' into optional-check-diagnose - Update using review comments
Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128352/new/ https://reviews.llvm.org/D128352 Files: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
Index: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h =================================================================== --- clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h +++ clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h @@ -109,14 +109,33 @@ /// dataflow analysis states that model the respective basic blocks. The /// returned vector, if any, will have the same size as the number of CFG /// blocks, with indices corresponding to basic block IDs. Returns an error if -/// the dataflow analysis cannot be performed successfully. +/// the dataflow analysis cannot be performed successfully. Otherwise, calls +/// `PostVisitStmt` on each statement with the final analysis results at that +/// program point. template <typename AnalysisT> llvm::Expected<std::vector< llvm::Optional<DataflowAnalysisState<typename AnalysisT::Lattice>>>> -runDataflowAnalysis(const ControlFlowContext &CFCtx, AnalysisT &Analysis, - const Environment &InitEnv) { - auto TypeErasedBlockStates = - runTypeErasedDataflowAnalysis(CFCtx, Analysis, InitEnv); +runDataflowAnalysis( + const ControlFlowContext &CFCtx, AnalysisT &Analysis, + const Environment &InitEnv, + std::function<void(const Stmt *, const DataflowAnalysisState< + typename AnalysisT::Lattice> &)> + PostVisitStmt = nullptr) { + std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)> + PostVisitStmtClosure = nullptr; + if (PostVisitStmt != nullptr) { + PostVisitStmtClosure = [&PostVisitStmt]( + const Stmt *Stmt, + const TypeErasedDataflowAnalysisState &State) { + auto *Lattice = + llvm::any_cast<typename AnalysisT::Lattice>(&State.Lattice.Value); + PostVisitStmt(Stmt, DataflowAnalysisState<typename AnalysisT::Lattice>{ + *Lattice, State.Env}); + }; + } + + auto TypeErasedBlockStates = runTypeErasedDataflowAnalysis( + CFCtx, Analysis, InitEnv, PostVisitStmtClosure); if (!TypeErasedBlockStates) return TypeErasedBlockStates.takeError(); Index: clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp =================================================================== --- clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp +++ clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp @@ -23,6 +23,7 @@ #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/Any.h" #include "llvm/ADT/Optional.h" +#include "llvm/ADT/STLExtras.h" #include "llvm/Support/Error.h" #include <memory> #include <vector> @@ -32,12 +33,13 @@ namespace bugprone { using ast_matchers::MatchFinder; using dataflow::SourceLocationsLattice; +using dataflow::UncheckedOptionalAccessDiagnoser; using dataflow::UncheckedOptionalAccessModel; using llvm::Optional; static constexpr llvm::StringLiteral FuncID("fun"); -static Optional<SourceLocationsLattice> +static Optional<std::vector<SourceLocation>> analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx) { using dataflow::ControlFlowContext; using dataflow::DataflowAnalysisState; @@ -52,23 +54,22 @@ std::make_unique<dataflow::WatchedLiteralsSolver>()); dataflow::Environment Env(AnalysisContext, FuncDecl); UncheckedOptionalAccessModel Analysis(ASTCtx); + UncheckedOptionalAccessDiagnoser Diagnoser; + std::vector<SourceLocation> Diagnostics; Expected<std::vector<Optional<DataflowAnalysisState<SourceLocationsLattice>>>> - BlockToOutputState = - dataflow::runDataflowAnalysis(*Context, Analysis, Env); + BlockToOutputState = dataflow::runDataflowAnalysis( + *Context, Analysis, Env, + [&ASTCtx, &Diagnoser, + &Diagnostics](const Stmt *Stmt, + const DataflowAnalysisState<SourceLocationsLattice> + &State) mutable { + auto StmtDiagnostics = Diagnoser.diagnose(ASTCtx, Stmt, State.Env); + llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics)); + }); if (!BlockToOutputState) return llvm::None; - assert(Context->getCFG().getExit().getBlockID() < BlockToOutputState->size()); - const Optional<DataflowAnalysisState<SourceLocationsLattice>> - &ExitBlockState = - (*BlockToOutputState)[Context->getCFG().getExit().getBlockID()]; - // `runDataflowAnalysis` doesn't guarantee that the exit block is visited; - // for example, when it is unreachable. - // FIXME: Diagnose violations even when the exit block is unreachable. - if (!ExitBlockState) - return llvm::None; - - return std::move(ExitBlockState->Lattice); + return Diagnostics; } void UncheckedOptionalAccessCheck::registerMatchers(MatchFinder *Finder) { @@ -97,9 +98,9 @@ if (FuncDecl->isTemplated()) return; - if (Optional<SourceLocationsLattice> Errors = + if (Optional<std::vector<SourceLocation>> Errors = analyzeFunction(*FuncDecl, *Result.Context)) - for (const SourceLocation &Loc : Errors->getSourceLocations()) + for (const SourceLocation &Loc : *Errors) diag(Loc, "unchecked access to optional value"); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits