diff --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp --- a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp @@ -18,7 +18,6 @@ #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/DataflowLattice.h" #include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h" -#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h" #include "clang/Analysis/FlowSensitive/WatchedLiteralsSolver.h" #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/Any.h" @@ -32,7 +31,6 @@ namespace tidy { namespace bugprone { using ast_matchers::MatchFinder; -using dataflow::SourceLocationsLattice; using dataflow::UncheckedOptionalAccessDiagnoser; using dataflow::UncheckedOptionalAccessModel; using llvm::Optional; @@ -56,13 +54,14 @@ UncheckedOptionalAccessModel Analysis(ASTCtx); UncheckedOptionalAccessDiagnoser Diagnoser; std::vector Diagnostics; - Expected>>> + Expected>>> BlockToOutputState = dataflow::runDataflowAnalysis( *Context, Analysis, Env, - [&ASTCtx, &Diagnoser, - &Diagnostics](const Stmt *Stmt, - const DataflowAnalysisState - &State) mutable { + [&ASTCtx, &Diagnoser, &Diagnostics]( + const Stmt *Stmt, + const DataflowAnalysisState + &State) mutable { auto StmtDiagnostics = Diagnoser.diagnose(ASTCtx, Stmt, State.Env); llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics)); }); diff --git a/clang/docs/tools/clang-formatted-files.txt b/clang/docs/tools/clang-formatted-files.txt --- a/clang/docs/tools/clang-formatted-files.txt +++ b/clang/docs/tools/clang-formatted-files.txt @@ -131,6 +131,7 @@ clang/include/clang/Analysis/FlowSensitive/DataflowWorklist.h clang/include/clang/Analysis/FlowSensitive/MapLattice.h clang/include/clang/Analysis/FlowSensitive/MatchSwitch.h +clang/include/clang/Analysis/FlowSensitive/NoopLattice.h clang/include/clang/Analysis/FlowSensitive/Solver.h clang/include/clang/Analysis/FlowSensitive/SourceLocationsLattice.h clang/include/clang/Analysis/FlowSensitive/StorageLocation.h diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h --- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h +++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h @@ -19,7 +19,7 @@ #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/MatchSwitch.h" -#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h" +#include "clang/Analysis/FlowSensitive/NoopLattice.h" #include "clang/Basic/SourceLocation.h" #include @@ -38,15 +38,11 @@ bool IgnoreSmartPointerDereference = false; }; -/// Dataflow analysis that discovers unsafe accesses of optional values and -/// adds the respective source locations to the lattice. +/// Dataflow analysis that models whether optionals hold values or not. /// /// Models the `std::optional`, `absl::optional`, and `base::Optional` types. -/// -/// FIXME: Consider separating the models from the unchecked access analysis. class UncheckedOptionalAccessModel - : public DataflowAnalysis { + : public DataflowAnalysis { public: UncheckedOptionalAccessModel( ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options = {}); @@ -54,12 +50,9 @@ /// Returns a matcher for the optional classes covered by this model. static ast_matchers::DeclarationMatcher optionalClassDecl(); - static SourceLocationsLattice initialElement() { - return SourceLocationsLattice(); - } + static NoopLattice initialElement() { return {}; } - void transfer(const Stmt *Stmt, SourceLocationsLattice &State, - Environment &Env); + void transfer(const Stmt *Stmt, NoopLattice &State, Environment &Env); bool compareEquivalent(QualType Type, const Value &Val1, const Environment &Env1, const Value &Val2, @@ -70,7 +63,7 @@ Environment &MergedEnv) override; private: - MatchSwitch> TransferMatchSwitch; + MatchSwitch> TransferMatchSwitch; }; class UncheckedOptionalAccessDiagnoser { diff --git a/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h b/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h new file mode 100644 --- /dev/null +++ b/clang/include/clang/Analysis/FlowSensitive/NoopLattice.h @@ -0,0 +1,41 @@ +//===-- NoopLattice.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 the lattice with exactly one element. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H +#define LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H + +#include "clang/Analysis/FlowSensitive/DataflowLattice.h" +#include + +namespace clang { +namespace dataflow { + +/// Trivial lattice for dataflow analysis with exactly one element. +/// +/// Useful for analyses that only need the Environment and nothing more. +class NoopLattice { +public: + bool operator==(const NoopLattice &Other) const { return true; } + + LatticeJoinEffect join(const NoopLattice &Other) { + return LatticeJoinEffect::Unchanged; + } +}; + +inline std::ostream &operator<<(std::ostream &OS, const NoopLattice &) { + return OS << "noop"; +} + +} // namespace dataflow +} // namespace clang + +#endif // LLVM_CLANG_ANALYSIS_FLOWSENSITIVE_NOOP_LATTICE_H diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -20,7 +20,7 @@ #include "clang/ASTMatchers/ASTMatchers.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" #include "clang/Analysis/FlowSensitive/MatchSwitch.h" -#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h" +#include "clang/Analysis/FlowSensitive/NoopLattice.h" #include "clang/Analysis/FlowSensitive/Value.h" #include "clang/Basic/SourceLocation.h" #include "llvm/ADT/StringRef.h" @@ -35,7 +35,7 @@ namespace { using namespace ::clang::ast_matchers; -using LatticeTransferState = TransferState; +using LatticeTransferState = TransferState; DeclarationMatcher optionalClass() { return classTemplateSpecializationDecl( @@ -312,18 +312,7 @@ if (auto *Loc = maybeInitializeOptionalValueMember( UnwrapExpr->getType(), *OptionalVal, State.Env)) State.Env.setStorageLocation(*UnwrapExpr, *Loc); - - auto *Prop = OptionalVal->getProperty("has_value"); - if (auto *HasValueVal = cast_or_null(Prop)) { - if (State.Env.flowConditionImplies(*HasValueVal)) - return; - } } - - // Record that this unwrap is *not* provably safe. - // FIXME: include either the name of the optional (if applicable) or a source - // range of the access for easier interpretation of the result. - State.Lattice.getSourceLocations().insert(ObjectExpr->getBeginLoc()); } void transferMakeOptionalCall(const CallExpr *E, @@ -716,12 +705,10 @@ UncheckedOptionalAccessModel::UncheckedOptionalAccessModel( ASTContext &Ctx, UncheckedOptionalAccessModelOptions Options) - : DataflowAnalysis( - Ctx), + : DataflowAnalysis(Ctx), TransferMatchSwitch(buildTransferMatchSwitch(Options)) {} -void UncheckedOptionalAccessModel::transfer(const Stmt *S, - SourceLocationsLattice &L, +void UncheckedOptionalAccessModel::transfer(const Stmt *S, NoopLattice &L, Environment &Env) { LatticeTransferState State(L, Env); TransferMatchSwitch(*S, getASTContext(), State); diff --git a/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h b/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h --- a/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h +++ b/clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h @@ -18,25 +18,11 @@ #include "clang/AST/Stmt.h" #include "clang/Analysis/FlowSensitive/DataflowAnalysis.h" #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h" -#include "clang/Analysis/FlowSensitive/DataflowLattice.h" -#include +#include "clang/Analysis/FlowSensitive/NoopLattice.h" namespace clang { namespace dataflow { -class NoopLattice { -public: - bool operator==(const NoopLattice &) const { return true; } - - LatticeJoinEffect join(const NoopLattice &) { - return LatticeJoinEffect::Unchanged; - } -}; - -inline std::ostream &operator<<(std::ostream &OS, const NoopLattice &) { - return OS << "noop"; -} - class NoopAnalysis : public DataflowAnalysis { public: /// `ApplyBuiltinTransfer` controls whether to run the built-in transfer