[clang] [clang-tools-extra] [clang-tidy] `bugprone-unchecked-optional-access`: handle inheritance from `BloombergLP::bslstl::Optional_Base` to prevent false-positives for allocator-aware BDE types (PR #168863)
Valentyn Yukhymenko via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 21 05:57:34 PST 2025
https://github.com/BaLiKfromUA updated https://github.com/llvm/llvm-project/pull/168863
>From 95bb5a4636caae4d82262deadac65f02a93cf763 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <vyuhimenko at bloomberg.net>
Date: Wed, 19 Nov 2025 00:26:47 +0000
Subject: [PATCH 1/6] make first version of mocks to illustrate AA branching
during inheritance
---
.../bde/types/bsl_optional.h | 76 +++++++++++--------
.../std/types/optional.h | 57 ++++++++++++++
2 files changed, 103 insertions(+), 30 deletions(-)
create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h
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 7e1a129e04a55..241a91fb819a2 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
@@ -1,44 +1,36 @@
#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_BDE_TYPES_OPTIONAL_H_
#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_BDE_TYPES_OPTIONAL_H_
-/// Mock of `bsl::optional`.
-namespace bsl {
+#include "../../std/types/optional.h"
-// clang-format off
-template <typename T> struct remove_reference { using type = T; };
-template <typename T> struct remove_reference<T&> { using type = T; };
-template <typename T> struct remove_reference<T&&> { using type = T; };
-// clang-format on
-template <typename T>
-using remove_reference_t = typename remove_reference<T>::type;
+/// Mock of `BloombergLP::bslstl::Optional_Base`
+namespace BloombergLP::bslstl {
-template <typename T>
-constexpr T &&forward(remove_reference_t<T> &t) noexcept;
+template <class T>
+constexpr bool isAllocatorAware() {
+ return true;
+}
-template <typename T>
-constexpr T &&forward(remove_reference_t<T> &&t) noexcept;
+template <>
+constexpr bool isAllocatorAware<int>() {
+ return false;
+}
-template <typename T>
-constexpr remove_reference_t<T> &&move(T &&x);
+// Note: in reality `Optional_Base` checks if type uses bsl::allocator<>
+// This is simplified mock to illustrate similar behaviour
+template <class T, bool AA = isAllocatorAware<T>()>
+class Optional_Base {
-struct nullopt_t {
- constexpr explicit nullopt_t() {}
};
-constexpr nullopt_t nullopt;
+template <class T>
+class Optional_Base<T, false> : public std::optional<T> {
+};
-template <typename T>
-class optional {
+template <class T>
+class Optional_Base<T, true> {
public:
- constexpr optional() noexcept;
-
- constexpr optional(nullopt_t) noexcept;
-
- optional(const optional &) = default;
-
- optional(optional &&) = default;
-
const T &operator*() const &;
T &operator*() &;
const T &&operator*() const &&;
@@ -65,9 +57,33 @@ class optional {
void reset() noexcept;
- void swap(optional &rhs) noexcept;
+ void swap(Optional_Base &rhs) noexcept;
+
+ template <typename U> Optional_Base &operator=(const U &u);
+};
- template <typename U> optional &operator=(const U &u);
+} // namespace BloombergLP::bslstl
+
+
+/// Mock of `bsl::optional`.
+namespace bsl {
+
+struct nullopt_t {
+ constexpr explicit nullopt_t() {}
+};
+
+constexpr nullopt_t nullopt;
+
+template <typename T>
+class optional : public BloombergLP::bslstl::Optional_Base<T> {
+public:
+ constexpr optional() noexcept;
+
+ constexpr optional(nullopt_t) noexcept;
+
+ optional(const optional &) = default;
+
+ optional(optional &&) = default;
};
} // namespace bsl
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h
new file mode 100644
index 0000000000000..75202b6cf1250
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h
@@ -0,0 +1,57 @@
+#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_STD_TYPES_OPTIONAL_H_
+#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_STD_TYPES_OPTIONAL_H_
+
+namespace std {
+
+struct nullopt_t {
+ constexpr explicit nullopt_t() {}
+};
+
+constexpr nullopt_t nullopt;
+
+template <typename T>
+class optional {
+public:
+ constexpr optional() noexcept;
+
+ constexpr optional(nullopt_t) noexcept;
+
+ optional(const optional &) = default;
+
+ optional(optional &&) = default;
+
+ const T &operator*() const &;
+ T &operator*() &;
+ const T &&operator*() const &&;
+ T &&operator*() &&;
+
+ const T *operator->() const;
+ T *operator->();
+
+ const T &value() const &;
+ T &value() &;
+ const T &&value() const &&;
+ T &&value() &&;
+
+ constexpr explicit operator bool() const noexcept;
+ constexpr bool has_value() const noexcept;
+
+ template <typename U>
+ constexpr T value_or(U &&v) const &;
+ template <typename U>
+ T value_or(U &&v) &&;
+
+ template <typename... Args>
+ T &emplace(Args &&...args);
+
+ void reset() noexcept;
+
+ void swap(optional &rhs) noexcept;
+
+ template <typename U> optional &operator=(const U &u);
+};
+
+} // namespace std
+
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_STD_TYPES_OPTIONAL_H_
\ No newline at end of file
>From 42d414c7a4106c68c314f50f5eb0194f734d77ba Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <vyuhimenko at bloomberg.net>
Date: Wed, 19 Nov 2025 22:18:27 +0000
Subject: [PATCH 2/6] add failing test
---
.../bde/types/bsl_optional.h | 9 ++++++---
.../bugprone/unchecked-optional-access.cpp | 15 +++++++++++++++
2 files changed, 21 insertions(+), 3 deletions(-)
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 241a91fb819a2..2db7126267366 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
@@ -3,18 +3,21 @@
#include "../../std/types/optional.h"
+namespace bsl {
+ class string {};
+}
/// Mock of `BloombergLP::bslstl::Optional_Base`
namespace BloombergLP::bslstl {
template <class T>
constexpr bool isAllocatorAware() {
- return true;
+ return false;
}
template <>
-constexpr bool isAllocatorAware<int>() {
- return false;
+constexpr bool isAllocatorAware<bsl::string>() {
+ return true;
}
// Note: in reality `Optional_Base` checks if type uses bsl::allocator<>
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 8d70a9dc4861e..37fe24b7fd3c4 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
@@ -67,6 +67,21 @@ void bsl_optional_unchecked_value_access(const bsl::optional<int> &opt) {
x = *opt;
}
+void bsl_optional_unchecked_value_access_for_allocation_aware_class(const bsl::optional<bsl::string> &opt) {
+ opt.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+
+ bsl::string x = *opt;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+
+ if (!opt) {
+ return;
+ }
+
+ opt.value();
+ x = *opt;
+}
+
void bsl_optional_checked_access(const bsl::optional<int> &opt) {
if (opt.has_value()) {
opt.value();
>From e0d7d07ad42e64daf2a4b1ed7e5368e6a5312a68 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <vyuhimenko at bloomberg.net>
Date: Thu, 20 Nov 2025 00:59:05 +0000
Subject: [PATCH 3/6] implement fix
---
.../bugprone/unchecked-optional-access.cpp | 29 +++++++++++++++++--
.../Models/UncheckedOptionalAccessModel.cpp | 6 ++++
2 files changed, 33 insertions(+), 2 deletions(-)
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 37fe24b7fd3c4..4b345cc2c8c5b 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
@@ -67,12 +67,12 @@ void bsl_optional_unchecked_value_access(const bsl::optional<int> &opt) {
x = *opt;
}
-void bsl_optional_unchecked_value_access_for_allocation_aware_class(const bsl::optional<bsl::string> &opt) {
+void bsl_optional_unchecked_value_access_for_allocator_aware_class(const bsl::optional<bsl::string> &opt) {
opt.value();
// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
bsl::string x = *opt;
- // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
if (!opt) {
return;
@@ -124,6 +124,31 @@ void nullable_value_unchecked_value_access(const BloombergLP::bdlb::NullableValu
x = *opt;
}
+void nullable_value_unchecked_value_access_for_allocator_aware_type(const BloombergLP::bdlb::NullableValue<bsl::string> &opt) {
+ opt.value();
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+
+ bsl::string x = *opt;
+ // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+
+ if (opt.isNull()) {
+ opt.value();
+ }
+ // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+
+ if (!opt) {
+ opt.value();
+ }
+ // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
+
+ if (!opt.has_value()) {
+ return;
+ }
+
+ opt.value();
+ x = *opt;
+}
+
void nullable_value_optional_checked_access(const BloombergLP::bdlb::NullableValue<int> &opt) {
if (opt.has_value()) {
opt.value();
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index d90f5d4eaf7bb..1a4409f277c18 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -78,6 +78,12 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) {
isFullyQualifiedNamespaceEqualTo(*N, "folly"));
}
+ if (RD.getName() == "Optional_Base") {
+ const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext());
+ return N != nullptr &&
+ isFullyQualifiedNamespaceEqualTo(*N, "bslstl", "BloombergLP");
+ }
+
if (RD.getName() == "NullableValue") {
const auto *N = dyn_cast_or_null<NamespaceDecl>(RD.getDeclContext());
return N != nullptr &&
>From 6ba2c7a64a6440d856d36104e8b248ce4f1c96a2 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <vyuhimenko at bloomberg.net>
Date: Thu, 20 Nov 2025 01:07:50 +0000
Subject: [PATCH 4/6] update note
---
.../unchecked-optional-access/bde/types/bsl_optional.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
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 2db7126267366..7d7d14cde6429 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
@@ -20,8 +20,9 @@ constexpr bool isAllocatorAware<bsl::string>() {
return true;
}
-// Note: in reality `Optional_Base` checks if type uses bsl::allocator<>
-// This is simplified mock to illustrate similar behaviour
+// Note: real `Optional_Base` uses `BloombergLP::bslma::UsesBslmaAllocator`
+// to check if type is alocator-aware.
+// This is simplified mock to illustrate similar behaviour.
template <class T, bool AA = isAllocatorAware<T>()>
class Optional_Base {
>From 92aacef502b22743d9db6aa4074dbb39d5af8484 Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <vyuhimenko at bloomberg.net>
Date: Thu, 20 Nov 2025 01:16:19 +0000
Subject: [PATCH 5/6] release note
---
clang-tools-extra/docs/ReleaseNotes.rst | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b982216297919..7226a4eb0ab59 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -377,8 +377,9 @@ Changes in existing checks
- Improved :doc:`bugprone-unchecked-optional-access
<clang-tidy/checks/bugprone/unchecked-optional-access>` check by supporting
``NullableValue::makeValue`` and ``NullableValue::makeValueInplace`` to
- prevent false-positives for ``BloombergLP::bdlb::NullableValue`` type, and by
- adding the `IgnoreValueCalls` option to suppress diagnostics for
+ prevent false-positives for ``BloombergLP::bdlb::NullableValue``. Fixed
+ false-positives for ``bsl::optional`` containing allocator-aware type.
+ Added the `IgnoreValueCalls` option to suppress diagnostics for
``optional::value()`` and the `IgnoreSmartPointerDereference` option to
ignore optionals reached via smart-pointer-like dereference, while still
diagnosing UB-prone dereferences via ``operator*`` and ``operator->``.
>From 3039dd2ce0b7eadc72c7c09623a62a6e8d8e424f Mon Sep 17 00:00:00 2001
From: Valentyn Yukhymenko <vyuhimenko at bloomberg.net>
Date: Fri, 21 Nov 2025 13:57:19 +0000
Subject: [PATCH 6/6] small code review remarks
---
.../bde/types/bsl_optional.h | 15 +++++----------
.../std/types/optional.h | 2 +-
2 files changed, 6 insertions(+), 11 deletions(-)
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 7d7d14cde6429..a12572351a41c 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
@@ -21,19 +21,10 @@ constexpr bool isAllocatorAware<bsl::string>() {
}
// Note: real `Optional_Base` uses `BloombergLP::bslma::UsesBslmaAllocator`
-// to check if type is alocator-aware.
+// to check if type is allocator-aware.
// This is simplified mock to illustrate similar behaviour.
template <class T, bool AA = isAllocatorAware<T>()>
class Optional_Base {
-
-};
-
-template <class T>
-class Optional_Base<T, false> : public std::optional<T> {
-};
-
-template <class T>
-class Optional_Base<T, true> {
public:
const T &operator*() const &;
T &operator*() &;
@@ -66,6 +57,10 @@ class Optional_Base<T, true> {
template <typename U> Optional_Base &operator=(const U &u);
};
+template <class T>
+class Optional_Base<T, false> : public std::optional<T> {
+};
+
} // namespace BloombergLP::bslstl
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h
index 75202b6cf1250..d331585a4c21c 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h
@@ -54,4 +54,4 @@ class optional {
} // namespace std
-#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_STD_TYPES_OPTIONAL_H_
\ No newline at end of file
+#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_STD_TYPES_OPTIONAL_H_
More information about the cfe-commits
mailing list