Skip to content

Commit cd4c82c

Browse files
[clang-tidy] bugprone-unchecked-optional-access: handle BloombergLP::bdlb:NullableValue::makeValue to prevent false-positives (llvm#144313)
llvm#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 <[email protected]>
1 parent a69a406 commit cd4c82c

File tree

4 files changed

+41
-0
lines changed

4 files changed

+41
-0
lines changed

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,11 @@ Changes in existing checks
201201
namespace are treated as the tag or the data part of a user-defined
202202
tagged union respectively.
203203

204+
- Improved :doc:`bugprone-unchecked-optional-access
205+
<clang-tidy/checks/bugprone/unchecked-optional-access>` check by supporting
206+
``NullableValue::makeValue`` and ``NullableValue::makeValueInplace`` to
207+
prevent false-positives for ``BloombergLP::bdlb::NullableValue`` type.
208+
204209
- Improved :doc:`bugprone-unhandled-self-assignment
205210
<clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding
206211
an additional matcher that generalizes the copy-and-swap idiom pattern

clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/bde/types/bdlb_nullablevalue.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,14 @@ class NullableValue : public bsl::optional<T> {
2020
const T &value() const &;
2121
T &value() &;
2222

23+
constexpr T &makeValue();
24+
25+
template <typename U>
26+
constexpr T &makeValue(U&& v);
27+
28+
template <typename... ARGS>
29+
constexpr T &makeValueInplace(ARGS &&... args);
30+
2331
// 'operator bool' is inherited from bsl::optional
2432

2533
constexpr bool isNull() const noexcept;

clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,20 @@ void nullable_value_after_swap(BloombergLP::bdlb::NullableValue<int> &opt1, Bloo
141141
}
142142
}
143143

144+
void nullable_value_make_value(BloombergLP::bdlb::NullableValue<int> &opt1, BloombergLP::bdlb::NullableValue<int> &opt2) {
145+
if (opt1.isNull()) {
146+
opt1.makeValue(42);
147+
}
148+
149+
opt1.value();
150+
151+
if (opt2.isNull()) {
152+
opt2.makeValueInplace(42);
153+
}
154+
155+
opt2.value();
156+
}
157+
144158
void assertion_handler() __attribute__((analyzer_noreturn));
145159

146160
void function_calling_analyzer_noreturn(const bsl::optional<int>& opt)

clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -982,6 +982,20 @@ auto buildTransferMatchSwitch() {
982982
isOptionalMemberCallWithNameMatcher(hasName("isNull")),
983983
transferOptionalIsNullCall)
984984

985+
// NullableValue::makeValue, NullableValue::makeValueInplace
986+
// Only NullableValue has these methods, but this
987+
// will also pass for other types
988+
.CaseOfCFGStmt<CXXMemberCallExpr>(
989+
isOptionalMemberCallWithNameMatcher(
990+
hasAnyName("makeValue", "makeValueInplace")),
991+
[](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
992+
LatticeTransferState &State) {
993+
if (RecordStorageLocation *Loc =
994+
getImplicitObjectLocation(*E, State.Env)) {
995+
setHasValue(*Loc, State.Env.getBoolLiteralValue(true), State.Env);
996+
}
997+
})
998+
985999
// optional::emplace
9861000
.CaseOfCFGStmt<CXXMemberCallExpr>(
9871001
isOptionalMemberCallWithNameMatcher(hasName("emplace")),

0 commit comments

Comments
 (0)