[clang-tools-extra] cd4c82c - [clang-tidy] `bugprone-unchecked-optional-access`: handle `BloombergLP::bdlb:NullableValue::makeValue` to prevent false-positives (#144313)

[via cfe-commits]

Author: Valentyn Yukhymenko
Date: 2025-09-09T16:20:46+03:00
New Revision: cd4c82c1c3763a54e964f7507964693290fd997b

URL: https://github.com/llvm/llvm-project/commit/cd4c82c1c3763a54e964f7507964693290fd997b
DIFF: https://github.com/llvm/llvm-project/commit/cd4c82c1c3763a54e964f7507964693290fd997b.diff

LOG: [clang-tidy] `bugprone-unchecked-optional-access`: handle `BloombergLP::bdlb:NullableValue::makeValue` to prevent false-positives (#144313)

https://github.com/llvm/llvm-project/pull/101450 added support for
`BloombergLP::bdlb::NullableValue`.

However, `NullableValue::makeValue` and
`NullableValue::makeValueInplace` have been missed which impacts code
like this:
```cpp
  if (opt.isNull()) {
    opt.makeValue(42);
  }

  opt.value(); // triggers false positive warning from `bugprone-unchecked-optional-access`
```

My patch addresses this issue.

[Docs that I used for methods
mocks](https://bloomberg.github.io/bde-resources/doxygen/bde_api_prod/classbdlb_1_1NullableValue.html)

---------

Co-authored-by: Baranov Victor <bar.victor.2002 at gmail.com>

Added: 
    

Modified: 
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bdlb_nullablevalue.h
    clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp
    clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index bd757851abe02..41a8a865f18ca 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -201,6 +201,11 @@ Changes in existing checks
   namespace are treated as the tag or the data part of a user-defined
   tagged union respectively.
 
+- 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.
+
 - Improved :doc:`bugprone-unhandled-self-assignment
   <clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding
   an additional matcher that generalizes the copy-and-swap idiom pattern

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 4411bcfd60a74..0812677111995 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
@@ -20,6 +20,14 @@ class NullableValue : public bsl::optional<T> {
   const T &value() const &;
   T &value() &;
 
+  constexpr T &makeValue();
+
+  template <typename U>
+  constexpr T &makeValue(U&& v);
+
+  template <typename... ARGS>
+  constexpr T &makeValueInplace(ARGS &&... args);
+
   // 'operator bool' is inherited from bsl::optional
 
   constexpr bool isNull() const noexcept;

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 4911157828765..8d70a9dc4861e 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
@@ -141,6 +141,20 @@ void nullable_value_after_swap(BloombergLP::bdlb::NullableValue<int> &opt1, Bloo
   }
 }
 
+void nullable_value_make_value(BloombergLP::bdlb::NullableValue<int> &opt1, BloombergLP::bdlb::NullableValue<int> &opt2) {
+  if (opt1.isNull()) {
+    opt1.makeValue(42);
+  }
+
+  opt1.value();
+
+  if (opt2.isNull()) {
+    opt2.makeValueInplace(42);
+  }
+
+  opt2.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 45c307d7469f0..bb703eff4baff 100644
--- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
@@ -982,6 +982,20 @@ auto buildTransferMatchSwitch() {
           isOptionalMemberCallWithNameMatcher(hasName("isNull")),
           transferOptionalIsNullCall)
 
+      // NullableValue::makeValue, NullableValue::makeValueInplace
+      // Only NullableValue has these methods, but this
+      // will also pass for other types
+      .CaseOfCFGStmt<CXXMemberCallExpr>(
+          isOptionalMemberCallWithNameMatcher(
+              hasAnyName("makeValue", "makeValueInplace")),
+          [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
+             LatticeTransferState &State) {
+            if (RecordStorageLocation *Loc =
+                    getImplicitObjectLocation(*E, State.Env)) {
+              setHasValue(*Loc, State.Env.getBoolLiteralValue(true), State.Env);
+            }
+          })
+
       // optional::emplace
       .CaseOfCFGStmt<CXXMemberCallExpr>(
           isOptionalMemberCallWithNameMatcher(hasName("emplace")),