[clang] [clang][dataflow] Change `diagnoseFunction` to use `llvm::SmallVector` instead of `std::vector`. (PR #66014)
Yitzhak Mandelbaum via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 13 07:49:09 PDT 2023
https://github.com/ymand updated https://github.com/llvm/llvm-project/pull/66014:
>From 335f12e897d8d776dc476b468bc0dc1e7b8019df Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum <yitzhakm at google.com>
Date: Mon, 11 Sep 2023 21:34:00 +0000
Subject: [PATCH 1/2] [clang][dataflow] Change `diagnoseFunction` to take type
of diagnostic list instead of diagnostic itself.
The template is agnostic as to the type used by the list, as long as it is
compatible with `llvm::move` and `std::back_inserter`. In practice, we've
encountered analyses which use different types (`llvm::SmallVector` vs
`std::vector`), so it seems preferable to leave this open.
---
.../clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp | 4 ++--
.../clang/Analysis/FlowSensitive/DataflowAnalysis.h | 8 ++++----
.../FlowSensitive/TypeErasedDataflowAnalysisTest.cpp | 4 ++--
3 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
index 183beb6bfb87d80..5811e2a4cd02266 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
@@ -56,8 +56,8 @@ void UncheckedOptionalAccessCheck::check(
// `diagnoseFunction` with config options.
if (llvm::Expected<std::vector<SourceLocation>> Locs =
dataflow::diagnoseFunction<UncheckedOptionalAccessModel,
- SourceLocation>(*FuncDecl, *Result.Context,
- Diagnoser))
+ std::vector<SourceLocation>>(
+ *FuncDecl, *Result.Context, Diagnoser))
for (const SourceLocation &Loc : *Locs)
diag(Loc, "unchecked access to optional value");
else
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index abd34f40922121e..fd2d7fce09073bf 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -245,10 +245,10 @@ runDataflowAnalysis(
/// iterations.
/// - This limit is still low enough to keep runtimes acceptable (on typical
/// machines) in cases where we hit the limit.
-template <typename AnalysisT, typename Diagnostic>
-llvm::Expected<std::vector<Diagnostic>> diagnoseFunction(
+template <typename AnalysisT, typename DiagnosticList>
+llvm::Expected<DiagnosticList> diagnoseFunction(
const FunctionDecl &FuncDecl, ASTContext &ASTCtx,
- llvm::function_ref<std::vector<Diagnostic>(
+ llvm::function_ref<DiagnosticList(
const CFGElement &, ASTContext &,
const TransferStateForDiagnostics<typename AnalysisT::Lattice> &)>
Diagnoser,
@@ -263,7 +263,7 @@ llvm::Expected<std::vector<Diagnostic>> diagnoseFunction(
DataflowAnalysisContext AnalysisContext(std::move(OwnedSolver));
Environment Env(AnalysisContext, FuncDecl);
AnalysisT Analysis(ASTCtx);
- std::vector<Diagnostic> Diagnostics;
+ DiagnosticList Diagnostics;
if (llvm::Error Err =
runTypeErasedDataflowAnalysis(
*Context, Analysis, Env,
diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index af34a2afd24685a..3a8253508016caa 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -98,12 +98,12 @@ TEST(DataflowAnalysisTest, DiagnoseFunctionDiagnoserCalledOnEachElement) {
cast<FunctionDecl>(findValueDecl(AST->getASTContext(), "target"));
auto Diagnoser = [](const CFGElement &Elt, ASTContext &,
const TransferStateForDiagnostics<NoopLattice> &) {
- std::vector<std::string> Diagnostics(1);
+ llvm::SmallVector<std::string> Diagnostics(1);
llvm::raw_string_ostream OS(Diagnostics.front());
Elt.dumpToStream(OS);
return Diagnostics;
};
- auto Result = diagnoseFunction<NoopAnalysis, std::string>(
+ auto Result = diagnoseFunction<NoopAnalysis, llvm::SmallVector<std::string>>(
*Func, AST->getASTContext(), Diagnoser);
// `diagnoseFunction` provides no guarantees about the order in which elements
// are visited, so we use `UnorderedElementsAre`.
>From 8b73f912828e4fc836ba3bf65dde36920e247c48 Mon Sep 17 00:00:00 2001
From: Yitzhak Mandelbaum <yitzhakm at google.com>
Date: Tue, 12 Sep 2023 16:03:45 +0000
Subject: [PATCH 2/2] fixup! [clang][dataflow] Change `diagnoseFunction` to
take type of diagnostic list instead of diagnostic itself.
---
.../bugprone/UncheckedOptionalAccessCheck.cpp | 10 ++++------
.../clang/Analysis/FlowSensitive/DataflowAnalysis.h | 9 +++++----
.../Models/UncheckedOptionalAccessModel.h | 6 +++---
.../Models/UncheckedOptionalAccessModel.cpp | 8 ++++----
.../FlowSensitive/TypeErasedDataflowAnalysisTest.cpp | 2 +-
5 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
index 5811e2a4cd02266..0a0e212f345ed8d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
@@ -13,10 +13,8 @@
#include "clang/Analysis/FlowSensitive/DataflowAnalysis.h"
#include "clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h"
#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Error.h"
-#include <memory>
-#include <optional>
-#include <vector>
namespace clang::tidy::bugprone {
using ast_matchers::MatchFinder;
@@ -54,10 +52,10 @@ void UncheckedOptionalAccessCheck::check(
UncheckedOptionalAccessDiagnoser Diagnoser(ModelOptions);
// FIXME: Allow user to set the (defaulted) SAT iterations max for
// `diagnoseFunction` with config options.
- if (llvm::Expected<std::vector<SourceLocation>> Locs =
+ if (llvm::Expected<llvm::SmallVector<SourceLocation>> Locs =
dataflow::diagnoseFunction<UncheckedOptionalAccessModel,
- std::vector<SourceLocation>>(
- *FuncDecl, *Result.Context, Diagnoser))
+ SourceLocation>(*FuncDecl, *Result.Context,
+ Diagnoser))
for (const SourceLocation &Loc : *Locs)
diag(Loc, "unchecked access to optional value");
else
diff --git a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
index fd2d7fce09073bf..dd1a685cb48ba2d 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h
@@ -31,6 +31,7 @@
#include "llvm/ADT/Any.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLFunctionalExtras.h"
+#include "llvm/ADT/SmallVector.h"
#include "llvm/Support/Errc.h"
#include "llvm/Support/Error.h"
@@ -245,10 +246,10 @@ runDataflowAnalysis(
/// iterations.
/// - This limit is still low enough to keep runtimes acceptable (on typical
/// machines) in cases where we hit the limit.
-template <typename AnalysisT, typename DiagnosticList>
-llvm::Expected<DiagnosticList> diagnoseFunction(
+template <typename AnalysisT, typename Diagnostic>
+llvm::Expected<llvm::SmallVector<Diagnostic>> diagnoseFunction(
const FunctionDecl &FuncDecl, ASTContext &ASTCtx,
- llvm::function_ref<DiagnosticList(
+ llvm::function_ref<llvm::SmallVector<Diagnostic>(
const CFGElement &, ASTContext &,
const TransferStateForDiagnostics<typename AnalysisT::Lattice> &)>
Diagnoser,
@@ -263,7 +264,7 @@ llvm::Expected<DiagnosticList> diagnoseFunction(
DataflowAnalysisContext AnalysisContext(std::move(OwnedSolver));
Environment Env(AnalysisContext, FuncDecl);
AnalysisT Analysis(ASTCtx);
- DiagnosticList Diagnostics;
+ llvm::SmallVector<Diagnostic> Diagnostics;
if (llvm::Error Err =
runTypeErasedDataflowAnalysis(
*Context, Analysis, Env,
diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
index 76a567f1399cd41..aeaf75b2154222c 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -21,7 +21,7 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/NoopLattice.h"
#include "clang/Basic/SourceLocation.h"
-#include <vector>
+#include "llvm/ADT/SmallVector.h"
namespace clang {
namespace dataflow {
@@ -74,14 +74,14 @@ class UncheckedOptionalAccessDiagnoser {
UncheckedOptionalAccessDiagnoser(
UncheckedOptionalAccessModelOptions Options = {});
- std::vector<SourceLocation>
+ llvm::SmallVector<SourceLocation>
operator()(const CFGElement &Elt, ASTContext &Ctx,
const TransferStateForDiagnostics<NoopLattice> &State) {
return DiagnoseMatchSwitch(Elt, Ctx, State.Env);
}
private:
- CFGMatchSwitch<const Environment, std::vector<SourceLocation>>
+ CFGMatchSwitch<const Environment, llvm::SmallVector<SourceLocation>>
DiagnoseMatchSwitch;
};
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index e40bd3d0199bdd1..f61f26ff27804ec 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -34,7 +34,6 @@
#include <memory>
#include <optional>
#include <utility>
-#include <vector>
namespace clang {
namespace dataflow {
@@ -913,8 +912,8 @@ auto buildTransferMatchSwitch() {
.Build();
}
-std::vector<SourceLocation> diagnoseUnwrapCall(const Expr *ObjectExpr,
- const Environment &Env) {
+llvm::SmallVector<SourceLocation> diagnoseUnwrapCall(const Expr *ObjectExpr,
+ const Environment &Env) {
if (auto *OptionalVal = getValueBehindPossiblePointer(*ObjectExpr, Env)) {
auto *Prop = OptionalVal->getProperty("has_value");
if (auto *HasValueVal = cast_or_null<BoolValue>(Prop)) {
@@ -935,7 +934,8 @@ auto buildDiagnoseMatchSwitch(
// lot of duplicated work (e.g. string comparisons), consider providing APIs
// that avoid it through memoization.
auto IgnorableOptional = ignorableOptional(Options);
- return CFGMatchSwitchBuilder<const Environment, std::vector<SourceLocation>>()
+ return CFGMatchSwitchBuilder<const Environment,
+ llvm::SmallVector<SourceLocation>>()
// optional::value
.CaseOfCFGStmt<CXXMemberCallExpr>(
valueCall(IgnorableOptional),
diff --git a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
index 3a8253508016caa..3fc1bb6692acf0b 100644
--- a/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -103,7 +103,7 @@ TEST(DataflowAnalysisTest, DiagnoseFunctionDiagnoserCalledOnEachElement) {
Elt.dumpToStream(OS);
return Diagnostics;
};
- auto Result = diagnoseFunction<NoopAnalysis, llvm::SmallVector<std::string>>(
+ auto Result = diagnoseFunction<NoopAnalysis, std::string>(
*Func, AST->getASTContext(), Diagnoser);
// `diagnoseFunction` provides no guarantees about the order in which elements
// are visited, so we use `UnorderedElementsAre`.
More information about the cfe-commits
mailing list