[clang] 58fe7f9 - [clang][dataflow] Add API to separate analysis from diagnosis
Sam Estep via cfe-commits
cfe-commits at lists.llvm.org
Wed Jun 29 12:20:34 PDT 2022
Author: Sam Estep
Date: 2022-06-29T19:18:39Z
New Revision: 58fe7f9683a020a880426a4d8d61b067b83a9380
URL: https://github.com/llvm/llvm-project/commit/58fe7f9683a020a880426a4d8d61b067b83a9380
DIFF: https://github.com/llvm/llvm-project/commit/58fe7f9683a020a880426a4d8d61b067b83a9380.diff
LOG: [clang][dataflow] Add API to separate analysis from diagnosis
This patch adds an optional `PostVisitStmt` parameter to the `runTypeErasedDataflowAnalysis` function, which does one more pass over all statements in the CFG after a fixpoint is reached. It then defines a `diagnose` method for the optional model in a new `UncheckedOptionalAccessDiagnosis` class, but only integrates that into the tests and not the actual optional check for `clang-tidy`. That will be done in a followup patch.
The primary motivation is to separate the implementation of the unchecked optional access check into two parts, to allow for further refactoring of just the model part later, while leaving the checking part alone. Currently there is duplication between the `transferUnwrapCall` and `diagnoseUnwrapCall` functions, but that will be dealt with in the followup.
Because diagnostics are now all gathered into one collection rather than being populated at each program point like when computing a fixpoint, this patch removes the usage of `Pair` and `UnorderedElementsAre` from the optional model tests, and instead modifies all their expectations to simply check the stringified set of diagnostics against a single string, either `"safe"` or some concatenation of `"unsafe: input.cc:y:x"`. This is not ideal as it loses any connection to the `/*[[check]]*/` annotations in the source strings, but it does still retain the source locations from the diagnostic strings themselves.
Reviewed By: sgatev, gribozavr2, xazax.hun
Differential Revision: https://reviews.llvm.org/D127898
Added:
Modified:
clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TestingSupport.h
clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
index 2102ed56907bb..701db6470ac2f 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h
@@ -20,6 +20,8 @@
#include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Basic/SourceLocation.h"
+#include <vector>
namespace clang {
namespace dataflow {
@@ -71,6 +73,19 @@ class UncheckedOptionalAccessModel
MatchSwitch<TransferState<SourceLocationsLattice>> TransferMatchSwitch;
};
+class UncheckedOptionalAccessDiagnoser {
+public:
+ UncheckedOptionalAccessDiagnoser(
+ UncheckedOptionalAccessModelOptions Options = {});
+
+ std::vector<SourceLocation> diagnose(ASTContext &Context, const Stmt *Stmt,
+ const Environment &Env);
+
+private:
+ MatchSwitch<const Environment, std::vector<SourceLocation>>
+ DiagnoseMatchSwitch;
+};
+
} // namespace dataflow
} // namespace clang
diff --git a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
index 2d3a9e456370d..5e168194064f4 100644
--- a/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
+++ b/clang/include/clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h
@@ -115,13 +115,17 @@ TypeErasedDataflowAnalysisState transferBlock(
HandleTransferredStmt = nullptr);
/// Performs dataflow analysis and returns a mapping from basic block IDs to
-/// dataflow analysis states that model the respective basic blocks. Indices
-/// of the returned vector correspond to basic block IDs. Returns an error if
-/// the dataflow analysis cannot be performed successfully.
+/// dataflow analysis states that model the respective basic blocks. Indices of
+/// the returned vector correspond to basic block IDs. Returns an error if the
+/// dataflow analysis cannot be performed successfully. Otherwise, calls
+/// `PostVisitStmt` on each statement with the final analysis results at that
+/// program point.
llvm::Expected<std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>>>
-runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
- TypeErasedDataflowAnalysis &Analysis,
- const Environment &InitEnv);
+runTypeErasedDataflowAnalysis(
+ const ControlFlowContext &CFCtx, TypeErasedDataflowAnalysis &Analysis,
+ const Environment &InitEnv,
+ std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)>
+ PostVisitStmt = nullptr);
} // namespace dataflow
} // namespace clang
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index f424dba021254..7d87d812dd575 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -22,11 +22,13 @@
#include "clang/Analysis/FlowSensitive/MatchSwitch.h"
#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
#include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/SourceLocation.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Casting.h"
#include <cassert>
#include <memory>
#include <utility>
+#include <vector>
namespace clang {
namespace dataflow {
@@ -551,6 +553,17 @@ ignorableOptional(const UncheckedOptionalAccessModelOptions &Options) {
return llvm::None;
}
+StatementMatcher
+valueCall(llvm::Optional<StatementMatcher> &IgnorableOptional) {
+ return isOptionalMemberCallWithName("value", IgnorableOptional);
+}
+
+StatementMatcher
+valueOperatorCall(llvm::Optional<StatementMatcher> &IgnorableOptional) {
+ return expr(anyOf(isOptionalOperatorCallWithName("*", IgnorableOptional),
+ isOptionalOperatorCallWithName("->", IgnorableOptional)));
+}
+
auto buildTransferMatchSwitch(
const UncheckedOptionalAccessModelOptions &Options) {
// FIXME: Evaluate the efficiency of matchers. If using matchers results in a
@@ -592,20 +605,18 @@ auto buildTransferMatchSwitch(
// optional::value
.CaseOf<CXXMemberCallExpr>(
- isOptionalMemberCallWithName("value", IgnorableOptional),
+ valueCall(IgnorableOptional),
[](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
LatticeTransferState &State) {
transferUnwrapCall(E, E->getImplicitObjectArgument(), State);
})
// optional::operator*, optional::operator->
- .CaseOf<CallExpr>(
- expr(anyOf(isOptionalOperatorCallWithName("*", IgnorableOptional),
- isOptionalOperatorCallWithName("->", IgnorableOptional))),
- [](const CallExpr *E, const MatchFinder::MatchResult &,
- LatticeTransferState &State) {
- transferUnwrapCall(E, E->getArg(0), State);
- })
+ .CaseOf<CallExpr>(valueOperatorCall(IgnorableOptional),
+ [](const CallExpr *E, const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ transferUnwrapCall(E, E->getArg(0), State);
+ })
// optional::has_value
.CaseOf<CXXMemberCallExpr>(isOptionalMemberCallWithName("has_value"),
@@ -653,6 +664,49 @@ auto buildTransferMatchSwitch(
.Build();
}
+std::vector<SourceLocation> diagnoseUnwrapCall(const Expr *UnwrapExpr,
+ const Expr *ObjectExpr,
+ const Environment &Env) {
+ if (auto *OptionalVal =
+ Env.getValue(*ObjectExpr, SkipPast::ReferenceThenPointer)) {
+ auto *Prop = OptionalVal->getProperty("has_value");
+ if (auto *HasValueVal = cast_or_null<BoolValue>(Prop)) {
+ if (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.
+ return {ObjectExpr->getBeginLoc()};
+}
+
+auto buildDiagnoseMatchSwitch(
+ const UncheckedOptionalAccessModelOptions &Options) {
+ // FIXME: Evaluate the efficiency of matchers. If using matchers results in a
+ // lot of duplicated work (e.g. string comparisons), consider providing APIs
+ // that avoid it through memoization.
+ auto IgnorableOptional = ignorableOptional(Options);
+ return MatchSwitchBuilder<const Environment, std::vector<SourceLocation>>()
+ // optional::value
+ .CaseOf<CXXMemberCallExpr>(
+ valueCall(IgnorableOptional),
+ [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
+ const Environment &Env) {
+ return diagnoseUnwrapCall(E, E->getImplicitObjectArgument(), Env);
+ })
+
+ // optional::operator*, optional::operator->
+ .CaseOf<CallExpr>(
+ valueOperatorCall(IgnorableOptional),
+ [](const CallExpr *E, const MatchFinder::MatchResult &,
+ const Environment &Env) {
+ return diagnoseUnwrapCall(E, E->getArg(0), Env);
+ })
+ .Build();
+}
+
} // namespace
ast_matchers::DeclarationMatcher
@@ -699,5 +753,14 @@ bool UncheckedOptionalAccessModel::merge(QualType Type, const Value &Val1,
return true;
}
+UncheckedOptionalAccessDiagnoser::UncheckedOptionalAccessDiagnoser(
+ UncheckedOptionalAccessModelOptions Options)
+ : DiagnoseMatchSwitch(buildDiagnoseMatchSwitch(Options)) {}
+
+std::vector<SourceLocation> UncheckedOptionalAccessDiagnoser::diagnose(
+ ASTContext &Context, const Stmt *Stmt, const Environment &Env) {
+ return DiagnoseMatchSwitch(*Stmt, Context, Env);
+}
+
} // namespace dataflow
} // namespace clang
diff --git a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
index ddd9daaff6bb5..4a35c39cf2fd5 100644
--- a/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ b/clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -326,9 +326,11 @@ TypeErasedDataflowAnalysisState transferBlock(
}
llvm::Expected<std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>>>
-runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
- TypeErasedDataflowAnalysis &Analysis,
- const Environment &InitEnv) {
+runTypeErasedDataflowAnalysis(
+ const ControlFlowContext &CFCtx, TypeErasedDataflowAnalysis &Analysis,
+ const Environment &InitEnv,
+ std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)>
+ PostVisitStmt) {
PostOrderCFGView POV(&CFCtx.getCFG());
ForwardDataflowWorklist Worklist(CFCtx.getCFG(), &POV);
@@ -387,6 +389,20 @@ runTypeErasedDataflowAnalysis(const ControlFlowContext &CFCtx,
// FIXME: Consider evaluating unreachable basic blocks (those that have a
// state set to `llvm::None` at this point) to also analyze dead code.
+ if (PostVisitStmt) {
+ for (const CFGBlock *Block : CFCtx.getCFG()) {
+ // Skip blocks that were not evaluated.
+ if (!BlockStates[Block->getBlockID()])
+ continue;
+ transferBlock(
+ CFCtx, BlockStates, *Block, InitEnv, Analysis,
+ [&PostVisitStmt](const clang::CFGStmt &Stmt,
+ const TypeErasedDataflowAnalysisState &State) {
+ PostVisitStmt(Stmt.getStmt(), State);
+ });
+ }
+ }
+
return BlockStates;
}
diff --git a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
index ce439f5613028..3d873eceeba3f 100644
--- a/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
+++ b/clang/unittests/Analysis/FlowSensitive/TestingSupport.h
@@ -62,22 +62,25 @@ llvm::Expected<llvm::DenseMap<const Stmt *, std::string>>
buildStatementToAnnotationMapping(const FunctionDecl *Func,
llvm::Annotations AnnotatedCode);
-// Runs dataflow on the body of the function that matches `func_matcher` in code
-// snippet `code`. Requires: `Analysis` contains a type `Lattice`.
+struct AnalysisData {
+ ASTContext &ASTCtx;
+ const ControlFlowContext &CFCtx;
+ const Environment &Env;
+ TypeErasedDataflowAnalysis &Analysis;
+ llvm::DenseMap<const clang::Stmt *, std::string> &Annotations;
+ std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>> &BlockStates;
+};
+
template <typename AnalysisT>
llvm::Error checkDataflow(
llvm::StringRef Code,
- ast_matchers::internal::Matcher<FunctionDecl> FuncMatcher,
+ ast_matchers::internal::Matcher<FunctionDecl> TargetFuncMatcher,
std::function<AnalysisT(ASTContext &, Environment &)> MakeAnalysis,
- std::function<void(
- llvm::ArrayRef<std::pair<
- std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>,
- ASTContext &)>
- Expectations,
- ArrayRef<std::string> Args,
+ std::function<void(ASTContext &, const Stmt *,
+ const TypeErasedDataflowAnalysisState &)>
+ PostVisitStmt,
+ std::function<void(AnalysisData)> VerifyResults, ArrayRef<std::string> Args,
const tooling::FileContentMappings &VirtualMappedFiles = {}) {
- using StateT = DataflowAnalysisState<typename AnalysisT::Lattice>;
-
llvm::Annotations AnnotatedCode(Code);
auto Unit = tooling::buildASTFromCodeWithArgs(
AnnotatedCode.code(), Args, "input.cc", "clang-dataflow-test",
@@ -93,10 +96,10 @@ llvm::Error checkDataflow(
const FunctionDecl *F = ast_matchers::selectFirst<FunctionDecl>(
"target",
- ast_matchers::match(
- ast_matchers::functionDecl(ast_matchers::isDefinition(), FuncMatcher)
- .bind("target"),
- Context));
+ ast_matchers::match(ast_matchers::functionDecl(
+ ast_matchers::isDefinition(), TargetFuncMatcher)
+ .bind("target"),
+ Context));
if (F == nullptr)
return llvm::make_error<llvm::StringError>(
llvm::errc::invalid_argument, "Could not find target function.");
@@ -109,6 +112,16 @@ llvm::Error checkDataflow(
Environment Env(DACtx, *F);
auto Analysis = MakeAnalysis(Context, Env);
+ std::function<void(const Stmt *, const TypeErasedDataflowAnalysisState &)>
+ PostVisitStmtClosure = nullptr;
+ if (PostVisitStmt != nullptr) {
+ PostVisitStmtClosure = [&PostVisitStmt, &Context](
+ const Stmt *Stmt,
+ const TypeErasedDataflowAnalysisState &State) {
+ PostVisitStmt(Context, Stmt, State);
+ };
+ }
+
llvm::Expected<llvm::DenseMap<const clang::Stmt *, std::string>>
StmtToAnnotations = buildStatementToAnnotationMapping(F, AnnotatedCode);
if (!StmtToAnnotations)
@@ -116,40 +129,72 @@ llvm::Error checkDataflow(
auto &Annotations = *StmtToAnnotations;
llvm::Expected<std::vector<llvm::Optional<TypeErasedDataflowAnalysisState>>>
- MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env);
+ MaybeBlockStates = runTypeErasedDataflowAnalysis(*CFCtx, Analysis, Env,
+ PostVisitStmtClosure);
if (!MaybeBlockStates)
return MaybeBlockStates.takeError();
auto &BlockStates = *MaybeBlockStates;
- if (BlockStates.empty()) {
- Expectations({}, Context);
- return llvm::Error::success();
- }
-
- // Compute a map from statement annotations to the state computed for
- // the program point immediately after the annotated statement.
- std::vector<std::pair<std::string, StateT>> Results;
- for (const CFGBlock *Block : CFCtx->getCFG()) {
- // Skip blocks that were not evaluated.
- if (!BlockStates[Block->getBlockID()])
- continue;
-
- transferBlock(
- *CFCtx, BlockStates, *Block, Env, Analysis,
- [&Results, &Annotations](const clang::CFGStmt &Stmt,
- const TypeErasedDataflowAnalysisState &State) {
- auto It = Annotations.find(Stmt.getStmt());
- if (It == Annotations.end())
- return;
- auto *Lattice =
- llvm::any_cast<typename AnalysisT::Lattice>(&State.Lattice.Value);
- Results.emplace_back(It->second, StateT{*Lattice, State.Env});
- });
- }
- Expectations(Results, Context);
+ AnalysisData AnalysisData{Context, *CFCtx, Env,
+ Analysis, Annotations, BlockStates};
+ VerifyResults(AnalysisData);
return llvm::Error::success();
}
+// Runs dataflow on the body of the function that matches `TargetFuncMatcher` in
+// code snippet `Code`. Requires: `AnalysisT` contains a type `Lattice`.
+template <typename AnalysisT>
+llvm::Error checkDataflow(
+ llvm::StringRef Code,
+ ast_matchers::internal::Matcher<FunctionDecl> TargetFuncMatcher,
+ std::function<AnalysisT(ASTContext &, Environment &)> MakeAnalysis,
+ std::function<void(
+ llvm::ArrayRef<std::pair<
+ std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>,
+ ASTContext &)>
+ VerifyResults,
+ ArrayRef<std::string> Args,
+ const tooling::FileContentMappings &VirtualMappedFiles = {}) {
+ using StateT = DataflowAnalysisState<typename AnalysisT::Lattice>;
+
+ return checkDataflow(
+ Code, std::move(TargetFuncMatcher), std::move(MakeAnalysis),
+ /*PostVisitStmt=*/nullptr,
+ [&VerifyResults](AnalysisData AnalysisData) {
+ if (AnalysisData.BlockStates.empty()) {
+ VerifyResults({}, AnalysisData.ASTCtx);
+ return;
+ }
+
+ auto &Annotations = AnalysisData.Annotations;
+
+ // Compute a map from statement annotations to the state computed for
+ // the program point immediately after the annotated statement.
+ std::vector<std::pair<std::string, StateT>> Results;
+ for (const CFGBlock *Block : AnalysisData.CFCtx.getCFG()) {
+ // Skip blocks that were not evaluated.
+ if (!AnalysisData.BlockStates[Block->getBlockID()])
+ continue;
+
+ transferBlock(
+ AnalysisData.CFCtx, AnalysisData.BlockStates, *Block,
+ AnalysisData.Env, AnalysisData.Analysis,
+ [&Results,
+ &Annotations](const clang::CFGStmt &Stmt,
+ const TypeErasedDataflowAnalysisState &State) {
+ auto It = Annotations.find(Stmt.getStmt());
+ if (It == Annotations.end())
+ return;
+ auto *Lattice = llvm::any_cast<typename AnalysisT::Lattice>(
+ &State.Lattice.Value);
+ Results.emplace_back(It->second, StateT{*Lattice, State.Env});
+ });
+ }
+ VerifyResults(Results, AnalysisData.ASTCtx);
+ },
+ Args, VirtualMappedFiles);
+}
+
// Runs dataflow on the body of the function named `target_fun` in code snippet
// `code`.
template <typename AnalysisT>
@@ -160,11 +205,11 @@ llvm::Error checkDataflow(
llvm::ArrayRef<std::pair<
std::string, DataflowAnalysisState<typename AnalysisT::Lattice>>>,
ASTContext &)>
- Expectations,
+ VerifyResults,
ArrayRef<std::string> Args,
const tooling::FileContentMappings &VirtualMappedFiles = {}) {
return checkDataflow(Code, ast_matchers::hasName(TargetFun),
- std::move(MakeAnalysis), std::move(Expectations), Args,
+ std::move(MakeAnalysis), std::move(VerifyResults), Args,
VirtualMappedFiles);
}
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
index 912754ebe8b6e..63f8ed344c061 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -11,9 +11,12 @@
#include "TestingSupport.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchers.h"
-#include "clang/Analysis/FlowSensitive/SourceLocationsLattice.h"
+#include "clang/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.h"
+#include "clang/Basic/SourceLocation.h"
#include "clang/Tooling/Tooling.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
#include "llvm/Support/Error.h"
#include "gmock/gmock.h"
@@ -26,8 +29,7 @@ using namespace clang;
using namespace dataflow;
using namespace test;
-using ::testing::Pair;
-using ::testing::UnorderedElementsAre;
+using ::testing::ContainerEq;
// FIXME: Move header definitions in separate file(s).
static constexpr char CSDtdDefHeader[] = R"(
@@ -1180,13 +1182,6 @@ constexpr Optional<T> make_optional(std::initializer_list<U> il,
} // namespace base
)";
-/// Converts `L` to string.
-static std::string ConvertToString(const SourceLocationsLattice &L,
- const ASTContext &Ctx) {
- return L.getSourceLocations().empty() ? "safe"
- : "unsafe: " + DebugString(L, Ctx);
-}
-
/// Replaces all occurrences of `Pattern` in `S` with `Replacement`.
static void ReplaceAllOccurrences(std::string &S, const std::string &Pattern,
const std::string &Replacement) {
@@ -1207,18 +1202,14 @@ struct OptionalTypeIdentifier {
class UncheckedOptionalAccessTest
: public ::testing::TestWithParam<OptionalTypeIdentifier> {
protected:
- template <typename LatticeChecksMatcher>
- void ExpectLatticeChecksFor(std::string SourceCode,
- LatticeChecksMatcher MatchesLatticeChecks) {
- ExpectLatticeChecksFor(SourceCode, ast_matchers::hasName("target"),
- MatchesLatticeChecks);
+ void ExpectDiagnosticsFor(std::string SourceCode) {
+ ExpectDiagnosticsFor(SourceCode, ast_matchers::hasName("target"));
}
private:
- template <typename FuncDeclMatcher, typename LatticeChecksMatcher>
- void ExpectLatticeChecksFor(std::string SourceCode,
- FuncDeclMatcher FuncMatcher,
- LatticeChecksMatcher MatchesLatticeChecks) {
+ template <typename FuncDeclMatcher>
+ void ExpectDiagnosticsFor(std::string SourceCode,
+ FuncDeclMatcher FuncMatcher) {
ReplaceAllOccurrences(SourceCode, "$ns", GetParam().NamespaceName);
ReplaceAllOccurrences(SourceCode, "$optional", GetParam().TypeName);
@@ -1245,27 +1236,44 @@ class UncheckedOptionalAccessTest
)");
const tooling::FileContentMappings FileContents(Headers.begin(),
Headers.end());
+ UncheckedOptionalAccessModelOptions Options{
+ /*IgnoreSmartPointerDereference=*/true};
+ std::vector<SourceLocation> Diagnostics;
llvm::Error Error = checkDataflow<UncheckedOptionalAccessModel>(
SourceCode, FuncMatcher,
- [](ASTContext &Ctx, Environment &) {
- return UncheckedOptionalAccessModel(
- Ctx, UncheckedOptionalAccessModelOptions{
- /*IgnoreSmartPointerDereference=*/true});
+ [Options](ASTContext &Ctx, Environment &) {
+ return UncheckedOptionalAccessModel(Ctx, Options);
+ },
+ [&Diagnostics, Diagnoser = UncheckedOptionalAccessDiagnoser(Options)](
+ ASTContext &Ctx, const Stmt *Stmt,
+ const TypeErasedDataflowAnalysisState &State) mutable {
+ auto StmtDiagnostics = Diagnoser.diagnose(Ctx, Stmt, State.Env);
+ llvm::move(StmtDiagnostics, std::back_inserter(Diagnostics));
},
- [&MatchesLatticeChecks](
- llvm::ArrayRef<std::pair<
- std::string, DataflowAnalysisState<SourceLocationsLattice>>>
- CheckToLatticeMap,
- ASTContext &Ctx) {
- // FIXME: Consider using a matcher instead of translating
- // `CheckToLatticeMap` to `CheckToStringifiedLatticeMap`.
- std::vector<std::pair<std::string, std::string>>
- CheckToStringifiedLatticeMap;
- for (const auto &E : CheckToLatticeMap) {
- CheckToStringifiedLatticeMap.emplace_back(
- E.first, ConvertToString(E.second.Lattice, Ctx));
+ [&Diagnostics](AnalysisData AnalysisData) {
+ auto &SrcMgr = AnalysisData.ASTCtx.getSourceManager();
+
+ llvm::DenseSet<unsigned> AnnotationLines;
+ for (const auto &Pair : AnalysisData.Annotations) {
+ auto *Stmt = Pair.getFirst();
+ AnnotationLines.insert(
+ SrcMgr.getPresumedLineNumber(Stmt->getBeginLoc()));
+ // We add both the begin and end locations, so that if the
+ // statement spans multiple lines then the test will fail.
+ //
+ // FIXME: Going forward, we should change this to instead just
+ // get the single line number from the annotation itself, rather
+ // than looking at the statement it's attached to.
+ AnnotationLines.insert(
+ SrcMgr.getPresumedLineNumber(Stmt->getEndLoc()));
+ }
+
+ llvm::DenseSet<unsigned> DiagnosticLines;
+ for (SourceLocation &Loc : Diagnostics) {
+ DiagnosticLines.insert(SrcMgr.getPresumedLineNumber(Loc));
}
- EXPECT_THAT(CheckToStringifiedLatticeMap, MatchesLatticeChecks);
+
+ EXPECT_THAT(DiagnosticLines, ContainerEq(AnnotationLines));
},
{"-fsyntax-only", "-std=c++17", "-Wno-undefined-inline"}, FileContents);
if (Error)
@@ -1283,65 +1291,55 @@ INSTANTIATE_TEST_SUITE_P(
});
TEST_P(UncheckedOptionalAccessTest, EmptyFunctionBody) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
void target() {
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingValueNoCheck) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- std::move(opt).value();
- /*[[check]]*/
+ std::move(opt).value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorStarNoCheck) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- *opt;
- /*[[check]]*/
+ *opt; // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- *std::move(opt);
- /*[[check]]*/
+ *std::move(opt); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:8")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorArrowNoCheck) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1350,13 +1348,11 @@ TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorArrowNoCheck) {
};
void target($ns::$optional<Foo> opt) {
- opt->foo();
- /*[[check]]*/
+ opt->foo(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1365,107 +1361,91 @@ TEST_P(UncheckedOptionalAccessTest, UnwrapUsingOperatorArrowNoCheck) {
};
void target($ns::$optional<Foo> opt) {
- std::move(opt)->foo();
- /*[[check]]*/
+ std::move(opt)->foo(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:9:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, HasValueCheck) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
if (opt.has_value()) {
opt.value();
- /*[[check]]*/
}
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OperatorBoolCheck) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
if (opt) {
opt.value();
- /*[[check]]*/
}
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UnwrapFunctionCallResultNoCheck) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
- Make<$ns::$optional<int>>().value();
+ Make<$ns::$optional<int>>().value(); // [[unsafe]]
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
- std::move(opt).value();
- /*[[check]]*/
+ std::move(opt).value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:5:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, DefaultConstructor) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt;
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, NulloptConstructor) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt($ns::nullopt);
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt($ns::in_place, 3);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1473,12 +1453,10 @@ TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
void target() {
$ns::$optional<Foo> opt($ns::in_place);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1488,12 +1466,10 @@ TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
void target() {
$ns::$optional<Foo> opt($ns::in_place, 3, false);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1503,46 +1479,38 @@ TEST_P(UncheckedOptionalAccessTest, InPlaceConstructor) {
void target() {
$ns::$optional<Foo> opt($ns::in_place, {3});
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, ValueConstructor) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt(21);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = $ns::$optional<int>(21);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
- ExpectLatticeChecksFor(R"(
+ )");
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<$ns::$optional<int>> opt(Make<$ns::$optional<int>>());
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct MyString {
@@ -1552,12 +1520,10 @@ TEST_P(UncheckedOptionalAccessTest, ValueConstructor) {
void target() {
$ns::$optional<MyString> opt("foo");
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1569,12 +1535,10 @@ TEST_P(UncheckedOptionalAccessTest, ValueConstructor) {
void target() {
$ns::$optional<Bar> opt(Make<Foo>());
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1584,14 +1548,12 @@ TEST_P(UncheckedOptionalAccessTest, ValueConstructor) {
void target() {
$ns::$optional<Foo> opt(3);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, ConvertibleOptionalConstructor) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1603,13 +1565,11 @@ TEST_P(UncheckedOptionalAccessTest, ConvertibleOptionalConstructor) {
void target() {
$ns::$optional<Bar> opt(Make<$ns::$optional<Foo>>());
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1621,13 +1581,11 @@ TEST_P(UncheckedOptionalAccessTest, ConvertibleOptionalConstructor) {
void target() {
$ns::$optional<Bar> opt(Make<$ns::$optional<Foo>>());
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -1640,13 +1598,11 @@ TEST_P(UncheckedOptionalAccessTest, ConvertibleOptionalConstructor) {
void target() {
$ns::$optional<Foo> opt1 = $ns::nullopt;
$ns::$optional<Bar> opt2(opt1);
- opt2.value();
- /*[[check]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:13:7")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1659,12 +1615,10 @@ TEST_P(UncheckedOptionalAccessTest, ConvertibleOptionalConstructor) {
$ns::$optional<Foo> opt1(Make<Foo>());
$ns::$optional<Bar> opt2(opt1);
opt2.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1677,25 +1631,21 @@ TEST_P(UncheckedOptionalAccessTest, ConvertibleOptionalConstructor) {
$ns::$optional<Foo> opt1(Make<Foo>());
$ns::$optional<Bar> opt2(opt1);
opt2.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, MakeOptional) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = $ns::make_optional(0);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1705,12 +1655,10 @@ TEST_P(UncheckedOptionalAccessTest, MakeOptional) {
void target() {
$ns::$optional<Foo> opt = $ns::make_optional<Foo>(21, 22);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {
@@ -1721,104 +1669,83 @@ TEST_P(UncheckedOptionalAccessTest, MakeOptional) {
char a = 'a';
$ns::$optional<Foo> opt = $ns::make_optional<Foo>({a});
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, ValueOr) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt;
opt.value_or(0);
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
// Pointers.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int*> opt) {
if (opt.value_or(nullptr) != nullptr) {
opt.value();
- /*[[check-ptrs-1]]*/
} else {
- opt.value();
- /*[[check-ptrs-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-ptrs-1", "safe"),
- Pair("check-ptrs-2", "unsafe: input.cc:9:9")));
+ )code");
// Integers.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> opt) {
if (opt.value_or(0) != 0) {
opt.value();
- /*[[check-ints-1]]*/
} else {
- opt.value();
- /*[[check-ints-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-ints-1", "safe"),
- Pair("check-ints-2", "unsafe: input.cc:9:9")));
+ )code");
// Strings.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<std::string> opt) {
if (!opt.value_or("").empty()) {
opt.value();
- /*[[check-strings-1]]*/
} else {
- opt.value();
- /*[[check-strings-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-strings-1", "safe"),
- Pair("check-strings-2", "unsafe: input.cc:9:9")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<std::string> opt) {
if (opt.value_or("") != "") {
opt.value();
- /*[[check-strings-neq-1]]*/
} else {
- opt.value();
- /*[[check-strings-neq-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(
- Pair("check-strings-neq-1", "safe"),
- Pair("check-strings-neq-2", "unsafe: input.cc:9:9")));
+ )code");
// Pointer-to-optional.
//
// FIXME: make `opt` a parameter directly, once we ensure that all `optional`
// values have a `has_value` property.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -1826,43 +1753,35 @@ TEST_P(UncheckedOptionalAccessTest, ValueOrComparison) {
$ns::$optional<int> *opt = &p;
if (opt->value_or(0) != 0) {
opt->value();
- /*[[check-pto-1]]*/
} else {
- opt->value();
- /*[[check-pto-2]]*/
+ opt->value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-pto-1", "safe"),
- Pair("check-pto-2", "unsafe: input.cc:10:9")));
+ )code");
}
TEST_P(UncheckedOptionalAccessTest, Emplace) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt;
opt.emplace(0);
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> *opt) {
opt->emplace(0);
opt->value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
// FIXME: Add tests that call `emplace` in conditional branches:
- // ExpectLatticeChecksFor(
+ // ExpectDiagnosticsFor(
// R"(
// #include "unchecked_optional_access_test.h"
//
@@ -1872,47 +1791,39 @@ TEST_P(UncheckedOptionalAccessTest, Emplace) {
// }
// if (b) {
// opt.value();
- // /*[[check-1]]*/
// } else {
- // opt.value();
- // /*[[check-2]]*/
+ // opt.value(); // [[unsafe]]
// }
// }
- // )",
- // UnorderedElementsAre(Pair("check-1", "safe"),
- // Pair("check-2", "unsafe: input.cc:12:9")));
+ // )");
}
TEST_P(UncheckedOptionalAccessTest, Reset) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = $ns::make_optional(0);
opt.reset();
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target($ns::$optional<int> &opt) {
if (opt.has_value()) {
opt.reset();
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:9")));
+ )");
// FIXME: Add tests that call `reset` in conditional branches:
- // ExpectLatticeChecksFor(
+ // ExpectDiagnosticsFor(
// R"(
// #include "unchecked_optional_access_test.h"
//
@@ -1922,20 +1833,16 @@ TEST_P(UncheckedOptionalAccessTest, Reset) {
// opt.reset();
// }
// if (b) {
- // opt.value();
- // /*[[check-1]]*/
+ // opt.value(); // [[unsafe]]
// } else {
// opt.value();
- // /*[[check-2]]*/
// }
// }
- // )",
- // UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:10:9"),
- // Pair("check-2", "safe")));
+ // )");
}
TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1944,12 +1851,10 @@ TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
$ns::$optional<Foo> opt;
opt = Foo();
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct Foo {};
@@ -1958,12 +1863,10 @@ TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
$ns::$optional<Foo> opt;
(opt = Foo()).value();
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct MyString {
@@ -1974,12 +1877,10 @@ TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
$ns::$optional<MyString> opt;
opt = "foo";
opt.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
struct MyString {
@@ -1989,14 +1890,12 @@ TEST_P(UncheckedOptionalAccessTest, ValueAssignment) {
void target() {
$ns::$optional<MyString> opt;
(opt = "foo").value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OptionalConversionAssignment) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2011,12 +1910,10 @@ TEST_P(UncheckedOptionalAccessTest, OptionalConversionAssignment) {
$ns::$optional<Bar> opt2;
opt2 = opt1;
opt2.value();
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2031,14 +1928,12 @@ TEST_P(UncheckedOptionalAccessTest, OptionalConversionAssignment) {
$ns::$optional<Bar> opt2;
if (opt2.has_value()) {
opt2 = opt1;
- opt2.value();
- /*[[check]]*/
+ opt2.value(); // [[unsafe]]
}
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:15:9")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2053,41 +1948,35 @@ TEST_P(UncheckedOptionalAccessTest, OptionalConversionAssignment) {
$ns::$optional<Bar> opt2;
(opt2 = opt1).value();
(void)0;
- /*[[check]]*/
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, NulloptAssignment) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
opt = $ns::nullopt;
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:7:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
- (opt = $ns::nullopt).value();
- /*[[check]]*/
+ (opt = $ns::nullopt).value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:6:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OptionalSwap) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2098,16 +1987,12 @@ TEST_P(UncheckedOptionalAccessTest, OptionalSwap) {
opt1.swap(opt2);
opt1.value();
- /*[[check-1]]*/
- opt2.value();
- /*[[check-2]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:13:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2118,18 +2003,14 @@ TEST_P(UncheckedOptionalAccessTest, OptionalSwap) {
opt2.swap(opt1);
opt1.value();
- /*[[check-3]]*/
- opt2.value();
- /*[[check-4]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-3", "safe"),
- Pair("check-4", "unsafe: input.cc:13:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, StdSwap) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2140,16 +2021,12 @@ TEST_P(UncheckedOptionalAccessTest, StdSwap) {
std::swap(opt1, opt2);
opt1.value();
- /*[[check-1]]*/
- opt2.value();
- /*[[check-2]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:13:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2160,20 +2037,16 @@ TEST_P(UncheckedOptionalAccessTest, StdSwap) {
std::swap(opt2, opt1);
opt1.value();
- /*[[check-3]]*/
- opt2.value();
- /*[[check-4]]*/
+ opt2.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-3", "safe"),
- Pair("check-4", "unsafe: input.cc:13:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, UniquePtrToStructWithOptionalField) {
// We suppress diagnostics for values reachable from smart pointers (other
// than `optional` itself).
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2190,16 +2063,13 @@ TEST_P(UncheckedOptionalAccessTest, UniquePtrToStructWithOptionalField) {
void target() {
smart_ptr<Foo> foo;
*foo->opt;
- /*[[check-1]]*/
*(*foo).opt;
- /*[[check-2]]*/
}
- )",
- UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2208,12 +2078,10 @@ TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
void target() {
$ns::$optional<int> opt = 0;
opt = MakeOpt();
- opt.value();
- /*[[check-1]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-1", "unsafe: input.cc:9:7")));
- ExpectLatticeChecksFor(
+ )");
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2222,13 +2090,11 @@ TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
void target() {
$ns::$optional<int> opt = 0;
opt = MakeOpt();
- opt.value();
- /*[[check-2]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-2", "unsafe: input.cc:9:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2238,13 +2104,11 @@ TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
void target() {
IntOpt opt = 0;
opt = MakeOpt();
- opt.value();
- /*[[check-3]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:10:7")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2254,16 +2118,14 @@ TEST_P(UncheckedOptionalAccessTest, CallReturningOptional) {
void target() {
IntOpt opt = 0;
opt = MakeOpt();
- opt.value();
- /*[[check-4]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:10:7")));
+ )");
}
// Verifies that the model sees through aliases.
TEST_P(UncheckedOptionalAccessTest, WithAlias) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2271,18 +2133,16 @@ TEST_P(UncheckedOptionalAccessTest, WithAlias) {
using MyOptional = $ns::$optional<T>;
void target(MyOptional<int> opt) {
- opt.value();
- /*[[check]]*/
+ opt.value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:8:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OptionalValueOptional) {
// Basic test that nested values are populated. We nest an optional because
// its easy to use in a test, but the type of the nested value shouldn't
// matter.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2291,14 +2151,12 @@ TEST_P(UncheckedOptionalAccessTest, OptionalValueOptional) {
void target($ns::$optional<Foo> foo) {
if (foo && *foo) {
foo->value();
- /*[[access]]*/
}
}
- )",
- UnorderedElementsAre(Pair("access", "safe")));
+ )");
// Mutation is supported for nested values.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2307,18 +2165,16 @@ TEST_P(UncheckedOptionalAccessTest, OptionalValueOptional) {
void target($ns::$optional<Foo> foo) {
if (foo && *foo) {
foo->reset();
- foo->value();
- /*[[reset]]*/
+ foo->value(); // [[unsafe]]
}
}
- )",
- UnorderedElementsAre(Pair("reset", "unsafe: input.cc:9:9")));
+ )");
}
// Tests that structs can be nested. We use an optional field because its easy
// to use in a test, but the type of the field shouldn't matter.
TEST_P(UncheckedOptionalAccessTest, OptionalValueStruct) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2329,18 +2185,16 @@ TEST_P(UncheckedOptionalAccessTest, OptionalValueStruct) {
void target($ns::$optional<Foo> foo) {
if (foo && foo->opt) {
foo->opt.value();
- /*[[access]]*/
}
}
- )",
- UnorderedElementsAre(Pair("access", "safe")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, OptionalValueInitialization) {
// FIXME: Fix when to initialize `value`. All unwrapping should be safe in
// this example, but `value` initialization is done multiple times during the
// fixpoint iterations and joining the environment won't correctly merge them.
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2359,15 +2213,13 @@ TEST_P(UncheckedOptionalAccessTest, OptionalValueInitialization) {
}
// Now we merge the two values. UncheckedOptionalAccessModel::merge() will
// throw away the "value" property.
- foo->value();
- /*[[merge]]*/
+ foo->value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("merge", "unsafe: input.cc:19:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, AssignThroughLvalueReferencePtr) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
@@ -2379,56 +2231,48 @@ TEST_P(UncheckedOptionalAccessTest, AssignThroughLvalueReferencePtr) {
void target() {
smart_ptr<$ns::$optional<float>> x;
*x = $ns::nullopt;
- (*x).value();
- /*[[check]]*/
+ (*x).value(); // [[unsafe]]
}
- )",
- UnorderedElementsAre(Pair("check", "unsafe: input.cc:12:7")));
+ )");
}
TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) {
- ExpectLatticeChecksFor(R"code(
+ ExpectDiagnosticsFor(R"code(
#include "unchecked_optional_access_test.h"
void target(bool b, $ns::$optional<int> opt) {
if (b || opt.has_value()) {
if (!b) {
opt.value();
- /*[[check-1]]*/
}
}
}
- )code",
- UnorderedElementsAre(Pair("check-1", "safe")));
+ )code");
- ExpectLatticeChecksFor(R"code(
+ ExpectDiagnosticsFor(R"code(
#include "unchecked_optional_access_test.h"
void target(bool b, $ns::$optional<int> opt) {
if (b && !opt.has_value()) return;
if (b) {
opt.value();
- /*[[check-2]]*/
}
}
- )code",
- UnorderedElementsAre(Pair("check-2", "safe")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
void target(bool b, $ns::$optional<int> opt) {
if (opt.has_value()) b = true;
if (b) {
- opt.value();
- /*[[check-3]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+ )code");
- ExpectLatticeChecksFor(R"code(
+ ExpectDiagnosticsFor(R"code(
#include "unchecked_optional_access_test.h"
void target(bool b, $ns::$optional<int> opt) {
@@ -2436,41 +2280,35 @@ TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) {
if (opt.has_value()) b = true;
if (b) {
opt.value();
- /*[[check-4]]*/
}
}
- )code",
- UnorderedElementsAre(Pair("check-4", "safe")));
+ )code");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target(bool b, $ns::$optional<int> opt) {
if (opt.has_value() == b) {
if (b) {
opt.value();
- /*[[check-5]]*/
}
}
}
- )",
- UnorderedElementsAre(Pair("check-5", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target(bool b, $ns::$optional<int> opt) {
if (opt.has_value() != b) {
if (!b) {
opt.value();
- /*[[check-6]]*/
}
}
}
- )",
- UnorderedElementsAre(Pair("check-6", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target(bool b) {
@@ -2483,30 +2321,26 @@ TEST_P(UncheckedOptionalAccessTest, CorrelatedBranches) {
}
if (opt2.has_value()) {
opt1.value();
- /*[[check]]*/
}
}
- )",
- UnorderedElementsAre(Pair("check", "safe")));
+ )");
// FIXME: Add support for operator==.
- // ExpectLatticeChecksFor(R"(
+ // ExpectDiagnosticsFor(R"(
// #include "unchecked_optional_access_test.h"
//
// void target($ns::$optional<int> opt1, $ns::$optional<int> opt2) {
// if (opt1 == opt2) {
// if (opt1.has_value()) {
// opt2.value();
- // /*[[check-7]]*/
// }
// }
// }
- // )",
- // UnorderedElementsAre(Pair("check-7", "safe")));
+ // )");
}
TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -2519,17 +2353,13 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
}
if (opt.has_value()) {
opt.value();
- /*[[check-1]]*/
} else {
- opt.value();
- /*[[check-2]]*/
+ opt.value(); // [[unsafe]]
}
}
- )code",
- UnorderedElementsAre(Pair("check-1", "safe"),
- Pair("check-2", "unsafe: input.cc:15:9")));
+ )code");
- ExpectLatticeChecksFor(R"code(
+ ExpectDiagnosticsFor(R"code(
#include "unchecked_optional_access_test.h"
void target(bool b) {
@@ -2542,12 +2372,10 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
if (!opt.has_value()) return;
}
opt.value();
- /*[[check-3]]*/
}
- )code",
- UnorderedElementsAre(Pair("check-3", "safe")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -2559,13 +2387,11 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
} else {
opt = Make<$ns::$optional<int>>();
}
- opt.value();
- /*[[check-4]]*/
+ opt.value(); // [[unsafe]]
}
- )code",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:12:7")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -2577,12 +2403,10 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
opt = 2;
}
opt.value();
- /*[[check-5]]*/
}
- )code",
- UnorderedElementsAre(Pair("check-5", "safe")));
+ )code");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"code(
#include "unchecked_optional_access_test.h"
@@ -2593,75 +2417,65 @@ TEST_P(UncheckedOptionalAccessTest, JoinDistinctValues) {
} else {
opt = Make<$ns::$optional<int>>();
}
- opt.value();
- /*[[check-6]]*/
+ opt.value(); // [[unsafe]]
}
- )code",
- UnorderedElementsAre(Pair("check-6", "unsafe: input.cc:11:7")));
+ )code");
}
TEST_P(UncheckedOptionalAccessTest, ReassignValueInLoop) {
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
while (Make<bool>()) {
opt.value();
- /*[[check-1]]*/
}
}
- )",
- UnorderedElementsAre(Pair("check-1", "safe")));
+ )");
- ExpectLatticeChecksFor(R"(
+ ExpectDiagnosticsFor(R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
while (Make<bool>()) {
opt.value();
- /*[[check-2]]*/
opt = Make<$ns::$optional<int>>();
if (!opt.has_value()) return;
}
}
- )",
- UnorderedElementsAre(Pair("check-2", "safe")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
while (Make<bool>()) {
- opt.value();
- /*[[check-3]]*/
+ opt.value(); // [[unsafe]]
opt = Make<$ns::$optional<int>>();
}
}
- )",
- UnorderedElementsAre(Pair("check-3", "unsafe: input.cc:7:9")));
+ )");
- ExpectLatticeChecksFor(
+ ExpectDiagnosticsFor(
R"(
#include "unchecked_optional_access_test.h"
void target() {
$ns::$optional<int> opt = 3;
while (Make<bool>()) {
- opt.value();
- /*[[check-4]]*/
+ opt.value(); // [[unsafe]]
opt = Make<$ns::$optional<int>>();
if (!opt.has_value()) continue;
}
}
- )",
- UnorderedElementsAre(Pair("check-4", "unsafe: input.cc:7:9")));
+ )");
}
// FIXME: Add support for:
More information about the cfe-commits
mailing list