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

Baranov Victor via cfe-commits cfe-commits at lists.llvm.org
Tue Jun 24 01:47:27 PDT 2025


================
@@ -985,6 +985,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);
+            }
+          })
+
----------------
vbvictor wrote:

I'd prefer 2nd option because we have more control over what method belong to what class. Imagine this situation:
```cpp
struct optional1() {
   hasNull(); // check that it has value but it's nullptr
   isNull(); // check that it has no value inside
}

struct optional2() {
   hasNull(); // check that it has no value inside
}
``` 
If we go with 1st option, we would write `NonStdNoValue: hasNull;isNull`, but that would produce errors because `optional1::hasNull` doesn't actually check for no value, `optional1::isNull` does.
To express intentional behavior, we need to write `NonStdNoValue: optional2::hasNull;optional1::isNull`.

I don't think we need to worry too much about the length of the default value.

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


More information about the cfe-commits mailing list