diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 7e8016536afa2..b13d37198c7f6 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -427,8 +427,9 @@ Changes in existing checks - Improved :doc:`bugprone-unchecked-optional-access ` check by supporting ``NullableValue::makeValue`` and ``NullableValue::makeValueInplace`` to - prevent false-positives for ``BloombergLP::bdlb::NullableValue`` type, and by - adding the `IgnoreValueCalls` option to suppress diagnostics for + prevent false-positives for ``BloombergLP::bdlb::NullableValue``. Fixed + false-positives for ``bsl::optional`` containing allocator-aware type. + Added the `IgnoreValueCalls` option to suppress diagnostics for ``optional::value()`` and the `IgnoreSmartPointerDereference` option to ignore optionals reached via smart-pointer-like dereference, while still diagnosing UB-prone dereferences via ``operator*`` and ``operator->``. 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 7e1a129e04a55..a12572351a41c 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 @@ -1,44 +1,31 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_BDE_TYPES_OPTIONAL_H_ #define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_BDE_TYPES_OPTIONAL_H_ -/// Mock of `bsl::optional`. -namespace bsl { - -// clang-format off -template struct remove_reference { using type = T; }; -template struct remove_reference { using type = T; }; -template struct remove_reference { using type = T; }; -// clang-format on - -template -using remove_reference_t = typename remove_reference::type; - -template -constexpr T &&forward(remove_reference_t &t) noexcept; - -template -constexpr T &&forward(remove_reference_t &&t) noexcept; - -template -constexpr remove_reference_t &&move(T &&x); - -struct nullopt_t { - constexpr explicit nullopt_t() {} -}; - -constexpr nullopt_t nullopt; +#include "../../std/types/optional.h" -template -class optional { +namespace bsl { + class string {}; +} + +/// Mock of `BloombergLP::bslstl::Optional_Base` +namespace BloombergLP::bslstl { + +template +constexpr bool isAllocatorAware() { + return false; +} + +template <> +constexpr bool isAllocatorAware() { + return true; +} + +// Note: real `Optional_Base` uses `BloombergLP::bslma::UsesBslmaAllocator` +// to check if type is allocator-aware. +// This is simplified mock to illustrate similar behaviour. +template ()> +class Optional_Base { public: - constexpr optional() noexcept; - - constexpr optional(nullopt_t) noexcept; - - optional(const optional &) = default; - - optional(optional &&) = default; - const T &operator*() const &; T &operator*() &; const T &&operator*() const &&; @@ -65,9 +52,37 @@ class optional { void reset() noexcept; - void swap(optional &rhs) noexcept; + void swap(Optional_Base &rhs) noexcept; + + template Optional_Base &operator=(const U &u); +}; + +template +class Optional_Base : public std::optional { +}; + +} // namespace BloombergLP::bslstl + - template optional &operator=(const U &u); +/// Mock of `bsl::optional`. +namespace bsl { + +struct nullopt_t { + constexpr explicit nullopt_t() {} +}; + +constexpr nullopt_t nullopt; + +template +class optional : public BloombergLP::bslstl::Optional_Base { +public: + constexpr optional() noexcept; + + constexpr optional(nullopt_t) noexcept; + + optional(const optional &) = default; + + optional(optional &&) = default; }; } // namespace bsl diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h new file mode 100644 index 0000000000000..d331585a4c21c --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/unchecked-optional-access/std/types/optional.h @@ -0,0 +1,57 @@ +#ifndef LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_STD_TYPES_OPTIONAL_H_ +#define LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_STD_TYPES_OPTIONAL_H_ + +namespace std { + +struct nullopt_t { + constexpr explicit nullopt_t() {} +}; + +constexpr nullopt_t nullopt; + +template +class optional { +public: + constexpr optional() noexcept; + + constexpr optional(nullopt_t) noexcept; + + optional(const optional &) = default; + + optional(optional &&) = default; + + const T &operator*() const &; + T &operator*() &; + const T &&operator*() const &&; + T &&operator*() &&; + + const T *operator->() const; + T *operator->(); + + const T &value() const &; + T &value() &; + const T &&value() const &&; + T &&value() &&; + + constexpr explicit operator bool() const noexcept; + constexpr bool has_value() const noexcept; + + template + constexpr T value_or(U &&v) const &; + template + T value_or(U &&v) &&; + + template + T &emplace(Args &&...args); + + void reset() noexcept; + + void swap(optional &rhs) noexcept; + + template optional &operator=(const U &u); +}; + +} // namespace std + + +#endif // LLVM_CLANG_TOOLS_EXTRA_TEST_CLANG_TIDY_CHECKERS_INPUTS_STD_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 8d70a9dc4861e..984156c028c00 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 @@ -65,15 +65,44 @@ void bsl_optional_unchecked_value_access(const bsl::optional &opt) { opt.value(); x = *opt; + + bsl::optional opt1; + opt1.value(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] +} + +void bsl_optional_unchecked_value_access_for_allocator_aware_class(const bsl::optional &opt) { + opt.value(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + + bsl::string x = *opt; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + + if (!opt) { + return; + } + + opt.value(); + x = *opt; + + bsl::optional opt1; + opt1.value(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] } void bsl_optional_checked_access(const bsl::optional &opt) { if (opt.has_value()) { opt.value(); } + if (opt) { opt.value(); } + + if (opt) { + bsl::optional opt1(opt); + opt1.value(); + } } void bsl_optional_value_after_swap(bsl::optional &opt1, bsl::optional &opt2) { @@ -109,6 +138,31 @@ void nullable_value_unchecked_value_access(const BloombergLP::bdlb::NullableValu x = *opt; } +void nullable_value_unchecked_value_access_for_allocator_aware_type(const BloombergLP::bdlb::NullableValue &opt) { + opt.value(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + + bsl::string x = *opt; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + + if (opt.isNull()) { + opt.value(); + } + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + + if (!opt) { + opt.value(); + } + // CHECK-MESSAGES: :[[@LINE-2]]:5: warning: unchecked access to optional value [bugprone-unchecked-optional-access] + + if (!opt.has_value()) { + return; + } + + opt.value(); + x = *opt; +} + void nullable_value_optional_checked_access(const BloombergLP::bdlb::NullableValue &opt) { if (opt.has_value()) { opt.value(); diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index d90f5d4eaf7bb..04a2a557debb2 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp @@ -66,8 +66,7 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) { if (RD.getName() == "optional") { if (const auto *N = dyn_cast_or_null(RD.getDeclContext())) return N->isStdNamespace() || - isFullyQualifiedNamespaceEqualTo(*N, "absl") || - isFullyQualifiedNamespaceEqualTo(*N, "bsl"); + isFullyQualifiedNamespaceEqualTo(*N, "absl"); return false; } @@ -78,6 +77,12 @@ static bool hasOptionalClassName(const CXXRecordDecl &RD) { isFullyQualifiedNamespaceEqualTo(*N, "folly")); } + if (RD.getName() == "Optional_Base") { + const auto *N = dyn_cast_or_null(RD.getDeclContext()); + return N != nullptr && + isFullyQualifiedNamespaceEqualTo(*N, "bslstl", "BloombergLP"); + } + if (RD.getName() == "NullableValue") { const auto *N = dyn_cast_or_null(RD.getDeclContext()); return N != nullptr &&