[clang] [clang-tools-extra] [clang-tidy] `bugprone-unchecked-optional-access`: Improve handling of value constructors for `bsl::optional` and `bdlb::NullableValue` (PR #224969)
Valentyn Yukhymenko via cfe-commits
cfe-commits at lists.llvm.org
Fri Oct 2 04:35:17 PDT 2026
https://github.com/BaLiKfromUA updated https://github.com/llvm/llvm-project/pull/224969
>From 4d384d1918bd82a44f0e136e3c13cb3321cdf31c Mon Sep 17 00:00:00 2001
From: BaLiKfromUA <valentin.yukhymenko at gmail.com>
Date: Sun, 20 Sep 2026 21:26:43 +0100
Subject: [PATCH 1/5] Add failing tests
---
.../bde/types/bdlb_nullablevalue.h | 17 +++
.../bde/types/bsl_optional.h | 119 +++++++++++++++++-
.../bugprone/unchecked-optional-access.cpp | 66 ++++++++++
3 files changed, 198 insertions(+), 4 deletions(-)
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bdlb_nullablevalue.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bdlb_nullablevalue.h
index 08126771119952..729d83268fc2c7 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bdlb_nullablevalue.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bdlb_nullablevalue.h
@@ -13,6 +13,23 @@ class NullableValue : public bsl::optional<T> {
constexpr NullableValue(bsl::nullopt_t) noexcept;
+ /// Mock of `bdlb::NullableValue::EnableType`.
+ struct EnableType {};
+
+ template <typename OTHER>
+ using IfConstructsFrom = typename bsl::enable_if<
+ BloombergLP::bslstl::Optional_ConstructsFromType<T, OTHER>::value &&
+ BloombergLP::bslstl::Optional_IsNotDerivedFromOptional<T,
+ OTHER>::value,
+ EnableType>::type;
+
+ template <typename OTHER>
+ NullableValue(OTHER &&value, IfConstructsFrom<OTHER> = EnableType());
+
+ template <typename OTHER>
+ NullableValue(OTHER &&value, const bsl::allocator &allocator,
+ IfConstructsFrom<OTHER> = EnableType());
+
NullableValue(const NullableValue &) = default;
NullableValue(NullableValue &&) = default;
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bsl_optional.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bsl_optional.h
index a12572351a41c6..364e557706b7aa 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bsl_optional.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bsl_optional.h
@@ -5,6 +5,32 @@
namespace bsl {
class string {};
+
+ template <typename T> class optional;
+
+ struct nullopt_t {
+ constexpr explicit nullopt_t() {}
+ };
+
+ constexpr nullopt_t nullopt;
+
+ struct in_place_t {
+ constexpr explicit in_place_t() {}
+ };
+
+ constexpr in_place_t in_place;
+
+ struct allocator_arg_t {
+ constexpr explicit allocator_arg_t() {}
+ };
+
+ constexpr allocator_arg_t allocator_arg;
+
+ /// Mock of the allocator type taken by the allocator-extended constructors.
+ class allocator {};
+
+ template <bool B, class T> struct enable_if {};
+ template <class T> struct enable_if<true, T> { using type = T; };
}
/// Mock of `BloombergLP::bslstl::Optional_Base`
@@ -20,6 +46,57 @@ constexpr bool isAllocatorAware<bsl::string>() {
return true;
}
+/// Mock of `BloombergLP::bslstl::Optional_OptNoSuchType`
+struct Optional_OptNoSuchType {
+ explicit Optional_OptNoSuchType(int) noexcept {}
+};
+
+template <class T> struct Optional_RemoveCVRef { using type = T; };
+template <class T> struct Optional_RemoveCVRef<T &> : Optional_RemoveCVRef<T> {};
+template <class T> struct Optional_RemoveCVRef<T &&> : Optional_RemoveCVRef<T> {};
+template <class T> struct Optional_RemoveCVRef<const T> : Optional_RemoveCVRef<T> {};
+
+template <class T> struct Optional_IsTagType {
+ static constexpr bool value = false;
+};
+template <> struct Optional_IsTagType<bsl::nullopt_t> {
+ static constexpr bool value = true;
+};
+template <> struct Optional_IsTagType<bsl::in_place_t> {
+ static constexpr bool value = true;
+};
+template <> struct Optional_IsTagType<bsl::allocator_arg_t> {
+ static constexpr bool value = true;
+};
+template <> struct Optional_IsTagType<bsl::allocator> {
+ static constexpr bool value = true;
+};
+
+template <class T> struct Optional_IsStdOptional {
+ static constexpr bool value = false;
+};
+template <class T> struct Optional_IsStdOptional<std::optional<T>> {
+ static constexpr bool value = true;
+};
+
+/// Mock of `BloombergLP::bslstl::Optional_ConstructsFromType`.
+template <class TYPE, class ANY_TYPE>
+struct Optional_ConstructsFromType {
+private:
+ using Any = typename Optional_RemoveCVRef<ANY_TYPE>::type;
+
+public:
+ static constexpr bool value =
+ !Optional_IsTagType<Any>::value && !Optional_IsStdOptional<Any>::value;
+};
+
+/// Mock of the trait behind `BSLSTL_OPTIONAL_DEFINE_IF_NOT_DERIVED_FROM_OPTIONAL`.
+template <class TYPE, class ANY_TYPE>
+struct Optional_IsNotDerivedFromOptional {
+ static constexpr bool value = !__is_base_of(
+ bsl::optional<TYPE>, typename Optional_RemoveCVRef<ANY_TYPE>::type);
+};
+
// Note: real `Optional_Base` uses `BloombergLP::bslma::UsesBslmaAllocator`
// to check if type is allocator-aware.
// This is simplified mock to illustrate similar behaviour.
@@ -67,11 +144,19 @@ class Optional_Base<T, false> : public std::optional<T> {
/// Mock of `bsl::optional`.
namespace bsl {
-struct nullopt_t {
- constexpr explicit nullopt_t() {}
-};
+/// Mocks of the `BSLSTL_OPTIONAL_DECLARE_IF_*` macros.
+template <class TYPE, class ANY_TYPE>
+using Optional_IfConstructsFrom = typename enable_if<
+ BloombergLP::bslstl::Optional_ConstructsFromType<TYPE, ANY_TYPE>::value,
+ BloombergLP::bslstl::Optional_OptNoSuchType>::type;
+
+template <class TYPE, class ANY_TYPE>
+using Optional_IfNotDerivedFromOptional = typename enable_if<
+ BloombergLP::bslstl::Optional_IsNotDerivedFromOptional<TYPE,
+ ANY_TYPE>::value,
+ BloombergLP::bslstl::Optional_OptNoSuchType>::type;
-constexpr nullopt_t nullopt;
+using Optional_NoSuchType = BloombergLP::bslstl::Optional_OptNoSuchType;
template <typename T>
class optional : public BloombergLP::bslstl::Optional_Base<T> {
@@ -80,11 +165,37 @@ class optional : public BloombergLP::bslstl::Optional_Base<T> {
constexpr optional(nullopt_t) noexcept;
+ template <typename ANY_TYPE = T>
+ optional(ANY_TYPE &&v,
+ Optional_IfConstructsFrom<T, ANY_TYPE> = Optional_NoSuchType(0),
+ Optional_IfNotDerivedFromOptional<T, ANY_TYPE> = Optional_NoSuchType(0));
+
+ template <typename ANY_TYPE>
+ optional(const std::optional<ANY_TYPE> &v,
+ Optional_IfConstructsFrom<T, ANY_TYPE> = Optional_NoSuchType(0),
+ Optional_IfNotDerivedFromOptional<T, ANY_TYPE> = Optional_NoSuchType(0));
+
+ template <typename... ARGS>
+ explicit optional(in_place_t, ARGS &&...args);
+
+ optional(allocator_arg_t, allocator);
+
+ template <typename ANY_TYPE = T>
+ optional(allocator_arg_t, allocator, ANY_TYPE &&v,
+ Optional_IfConstructsFrom<T, ANY_TYPE> = Optional_NoSuchType(0),
+ Optional_IfNotDerivedFromOptional<T, ANY_TYPE> = Optional_NoSuchType(0));
+
+ template <typename... ARGS>
+ explicit optional(allocator_arg_t, allocator, in_place_t, ARGS &&...args);
+
optional(const optional &) = default;
optional(optional &&) = default;
};
+template <typename T, typename... ARGS>
+optional<T> make_optional(ARGS &&...args);
+
} // namespace bsl
#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_BDE_TYPES_OPTIONAL_H_
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
index 337474bdf7535d..8bff66670f0da1 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
@@ -231,6 +231,72 @@ void nullable_value_make_value(BloombergLP::bdlb::NullableValue<int> &opt1, Bloo
opt2.value();
}
+
+void bsl_optional_value_constructor(int v, bsl::string s) {
+ bsl::optional<int> opt1 = v;
+ opt1.value();
+
+ bsl::optional<int> opt2(v);
+ opt2.value();
+
+ bsl::optional<bsl::string> opt3 = s;
+ opt3.value();
+
+ bsl::optional<bsl::string> opt4(s);
+ opt4.value();
+}
+
+void bsl_optional_allocator_extended_value_constructor(bsl::string s) {
+ bsl::optional<bsl::string> opt(bsl::allocator_arg, bsl::allocator{}, s);
+ opt.value();
+}
+
+void bsl_optional_in_place_constructor(bsl::string s) {
+ bsl::optional<bsl::string> opt1(bsl::in_place, s);
+ opt1.value();
+
+ bsl::optional<bsl::string> opt2(bsl::allocator_arg, bsl::allocator{},
+ bsl::in_place, s);
+ opt2.value();
+}
+
+void bsl_optional_make_optional() {
+ bsl::optional<int> opt = bsl::make_optional<int>(1);
+ opt.value();
+}
+
+void bsl_optional_converting_constructor(std::optional<int> src) {
+ bsl::optional<int> opt1 = src;
+ opt1.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+
+ if (src.has_value()) {
+ bsl::optional<int> opt2 = src;
+ opt2.value();
+ }
+}
+
+void bsl_optional_empty_constructors() {
+ bsl::optional<bsl::string> opt1(bsl::allocator_arg, bsl::allocator{});
+ opt1.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+
+ bsl::optional<int> opt2 = bsl::nullopt;
+ opt2.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+}
+
+void nullable_value_value_constructor(int v, bsl::string s) {
+ BloombergLP::bdlb::NullableValue<int> opt1 = v;
+ opt1.value();
+
+ BloombergLP::bdlb::NullableValue<bsl::string> opt2 = s;
+ opt2.value();
+
+ BloombergLP::bdlb::NullableValue<bsl::string> opt3(s, bsl::allocator{});
+ opt3.value();
+}
+
void assertion_handler() __attribute__((analyzer_noreturn));
void function_calling_analyzer_noreturn(const bsl::optional<int>& opt)
>From f05219a0ce864da0f985edc49e09db65fe3e9ba7 Mon Sep 17 00:00:00 2001
From: BaLiKfromUA <valentin.yukhymenko at gmail.com>
Date: Sun, 20 Sep 2026 21:30:57 +0100
Subject: [PATCH 2/5] Add release note
---
clang-tools-extra/docs/ReleaseNotes.md | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 0447458c147ad7..b3063cf09d44a1 100644
--- a/clang-tools-extra/docs/ReleaseNotes.md
+++ b/clang-tools-extra/docs/ReleaseNotes.md
@@ -189,6 +189,11 @@ infrastructure are described first, followed by tool-specific sections.
<clang-tidy/checks/bugprone/std-namespace-modification>` when checking
lambda closure types used as template arguments.
+- Improved {doc}`bugprone-unchecked-optional-access
+ <clang-tidy/checks/bugprone/unchecked-optional-access>` by fixing false
+ positives on `bsl::optional` and `bdlb::NullableValue` constructed from a
+ value or returned by `bsl::make_optional`.
+
- Improved {doc}`cppcoreguidelines-missing-std-forward
<clang-tidy/checks/cppcoreguidelines/missing-std-forward>` check by diagnosing
unforwarded `auto&&` parameters in C++20 abbreviated function templates.
>From 70ed5bd3fcbd139d8b53ebc492173ae2629ab8bb Mon Sep 17 00:00:00 2001
From: BaLiKfromUA <valentin.yukhymenko at gmail.com>
Date: Sun, 20 Sep 2026 21:56:48 +0100
Subject: [PATCH 3/5] Implementation of fix
---
.../Models/UncheckedOptionalAccessModel.cpp | 58 ++++++++++++++++---
1 file changed, 49 insertions(+), 9 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 568564fb361f4a..0c48c441e2e025 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -247,7 +247,7 @@ auto isMakeOptionalCall() {
callee(functionDecl(hasAnyName(
"std::make_optional", "base::make_optional", "absl::make_optional",
"folly::make_optional", "bsl::make_optional"))),
- hasOptionalType());
+ hasOptionalOrDerivedType());
}
auto nulloptTypeDecl() {
@@ -264,6 +264,12 @@ auto inPlaceClass() {
"bsl::in_place_t"));
}
+auto allocatorArgClass() {
+ return namedDecl(hasAnyName("std::allocator_arg_t", "bsl::allocator_arg_t"));
+}
+
+auto hasAllocatorArgType() { return hasType(allocatorArgClass()); }
+
auto isOptionalNulloptConstructor() {
return cxxConstructExpr(
hasDeclaration(cxxConstructorDecl(parameterCountIs(1),
@@ -272,15 +278,31 @@ auto isOptionalNulloptConstructor() {
}
auto isOptionalInPlaceConstructor() {
- return cxxConstructExpr(hasArgument(0, hasType(inPlaceClass())),
+ return cxxConstructExpr(hasAnyArgument(hasType(inPlaceClass())),
hasOptionalOrDerivedType());
}
+// Arguments after the value -- an allocator, or defaulted parameters carrying
+// SFINAE constraints -- are ignored. Leading tags are excluded because they
+// denote other constructions, e.g. `optional(allocator_arg_t, allocator)` is
+// empty.
auto isOptionalValueOrConversionConstructor() {
return cxxConstructExpr(
unless(hasDeclaration(
cxxConstructorDecl(anyOf(isCopyConstructor(), isMoveConstructor())))),
- argumentCountIs(1), hasArgument(0, unless(hasNulloptType())),
+ argumentCountAtLeast(1),
+ hasArgument(0, unless(anyOf(hasNulloptType(), hasType(inPlaceClass()),
+ hasAllocatorArgType()))),
+ hasOptionalOrDerivedType());
+}
+
+// `optional(allocator_arg_t, allocator, value, ...)`.
+auto isOptionalAllocatorExtendedValueOrConversionConstructor() {
+ return cxxConstructExpr(
+ unless(hasDeclaration(
+ cxxConstructorDecl(anyOf(isCopyConstructor(), isMoveConstructor())))),
+ hasArgument(0, hasAllocatorArgType()), argumentCountAtLeast(3),
+ hasArgument(2, unless(anyOf(hasNulloptType(), hasType(inPlaceClass())))),
hasOptionalOrDerivedType());
}
@@ -757,16 +779,30 @@ BoolValue &valueOrConversionHasValue(QualType DestType, const Expr &E,
return State.Env.makeAtomicBoolValue();
}
-void transferValueOrConversionConstructor(
- const CXXConstructExpr *E, const MatchFinder::MatchResult &MatchRes,
- LatticeTransferState &State) {
- assert(E->getNumArgs() > 0);
+void transferValueOrConversionConstructorImpl(
+ const CXXConstructExpr *E, unsigned ValueArgIdx,
+ const MatchFinder::MatchResult &MatchRes, LatticeTransferState &State) {
+ assert(E->getNumArgs() > ValueArgIdx);
constructOptionalValue(
*E, State.Env,
valueOrConversionHasValue(
- E->getConstructor()->getThisType()->getPointeeType(), *E->getArg(0),
- MatchRes, State));
+ E->getConstructor()->getThisType()->getPointeeType(),
+ *E->getArg(ValueArgIdx), MatchRes, State));
+}
+
+void transferValueOrConversionConstructor(
+ const CXXConstructExpr *E, const MatchFinder::MatchResult &MatchRes,
+ LatticeTransferState &State) {
+ transferValueOrConversionConstructorImpl(E, /*ValueArgIdx=*/0, MatchRes,
+ State);
+}
+
+void transferAllocatorExtendedValueOrConversionConstructor(
+ const CXXConstructExpr *E, const MatchFinder::MatchResult &MatchRes,
+ LatticeTransferState &State) {
+ transferValueOrConversionConstructorImpl(E, /*ValueArgIdx=*/2, MatchRes,
+ State);
}
void transferAssignment(const CXXOperatorCallExpr *E, BoolValue &HasValueVal,
@@ -1015,6 +1051,10 @@ auto buildTransferMatchSwitch() {
// optional::optional (value/conversion)
.CaseOfCFGStmt<CXXConstructExpr>(isOptionalValueOrConversionConstructor(),
transferValueOrConversionConstructor)
+ // optional::optional (allocator-extended value/conversion)
+ .CaseOfCFGStmt<CXXConstructExpr>(
+ isOptionalAllocatorExtendedValueOrConversionConstructor(),
+ transferAllocatorExtendedValueOrConversionConstructor)
// optional::operator=
.CaseOfCFGStmt<CXXOperatorCallExpr>(
>From ddfd7ced5817ace94584374b64738d72e18941f0 Mon Sep 17 00:00:00 2001
From: BaLiKfromUA <valentin.yukhymenko at gmail.com>
Date: Sun, 20 Sep 2026 22:07:54 +0100
Subject: [PATCH 4/5] Smaller comment
---
.../FlowSensitive/Models/UncheckedOptionalAccessModel.cpp | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 0c48c441e2e025..8bc7204a540ad1 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -282,10 +282,8 @@ auto isOptionalInPlaceConstructor() {
hasOptionalOrDerivedType());
}
-// Arguments after the value -- an allocator, or defaulted parameters carrying
-// SFINAE constraints -- are ignored. Leading tags are excluded because they
-// denote other constructions, e.g. `optional(allocator_arg_t, allocator)` is
-// empty.
+// `optional(value, ...)`. Arguments after the value are ignored. Tag types are
+// excluded because they denote other constructions.
auto isOptionalValueOrConversionConstructor() {
return cxxConstructExpr(
unless(hasDeclaration(
>From 282ed09e677cd9880a6de1855aaf2e60d5f858e9 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <valentin.yukhymenko at gmail.com>
Date: Fri, 2 Oct 2026 12:34:52 +0100
Subject: [PATCH 5/5] Add comment about BDE use-case
---
.../FlowSensitive/Models/UncheckedOptionalAccessModel.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 8bc7204a540ad1..c053f7193d4fcf 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -295,6 +295,8 @@ auto isOptionalValueOrConversionConstructor() {
}
// `optional(allocator_arg_t, allocator, value, ...)`.
+// Used only for BDE components (`bsl::optional`, `bdlb::NullableValue`)
+// which support allocators.
auto isOptionalAllocatorExtendedValueOrConversionConstructor() {
return cxxConstructExpr(
unless(hasDeclaration(
More information about the cfe-commits
mailing list