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

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 496527.
ymandel added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143750/new/

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,27 @@
 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 behavior (either throwing an exception
+or terminating the program), 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.496527.patch
Type: text/x-patch
Size: 3136 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20230210/d8c13d9c/attachment-0001.bin>