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.
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.
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
