[clang] [clang-tools-extra] [clang-tidy] `bugprone-unchecked-optional-access`: Improve handling of value constructors for `bsl::optional` and `bdlb::NullableValue` (PR #224969)

Sun Sep 20 14:35:48 PDT 2026

llvmorg-github-actions[bot] wrote:




@llvm/pr-subscribers-clang-analysis

Author: Valentyn Yukhymenko (BaLiKfromUA)

<details>
<summary>Changes</summary>

Initially was found in https://github.com/llvm/llvm-project/pull/168863#discussion_r2620106435 but I have reports from internal users as well.

🔎 [**Compiler explorer link** to illustrate false-positives in the main branch.](https://compiler-explorer.com/z/1GxzzK4xj)

🤖 **AI usage:** I used LLM to generate mocks by pointing to BDE source code locally. I also used LLM for code review and documentation.

Manual test with real headers:
```bash
balik@BaLiKfromUA:~/Desktop/llvm-project$ build/bin/clang-tidy --checks='-*,bugprone-unchecked-optional-access' \
  bde-optional-repro.cpp -- -std=c++17 \
  $(find ~/Desktop/bde/groups ~/Desktop/bde/standalones \
       -mindepth 2 -maxdepth 2 -type d -printf '-I%p ')
3 warnings generated.
/home/balik/Desktop/llvm-project/bde-optional-repro.cpp:41:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
   41 |   opt1.value();  // <-- WARNING EXPECTED (true positive)
      |   ^~~~
/home/balik/Desktop/llvm-project/bde-optional-repro.cpp:51:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
   51 |   opt1.value();  // <-- WARNING EXPECTED (true positive)
      |   ^~~~
/home/balik/Desktop/llvm-project/bde-optional-repro.cpp:54:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access]
   54 |   opt2.value();  // <-- WARNING EXPECTED (true positive)
      |   ^~~~


```




---
Full diff: https://github.com/llvm/llvm-project/pull/224969.diff


5 Files Affected:

- (modified) clang-tools-extra/docs/ReleaseNotes.md (+5) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bdlb_nullablevalue.h (+17) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bsl_optional.h (+115-4) 
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp (+66) 
- (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp (+47-9) 


``````````diff

diff --git a/clang-tools-extra/docs/ReleaseNotes.md b/clang-tools-extra/docs/ReleaseNotes.md
index 0447458c147ad..b3063cf09d44a 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.
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 0812677111995..729d83268fc2c 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 a12572351a41c..364e557706b7a 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 337474bdf7535..8bff66670f0da 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)
diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
index 568564fb361f4..8bc7204a540ad 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,29 @@ auto isOptionalNulloptConstructor() {
 }
 
 auto isOptionalInPlaceConstructor() {
-  return cxxConstructExpr(hasArgument(0, hasType(inPlaceClass())),
+  return cxxConstructExpr(hasAnyArgument(hasType(inPlaceClass())),
                           hasOptionalOrDerivedType());
 }
 
+// `optional(value, ...)`. Arguments after the value are ignored. Tag types are
+// excluded because they denote other constructions.
 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 +777,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 +1049,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>(

``````````

</details>


https://github.com/llvm/llvm-project/pull/224969
