[clang-tidy] Add IgnoreValueCalls option to bugprone-unchecked-optional-access

clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h

@@ -25,7 +25,8 @@ class UncheckedOptionalAccessCheck : public ClangTidyCheck {
 public:
   UncheckedOptionalAccessCheck(StringRef Name, ClangTidyContext *Context)
       : ClangTidyCheck(Name, Context),
-        ModelOptions{Options.get("IgnoreSmartPointerDereference", false)} {}
+        ModelOptions{Options.get("IgnoreSmartPointerDereference", false),
+                     Options.get("IgnoreValueCalls", false)} {}
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
@@ -34,6 +35,7 @@ class UncheckedOptionalAccessCheck : public ClangTidyCheck {
   void storeOptions(ClangTidyOptions::OptionMap &Opts) override {
     Options.store(Opts, "IgnoreSmartPointerDereference",
                   ModelOptions.IgnoreSmartPointerDereference);
+    Options.store(Opts, "IgnoreValueCalls", ModelOptions.IgnoreValueCalls);
   }
 
 private:

clang-tools-extra/docs/ReleaseNotes.rst

@@ -351,7 +351,10 @@ Changes in existing checks
 - 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.
+  prevent false-positives for ``BloombergLP::bdlb::NullableValue`` type, and by
+  adding the ``IgnoreValueCalls`` option to suppress diagnostics for
+  ``optional::value()`` while still diagnosing UB-prone dereferences via
+  ``operator*`` and ``operator->``.
 
 - Improved :doc:`bugprone-unhandled-self-assignment
   <clang-tidy/checks/bugprone/unhandled-self-assignment>` check by adding

clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst

@@ -308,3 +308,22 @@ advantages:
   * Performance. A single check can cover many or even all accesses within
     scope. This gives the user the best of both worlds -- the safety of a
     dynamic check, but without incurring redundant costs.
+
+Options
+-------
+
+.. option:: IgnoreSmartPointerDereference
+
+   If set to ``true`` (default is ``false``), the check ignores optionals that
+   are reached through overloaded smart-pointer-like dereference (``operator*``,
+   ``operator->``) on classes other than the optional type itself. This helps
+   avoid false positives where the analysis cannot equate results across such
+   calls. Note: This does not cover access through ``operator[]``.
+
+.. option:: IgnoreValueCalls
+
+   If set to ``true`` (default is ``false``), the check does not diagnose calls
+   to ``optional::value()``. Diagnostics for ``operator*()`` and
+   ``operator->()`` remain enabled. This is useful for codebases that
+   intentionally rely on ``value()`` for defined, guarded access while still
+   flagging UB-prone operator dereferences.

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

@@ -0,0 +1,25 @@
+// RUN: %check_clang_tidy %s bugprone-unchecked-optional-access %t -- \
+// RUN:   -config="{CheckOptions:  \
+// RUN:     {bugprone-unchecked-optional-access.IgnoreValueCalls: true}}" -- \
+// RUN:   -I %S/Inputs/unchecked-optional-access
+
+#include "absl/types/optional.h"
+
+struct Foo {
+  void foo() const {}
+};
+
+void unchecked_value_access(const absl::optional<int> &opt) {
+  opt.value(); // no-warning
+}
+
+void unchecked_deref_operator_access(const absl::optional<int> &opt) {
+  *opt;
+  // CHECK-MESSAGES: :[[@LINE-1]]:4: warning: unchecked access to optional value
+}
+
+void unchecked_arrow_operator_access(const absl::optional<Foo> &opt) {
+  opt->foo();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: unchecked access to optional value
+}
+

clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h

@@ -46,6 +46,13 @@ struct UncheckedOptionalAccessModelOptions {
   /// are confident in this const accessor caching, we shouldn't need the
   /// IgnoreSmartPointerDereference option anymore.
   bool IgnoreSmartPointerDereference = false;
+
+  /// In generating diagnostics, ignore calls to `optional::value()`.
+  ///
+  /// Projects that intentionally use `value()` as a guarded access pattern may
+  /// set this to true to suppress diagnostics for `value()` while continuing to
+  /// diagnose UB-prone operator accesses (`operator*`, `operator->`).
+  bool IgnoreValueCalls = false;
 };
 
 using UncheckedOptionalAccessLattice = CachedConstAccessorsLattice<NoopLattice>;

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

@@ -1154,6 +1154,21 @@ auto buildDiagnoseMatchSwitch(
   // lot of duplicated work (e.g. string comparisons), consider providing APIs
   // that avoid it through memoization.
   auto IgnorableOptional = ignorableOptional(Options);
+  if (Options.IgnoreValueCalls) {
+    // Only diagnose operator-based unwraps. Value calls are ignored.
+    return CFGMatchSwitchBuilder<
+               const Environment,
+               llvm::SmallVector<UncheckedOptionalAccessDiagnostic>>()
+        // optional::operator*, optional::operator->
+        .CaseOfCFGStmt<CallExpr>(valueOperatorCall(IgnorableOptional),
+                                 [](const CallExpr *E,
+                                    const MatchFinder::MatchResult &,
+                                    const Environment &Env) {
+                                   return diagnoseUnwrapCall(E->getArg(0), Env);
+                                 })
+        .Build();
+  }
+
   return CFGMatchSwitchBuilder<
              const Environment,
              llvm::SmallVector<UncheckedOptionalAccessDiagnostic>>()