llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: mitchell (zeyi2) <details> <summary>Changes</summary> Add a new option `IgnoreValueCalls` to `bugprone-unchecked-optional-access` Closes [#<!-- -->163831](https://github.com/llvm/llvm-project/issues/163831) --- Full diff: https://github.com/llvm/llvm-project/pull/167209.diff 6 Files Affected: - (modified) clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h (+3-1) - (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4-1) - (modified) clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst (+19) - (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access-ignore-value.cpp (+25) - (modified) clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h (+7) - (modified) clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp (+15) ``````````diff diff --git a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h b/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h index 11086fb4bfda1..62bf42da4f9f9 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h +++ b/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: diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 666865cfb2fcd..7e1e1802ade22 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/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 diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst index 552e6db699696..76078571f8c2d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst +++ b/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. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access-ignore-value.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unchecked-optional-access-ignore-value.cpp new file mode 100644 index 0000000000000..f54621269f8c0 --- /dev/null +++ b/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 +} + diff --git a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h b/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h index 696c9f4a6cf5c..07fa95fe81059 100644 --- a/clang/include/clang/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.h +++ b/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>; diff --git a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp b/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp index 0fa333eedcfdd..f4dacfb0bcff4 100644 --- a/clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp +++ b/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>>() `````````` </details> https://github.com/llvm/llvm-project/pull/167209 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
