[PATCH] D143750: [clang-tidy] Clarify documention of `bugprone-unchecked-optional-access`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: xazax.hun, gribozavr2, sgatev.
Herald added a subscriber: rnkovacs.
Herald added a reviewer: njames93.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang-tools-extra.

Removes a reference to google-internal document and expands the relevant material in place.

Fixes #60633.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D143750

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


Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/unchecked-optional-access.rst
@@ -8,16 +8,14 @@
 average clang-tidy check.
 
 This check identifies unsafe accesses to values contained in
-``std::optional<T>``, ``absl::optional<T>``, or ``base::std::optional<T>``
-objects. Below we will refer to all these types collectively as
-``optional<T>``.
+``std::optional<T>``, ``absl::optional<T>``, or ``base::Optional<T>``
+objects. Below we will refer to all these types collectively as ``optional<T>``.
 
-An access to the value of an ``optional<T>`` occurs when one of its
-``value``, ``operator*``, or ``operator->`` member functions is invoked.
-To align with common misconceptions, the check considers these member
-functions as equivalent, even though there are subtle differences
-related to exceptions versus undefined behavior. See
-go/optional-style-recommendations for more information on that topic.
+An access to the value of an ``optional<T>`` occurs when one of its ``value``,
+``operator*``, or ``operator->`` member functions is invoked.  To align with
+common misconceptions, the check considers these member functions as equivalent,
+even though there are subtle differences related to exceptions versus undefined
+behavior. See *Additional notes*, below, for more information on this topic.
 
 An access to the value of an ``optional<T>`` is considered safe if and only if
 code in the local scope (for example, a function body) ensures that the
@@ -273,3 +271,26 @@
 A future version will expand the scope to lambdas, following the rules
 outlined above. It is best to follow the same principles when using
 optionals in lambdas.
+
+Access with ``operator*()`` vs. ``value()``
+-------------------------------------------
+
+Given that ``value()`` has well-defined program termination behavior, why treat
+it the same as ``operator*()`` which causes undefined behavior (UB)? That is,
+why is it considered unsafe to access an optional with ``value()``, if it's not
+provably populated with a value?  For that matter, why is ``CHECK()`` followed
+by ``operator*()`` any better than ``value()``, given that they are semantically
+equivalent (on configurations that disable exceptions)?
+
+The answer is that we assume most users do not realize the difference between
+``value()`` and ``operator*()``. Shifting to ``operator*()`` and some form of
+explicit value-presence check or explicit program termination has two
+advantages:
+
+  * Readability. The check, and any potential side effects like program
+    shutdown, are very clear in the code. Separating access from checks can
+    actually make the checks more obvious.
+
+  * 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.


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D143750.496502.patch
Type: text/x-patch
Size: 3097 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230210/35bc35bd/attachment.bin>