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.2...@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")), _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
