[clang-tidy] Add IgnoreValueCalls option to bugprone-unchecked-optional-access (#167209)

Add a new option `IgnoreValueCalls` to
`bugprone-unchecked-optional-access`

Closes [#163831](#163831)

Author: zeyi2

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

@@ -367,7 +367,11 @@ 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()`` and the `IgnoreSmartPointerDereference` option to
+  ignore optionals reached via smart-pointer-like dereference, 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`, 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. This does not cover access through ``operator[]``. Default is `false`.
+
+.. option:: IgnoreValueCalls
+
+   If set to `true`, 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. Default is `false`.

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,9 @@ 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()`.
+  bool IgnoreValueCalls = false;
 };
 
 using UncheckedOptionalAccessLattice = CachedConstAccessorsLattice<NoopLattice>;

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

@@ -1153,26 +1153,34 @@ auto buildDiagnoseMatchSwitch(
   // FIXME: Evaluate the efficiency of matchers. If using matchers results in a
   // lot of duplicated work (e.g. string comparisons), consider providing APIs
   // that avoid it through memoization.
-  auto IgnorableOptional = ignorableOptional(Options);
-  return CFGMatchSwitchBuilder<
-             const Environment,
-             llvm::SmallVector<UncheckedOptionalAccessDiagnostic>>()
-      // optional::value
-      .CaseOfCFGStmt<CXXMemberCallExpr>(
-          valueCall(IgnorableOptional),
-          [](const CXXMemberCallExpr *E, const MatchFinder::MatchResult &,
-             const Environment &Env) {
-            return diagnoseUnwrapCall(E->getImplicitObjectArgument(), Env);
-          })
-
-      // optional::operator*, optional::operator->
-      .CaseOfCFGStmt<CallExpr>(valueOperatorCall(IgnorableOptional),
-                               [](const CallExpr *E,
+  const auto IgnorableOptional = ignorableOptional(Options);
+
+  auto DiagBuilder =
+      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);
+              });
+
+  auto Builder = Options.IgnoreValueCalls
+                     ? std::move(DiagBuilder)
+                     : std::move(DiagBuilder)
+                           // optional::value
+                           .CaseOfCFGStmt<CXXMemberCallExpr>(
+                               valueCall(IgnorableOptional),
+                               [](const CXXMemberCallExpr *E,
                                   const MatchFinder::MatchResult &,
                                   const Environment &Env) {
-                                 return diagnoseUnwrapCall(E->getArg(0), Env);
-                               })
-      .Build();
+                                 return diagnoseUnwrapCall(
+                                     E->getImplicitObjectArgument(), Env);
+                               });
+
+  return std::move(Builder).Build();
 }
 
 } // namespace