[llvm-branch-commits] [FlowSensitive] [StatusOr] [13/N] Add support for gtest ASSERTs (PR #170947)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Fri Dec 5 15:17:49 PST 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Florian Mayer (fmayer)
<details>
<summary>Changes</summary>
---
Full diff: https://github.com/llvm/llvm-project/pull/170947.diff
2 Files Affected:
- (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp (+221)
- (modified) clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp (+175)
``````````diff
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
index 3e56094fcbc32..1b68d704239e8 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedStatusOrAccessModel.cpp
@@ -281,6 +281,76 @@ static auto isNonConstMemberOperatorCall() {
return cxxOperatorCallExpr(callee(cxxMethodDecl(unless(isConst()))));
}
+static auto isMakePredicateFormatterFromIsOkMatcherCall() {
+ using namespace ::clang::ast_matchers; // NOLINT: Too many names
+ return callExpr(
+ callee(functionDecl(
+ hasName("::testing::internal::MakePredicateFormatterFromMatcher"))),
+ hasArgument(
+ 0, hasType(cxxRecordDecl(hasAnyName(
+ "::testing::status::internal_status::IsOkMatcher",
+ "::absl_testing::status_internal::IsOkMatcher",
+ "::testing::status::internal_status::IsOkAndHoldsMatcher",
+ "::absl_testing::status_internal::IsOkAndHoldsMatcher")))));
+}
+
+static auto isStatusIsOkMatcherCall() {
+ using namespace ::clang::ast_matchers; // NOLINT: Too many names
+ return callExpr(callee(functionDecl(hasAnyName(
+ "::testing::status::StatusIs", "absl_testing::StatusIs",
+ "::testing::status::CanonicalStatusIs",
+ "::absl_testing::CanonicalStatusIs"))),
+ hasArgument(0, declRefExpr(to(enumConstantDecl(hasAnyName(
+ "::absl::StatusCode::kOk", "OK"))))));
+}
+
+static auto isMakePredicateFormatterFromStatusIsMatcherCall() {
+ using namespace ::clang::ast_matchers; // NOLINT: Too many names
+ return callExpr(
+ callee(functionDecl(
+ hasName("::testing::internal::MakePredicateFormatterFromMatcher"))),
+ hasArgument(0, hasType(cxxRecordDecl(hasAnyName(
+ "::testing::status::internal_status::StatusIsMatcher",
+ "::testing::status::internal_status::"
+ "CanonicalStatusIsMatcher",
+ "::absl_testing::status_internal::StatusIsMatcher",
+ "::absl_testing::status_internal::"
+ "CanonicalStatusIsMatcher")))));
+}
+
+static auto isPredicateFormatterFromStatusMatcherCall() {
+ using namespace ::clang::ast_matchers; // NOLINT: Too many names
+ return cxxOperatorCallExpr(
+ hasOverloadedOperatorName("()"),
+ callee(cxxMethodDecl(ofClass(
+ hasName("testing::internal::PredicateFormatterFromMatcher")))),
+ hasArgument(2, hasType(cxxRecordDecl(hasName("absl::Status")))));
+}
+
+static auto isPredicateFormatterFromStatusOrMatcherCall() {
+ using namespace ::clang::ast_matchers; // NOLINT: Too many names
+ return cxxOperatorCallExpr(
+ hasOverloadedOperatorName("()"),
+ callee(cxxMethodDecl(ofClass(
+ hasName("testing::internal::PredicateFormatterFromMatcher")))),
+ hasArgument(2, hasType(statusOrType())));
+}
+
+static auto isAssertionResultOperatorBoolCall() {
+ using namespace ::clang::ast_matchers; // NOLINT: Too many names
+ return cxxMemberCallExpr(
+ on(expr(unless(cxxThisExpr()))),
+ callee(cxxMethodDecl(hasName("operator bool"),
+ ofClass(hasName("testing::AssertionResult")))));
+}
+
+static auto isAssertionResultConstructFromBoolCall() {
+ using namespace ::clang::ast_matchers; // NOLINT: Too many names
+ return cxxConstructExpr(
+ hasType(recordDecl(hasName("testing::AssertionResult"))),
+ hasArgument(0, hasType(booleanType())));
+}
+
static auto
buildDiagnoseMatchSwitch(const UncheckedStatusOrAccessModelOptions &Options) {
return CFGMatchSwitchBuilder<const Environment,
@@ -365,6 +435,26 @@ bool isStatusType(QualType Type) {
return isTypeNamed(Type, {"absl"}, "Status");
}
+static bool isPredicateFormatterFromMatcherType(QualType Type) {
+ return isTypeNamed(Type, {"testing", "internal"},
+ "PredicateFormatterFromMatcher");
+}
+
+static bool isAssertionResultType(QualType Type) {
+ return isTypeNamed(Type, {"testing"}, "AssertionResult");
+}
+
+static bool isStatusIsMatcherType(QualType Type) {
+ return isTypeNamed(Type, {"testing", "status", "internal_status"},
+ "StatusIsMatcher") ||
+ isTypeNamed(Type, {"testing", "status", "internal_status"},
+ "CanonicalStatusIsMatcher") ||
+ isTypeNamed(Type, {"absl_testing", "status_internal"},
+ "StatusIsMatcher") ||
+ isTypeNamed(Type, {"absl_testing", "status_internal"},
+ "CanonicalStatusIsMatcher");
+}
+
llvm::StringMap<QualType> getSyntheticFields(QualType Ty, QualType StatusType,
const CXXRecordDecl &RD) {
if (auto *TRD = getStatusOrBaseClass(Ty))
@@ -372,6 +462,12 @@ llvm::StringMap<QualType> getSyntheticFields(QualType Ty, QualType StatusType,
if (isStatusType(Ty) || (RD.hasDefinition() &&
RD.isDerivedFrom(StatusType->getAsCXXRecordDecl())))
return {{"ok", RD.getASTContext().BoolTy}};
+ if (isAssertionResultType(Ty))
+ return {{"ok", RD.getASTContext().BoolTy}};
+ if (isPredicateFormatterFromMatcherType(Ty))
+ return {{"ok_predicate", RD.getASTContext().BoolTy}};
+ if (isStatusIsMatcherType(Ty))
+ return {{"ok_matcher", RD.getASTContext().BoolTy}};
return {};
}
@@ -388,6 +484,13 @@ BoolValue &valForOk(RecordStorageLocation &StatusLoc, Environment &Env) {
return *Val;
return initializeStatus(StatusLoc, Env);
}
+static StorageLocation &locForOkPredicate(RecordStorageLocation &StatusLoc) {
+ return StatusLoc.getSyntheticField("ok_predicate");
+}
+
+static StorageLocation &locForOkMatcher(RecordStorageLocation &StatusLoc) {
+ return StatusLoc.getSyntheticField("ok_matcher");
+}
static void transferStatusOrOkCall(const CXXMemberCallExpr *Expr,
const MatchFinder::MatchResult &,
@@ -843,6 +946,97 @@ transferNonConstMemberOperatorCall(const CXXOperatorCallExpr *Expr,
handleNonConstMemberCall(Expr, RecordLoc, Result, State);
}
+static void transferMakePredicateFormatterFromIsOkMatcherCall(
+ const CallExpr *Expr, const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ State.Env.setValue(
+ locForOkPredicate(State.Env.getResultObjectLocation(*Expr)),
+ State.Env.getBoolLiteralValue(true));
+}
+
+static void transferStatusIsOkMatcherCall(const CallExpr *Expr,
+ const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ BoolValue &OkMatcherVal = State.Env.getBoolLiteralValue(true);
+ State.Env.setValue(locForOkMatcher(State.Env.getResultObjectLocation(*Expr)),
+ OkMatcherVal);
+}
+
+static void transferMakePredicateFormatterFromStatusIsMatcherCall(
+ const CallExpr *Expr, const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ assert(Expr->isPRValue());
+ auto &Loc = State.Env.getResultObjectLocation(*Expr->getArg(0));
+ auto &OkMatcherLoc = locForOkMatcher(Loc);
+ BoolValue *OkMatcherVal = State.Env.get<BoolValue>(OkMatcherLoc);
+ if (OkMatcherVal == nullptr)
+ return;
+ State.Env.setValue(
+ locForOkPredicate(State.Env.getResultObjectLocation(*Expr)),
+ *OkMatcherVal);
+}
+
+static void
+transferPredicateFormatterMatcherCall(const CXXOperatorCallExpr *Expr,
+ LatticeTransferState &State,
+ bool IsStatusOr) {
+ auto *Loc = State.Env.get<RecordStorageLocation>(*Expr->getArg(0));
+ if (Loc == nullptr)
+ return;
+
+ auto *ObjectLoc = State.Env.get<RecordStorageLocation>(*Expr->getArg(2));
+ if (ObjectLoc == nullptr)
+ return;
+
+ auto &OkPredicateLoc = locForOkPredicate(*Loc);
+ BoolValue *OkPredicateVal = State.Env.get<BoolValue>(OkPredicateLoc);
+ if (OkPredicateVal == nullptr)
+ return;
+
+ if (IsStatusOr)
+ ObjectLoc = &locForStatus(*ObjectLoc);
+ auto &StatusOk = valForOk(*ObjectLoc, State.Env);
+
+ auto &A = State.Env.arena();
+ auto &Res = State.Env.makeAtomicBoolValue();
+ State.Env.assume(
+ A.makeImplies(OkPredicateVal->formula(),
+ A.makeEquals(StatusOk.formula(), Res.formula())));
+ State.Env.setValue(locForOk(State.Env.getResultObjectLocation(*Expr)), Res);
+}
+
+static void
+transferAssertionResultConstructFromBoolCall(const CXXConstructExpr *Expr,
+ const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ assert(Expr->getNumArgs() > 0);
+
+ auto *StatusAdaptorLoc = State.Env.get<StorageLocation>(*Expr->getArg(0));
+ if (StatusAdaptorLoc == nullptr)
+ return;
+ BoolValue *OkVal = State.Env.get<BoolValue>(*StatusAdaptorLoc);
+ if (OkVal == nullptr)
+ return;
+ State.Env.setValue(locForOk(State.Env.getResultObjectLocation(*Expr)),
+ *OkVal);
+}
+
+static void
+transferAssertionResultOperatorBoolCall(const CXXMemberCallExpr *Expr,
+ const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ auto *RecordLoc = getImplicitObjectLocation(*Expr, State.Env);
+ if (RecordLoc == nullptr)
+ return;
+ BoolValue *OkVal = State.Env.get<BoolValue>(locForOk(*RecordLoc));
+ if (OkVal == nullptr)
+ return;
+ auto &A = State.Env.arena();
+ auto &Res = State.Env.makeAtomicBoolValue();
+ State.Env.assume(A.makeEquals(OkVal->formula(), Res.formula()));
+ State.Env.setValue(*Expr, Res);
+}
+
static RecordStorageLocation *
getSmartPtrLikeStorageLocation(const Expr &E, const Environment &Env) {
if (!E.isPRValue())
@@ -858,6 +1052,33 @@ buildTransferMatchSwitch(ASTContext &Ctx,
CFGMatchSwitchBuilder<LatticeTransferState> Builder) {
using namespace ::clang::ast_matchers; // NOLINT: Too many names
return std::move(Builder)
+ .CaseOfCFGStmt<CallExpr>(
+ isMakePredicateFormatterFromIsOkMatcherCall(),
+ transferMakePredicateFormatterFromIsOkMatcherCall)
+ .CaseOfCFGStmt<CallExpr>(isStatusIsOkMatcherCall(),
+ transferStatusIsOkMatcherCall)
+ .CaseOfCFGStmt<CallExpr>(
+ isMakePredicateFormatterFromStatusIsMatcherCall(),
+ transferMakePredicateFormatterFromStatusIsMatcherCall)
+ .CaseOfCFGStmt<CXXOperatorCallExpr>(
+ isPredicateFormatterFromStatusOrMatcherCall(),
+ [](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ transferPredicateFormatterMatcherCall(Expr, State,
+ /*IsStatusOr=*/true);
+ })
+ .CaseOfCFGStmt<CXXOperatorCallExpr>(
+ isPredicateFormatterFromStatusMatcherCall(),
+ [](const CXXOperatorCallExpr *Expr, const MatchFinder::MatchResult &,
+ LatticeTransferState &State) {
+ transferPredicateFormatterMatcherCall(Expr, State,
+ /*IsStatusOr=*/false);
+ })
+ .CaseOfCFGStmt<CXXConstructExpr>(
+ isAssertionResultConstructFromBoolCall(),
+ transferAssertionResultConstructFromBoolCall)
+ .CaseOfCFGStmt<CXXMemberCallExpr>(isAssertionResultOperatorBoolCall(),
+ transferAssertionResultOperatorBoolCall)
.CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("ok"),
transferStatusOrOkCall)
.CaseOfCFGStmt<CXXMemberCallExpr>(isStatusOrMemberCallWithName("status"),
diff --git a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
index dcb1cc13146bd..48e61abf09f19 100644
--- a/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/UncheckedStatusOrAccessModelTestFixture.cpp
@@ -2173,6 +2173,181 @@ TEST_P(UncheckedStatusOrAccessModelTest, ExpectOkMacro) {
)cc");
}
+TEST_P(UncheckedStatusOrAccessModelTest, AssertThatMacro) {
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor, testing::status::IsOk());
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor, absl_testing::IsOk());
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor.status(), testing::status::IsOk());
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+ void target() {
+ STATUSOR_INT sor = Make<STATUSOR_INT>();
+ ASSERT_THAT(sor, testing::status::IsOk());
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ using absl::StatusCode::kOk;
+ ASSERT_THAT(sor, testing::status::StatusIs(kOk));
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor, testing::status::StatusIs(absl::StatusCode::kOk));
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor, testing::status::CanonicalStatusIs(absl::StatusCode::kOk));
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor, absl_testing::CanonicalStatusIs(absl::StatusCode::kOk));
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ auto matcher = testing::status::StatusIs(absl::StatusCode::kOk);
+ ASSERT_THAT(sor, matcher);
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ auto matcher = absl_testing::StatusIs(absl::StatusCode::kOk);
+ ASSERT_THAT(sor, matcher);
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(
+ sor, testing::status::StatusIs(absl::StatusCode::kInvalidArgument));
+
+ sor.value(); // [[unsafe]]
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor, absl_testing::StatusIs(absl::StatusCode::kInvalidArgument));
+
+ sor.value(); // [[unsafe]]
+ }
+ )cc");
+
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor, testing::status::IsOkAndHolds(testing::IsTrue()));
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_THAT(sor, absl_testing::IsOkAndHolds(testing::IsTrue()));
+
+ sor.value();
+ }
+ )cc");
+}
+
+TEST_P(UncheckedStatusOrAccessModelTest, AssertOkMacro) {
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_OK(sor);
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(STATUSOR_INT sor) {
+ ASSERT_OK(sor.status());
+
+ sor.value();
+ }
+ )cc");
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+ void target() {
+ STATUSOR_INT sor = Make<STATUSOR_INT>();
+ ASSERT_OK(sor);
+
+ sor.value();
+ }
+ )cc");
+
+ ExpectDiagnosticsFor(R"cc(
+#include "unchecked_statusor_access_test_defs.h"
+
+ void target(std::optional<STATUSOR_INT> sor_opt) {
+ STATUSOR_INT sor = *sor_opt;
+ ASSERT_OK(sor);
+
+ sor.value();
+ }
+ )cc");
+}
+
TEST_P(UncheckedStatusOrAccessModelTest, BreadthFirstBlockTraversalLoop) {
// Evaluating the CFG blocks of the code below in breadth-first order results
// in an infinite loop. Each iteration of the while loop below results in a
``````````
</details>
https://github.com/llvm/llvm-project/pull/170947
More information about the llvm-branch-commits
mailing list