[clang] cf1f978 - [clang][dataflow] Use NoopLattice in optional model
Sam Estep via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 29 13:10:47 PDT 2022
Author: Sam Estep
Date: 2022-06-29T20:10:42Z
New Revision: cf1f978d319b91464370d71289e1c7c30baa4243
URL: https://github.com/llvm/llvm-project/commit/cf1f978d319b91464370d71289e1c7c30baa4243
DIFF: https://github.com/llvm/llvm-project/commit/cf1f978d319b91464370d71289e1c7c30baa4243.diff
LOG: [clang][dataflow] Use NoopLattice in optional model
Followup to D128352. This patch pulls the `NoopLattice` class out from the `NoopAnalysis.h` test file into its own `NoopLattice.h` source file, and uses it to replace usage of `SourceLocationsLattice` in `UncheckedOptionalAccessModel`.
Reviewed By: ymandel, sgatev, gribozavr2, xazax.hun
Differential Revision: https://reviews.llvm.org/D128356
Added:
clang/include/clang/Analysis/FlowSensitive/NoopLattice.h
Modified:
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
clang/docs/tools/clang-formatted-files.txt
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/unittests/Analysis/FlowSensitive/NoopAnalysis.h
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
index 9766450e81654..8a945a5143067 100644
--- 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 clang {
namespace tidy {
namespace bugprone {
using ast_matchers::MatchFinder;
-using dataflow::SourceLocationsLattice;
using dataflow::UncheckedOptionalAccessDiagnoser;
using dataflow::UncheckedOptionalAccessModel;
using llvm::Optional;
@@ -56,13 +54,14 @@ analyzeFunction(const FunctionDecl &FuncDecl, ASTContext &ASTCtx) {
UncheckedOptionalAccessModel Analysis(ASTCtx);
UncheckedOptionalAccessDiagnoser Diagnoser;
std::vector<SourceLocation> Diagnostics;
- Expected<std::vector<Optional<DataflowAnalysisState<SourceLocationsLattice>>>>
+ Expected<std::vector<
+ Optional<DataflowAnalysisState<UncheckedOptionalAccessModel::Lattice>>>>
BlockToOutputState = dataflow::runDataflowAnalysis(
*Context, Analysis, Env,
- [&ASTCtx, &Diagnoser,
- &Diagnostics](const Stmt *Stmt,
- const DataflowAnalysisState<SourceLocationsLattice>
- &State) mutable {
+ [&ASTCtx, &Diagnoser, &Diagnostics](
+ const Stmt *Stmt,
+ const DataflowAnalysisState<UncheckedOptionalAccessModel::Lattice>
+ &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
index ed8b5e7e7ae7c..40d95411c3a0a 100644
--- 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/DataflowLattice.h
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
index 701db6470ac2f..25054deaf8afc 100644
--- 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 <vector>
@@ -38,15 +38,11 @@ struct UncheckedOptionalAccessModelOptions {
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<UncheckedOptionalAccessModel,
- SourceLocationsLattice> {
+ : public DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice> {
public:
UncheckedOptionalAccessModel(
ASTContext &AstContext, UncheckedOptionalAccessModelOptions Options = {});
@@ -54,12 +50,9 @@ class UncheckedOptionalAccessModel
/// 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 @@ class UncheckedOptionalAccessModel
Environment &MergedEnv) override;
private:
- MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch;
+ MatchSwitch<TransferState<NoopLattice>> 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
index 0000000000000..0192193281119
--- /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 <ostream>
+
+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
index 7d87d812dd575..eef3cc813a4ac 100644
--- 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 dataflow {
namespace {
using namespace ::clang::ast_matchers;
-using LatticeTransferState = TransferState<SourceLocationsLattice>;
+using LatticeTransferState = TransferState<NoopLattice>;
DeclarationMatcher optionalClass() {
return classTemplateSpecializationDecl(
@@ -312,18 +312,7 @@ void transferUnwrapCall(const Expr *UnwrapExpr, const Expr *ObjectExpr,
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<BoolValue>(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::optionalClassDecl() {
UncheckedOptionalAccessModel::UncheckedOptionalAccessModel(
ASTContext &Ctx, UncheckedOptionalAccessModelOptions Options)
- : DataflowAnalysis<UncheckedOptionalAccessModel, SourceLocationsLattice>(
- Ctx),
+ : DataflowAnalysis<UncheckedOptionalAccessModel, NoopLattice>(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
index eab5782095bbc..45ed414406372 100644
--- 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 <ostream>
+#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<NoopAnalysis, NoopLattice> {
public:
/// `ApplyBuiltinTransfer` controls whether to run the built-in transfer
More information about the cfe-commits
mailing list