================ @@ -0,0 +1,377 @@ +.. title:: clang-tidy - abseil-unchecked-statusor-access + +abseil-unchecked-statusor-access +================================ + +This check identifies unsafe accesses to values contained in +``absl::StatusOr<T>`` objects. Below we will refer to this type as +``StatusOr<T>``. + +An access to the value of an ``StatusOr<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 vs. undefined behavior. + +An access to the value of a ``StatusOr<T>`` is considered safe if and +only if code in the local scope (e.g. function body) ensures that the +status of the ``StatusOr<T>`` is ok in all possible execution paths that +can reach the access. That should happen either through an explicit +check, using the ``StatusOr<T>::ok`` member function, or by constructing +the ``StatusOr<T>`` in a way that shows that its status is unambiguously +ok (e.g. by passing a value to its constructor). + +Below we list some examples of safe and unsafe ``StatusOr<T>`` access +patterns. + +Note: If the check isn’t behaving as you would have expected on a code +snippet, please `report it <http://github.com/llvm/llvm-project/issues/new>`__. + +False negatives +--------------- + +This check generally does **not** generate false negatives. If it cannot +prove an access safe, it is assumed to be unsafe. That being said, there +are some heuristics used that in very rare cases might be incorrect: + +- `a const method accessor (without arguments) that returns different + values when called multiple times <#functionstability>`__. + +If you think the check generated a false negative, please `report +it <http://github.com/llvm/llvm-project/issues/new>`__. + +Known limitations +----------------- + +This is a non-exhaustive list of constructs that are currently not +modelled in the check and will lead to false positives: + +- `Checking a StatusOr and then capturing it in a lambda <#lambdas>`__ +- `Indexing into a container with the same index <#containers>`__ +- `Project specific helper-functions <#uncommonapi>`__, +- `Functions with a stable return value <#functionstability>`__ +- **Any** `cross-function reasoning <#crossfunction>`__. This is by + design and will not change in the future. + +Checking if the status is ok, then accessing the value +------------------------------------------------------ + +The check recognizes all straightforward ways for checking the status +and accessing the value contained in a ``StatusOr<T>`` object. For +example: + +.. code:: cpp + + void f(absl::StatusOr<int> sor) { + if (sor.ok()) { + use(*sor); + } + } + +Checking if the status is ok, then accessing the value from a copy +------------------------------------------------------------------ + +The criteria that the check uses is semantic, not syntactic. It +recognizes when a copy of the ``StatusOr<T>`` object being accessed is +known to have ok status. For example: + +.. code:: cpp + + void f(absl::StatusOr<int> sor1) { + if (sor1.ok()) { + absl::optional<int> sor2 = sor1; + use(*sor2); + } + } + +Ensuring that the status is ok using common macros +-------------------------------------------------- + +The check is aware of common macros like ``ABSL_CHECK`` and ``ASSERT_THAT``. +Those can be used to ensure that the status of a ``StatusOr<T>`` object +is ok. For example: + +.. code:: cpp + + void f(absl::StatusOr<int> sor) { + ABSL_DCHECK_OK(sor); + use(*sor); + } + +Ensuring that the status is ok, then accessing the value in a correlated branch +------------------------------------------------------------------------------- + +The check is aware of correlated branches in the code and can figure out +when a ``StatusOr<T>`` object is ensured to have ok status on all +execution paths that lead to an access. For example: + +.. code:: cpp + + void f(absl::StatusOr<int> sor) { + bool safe = false; + if (sor.ok() && SomeOtherCondition()) { + safe = true; + } + // ... more code... + if (safe) { + use(*sor); + } + } + +Accessing the value without checking the status +----------------------------------------------- + +The check flags accesses to the value that are not locally guarded by a +status check: + +.. code:: cpp + + void f1(absl::StatusOr<int> sor) { + use(*sor); // unsafe: it is unclear whether the status of `sor` is ok. + } + + void f2(absl::StatusOr<MyStruct> sor) { + use(sor->member); // unsafe: it is unclear whether the status of `sor` is ok. + } + + void f3(absl::StatusOr<int> sor) { + use(sor.value()); // unsafe: it is unclear whether the status of `sor` is ok. + } + +Use ``ABSL_CHECK_OK`` to signal that you knowingly want to crash on +non-OK values. + +NOTE: Even though using ``.value()`` on a ``nullopt`` is defined to crash, ---------------- jvoung wrote:
Is `nullopt` the right term for StatusOr, or something more like "no value is present" / "status is not ok"? https://github.com/llvm/llvm-project/pull/171188 _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
