llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-tidy Author: Rohan Jacob-Rao (rohanjr) <details> <summary>Changes</summary> Specifically: 1. Avoid the "or" suffix for variable names per [abseil.io/tips/181](https://abseil.io/tips/181) 2. Replace DCHECK with CHECK which works in non-debug mode 3. Suggest init-capture in workaround for lambda captures --- Full diff: https://github.com/llvm/llvm-project/pull/176498.diff 1 Files Affected: - (modified) clang-tools-extra/docs/clang-tidy/checks/abseil/unchecked-statusor-access.rst (+59-59) ``````````diff diff --git a/clang-tools-extra/docs/clang-tidy/checks/abseil/unchecked-statusor-access.rst b/clang-tools-extra/docs/clang-tidy/checks/abseil/unchecked-statusor-access.rst index 0f1a0e55af022..d45743d51f279 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/abseil/unchecked-statusor-access.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/abseil/unchecked-statusor-access.rst @@ -68,9 +68,9 @@ example: .. code:: cpp - void f(absl::StatusOr<int> sor) { - if (sor.ok()) { - use(*sor); + void f(absl::StatusOr<int> x) { + if (x.ok()) { + use(*x); } } @@ -83,10 +83,10 @@ 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); + void f(absl::StatusOr<int> x1) { + if (x1.ok()) { + absl::optional<int> x2 = x1; + use(*x2); } } @@ -99,9 +99,9 @@ is ok. For example: .. code:: cpp - void f(absl::StatusOr<int> sor) { - ABSL_DCHECK_OK(sor); - use(*sor); + void f(absl::StatusOr<int> x) { + ABSL_CHECK_OK(x); + use(*x); } Ensuring that the status is ok, then accessing the value in a correlated branch @@ -113,14 +113,14 @@ execution paths that lead to an access. For example: .. code:: cpp - void f(absl::StatusOr<int> sor) { + void f(absl::StatusOr<int> x) { bool safe = false; - if (sor.ok() && SomeOtherCondition()) { + if (x.ok() && SomeOtherCondition()) { safe = true; } // ... more code... if (safe) { - use(*sor); + use(*x); } } @@ -132,16 +132,16 @@ status check: .. code:: cpp - void f1(absl::StatusOr<int> sor) { - use(*sor); // unsafe: it is unclear whether the status of `sor` is ok. + void f1(absl::StatusOr<int> x) { + use(*x); // unsafe: it is unclear whether the status of `x` is ok. } - void f2(absl::StatusOr<MyStruct> sor) { - use(sor->member); // unsafe: it is unclear whether the status of `sor` is ok. + void f2(absl::StatusOr<MyStruct> s) { + use(s->member); // unsafe: it is unclear whether the status of `s` is ok. } - void f3(absl::StatusOr<int> sor) { - use(sor.value()); // unsafe: it is unclear whether the status of `sor` is ok. + void f3(absl::StatusOr<int> x) { + use(x.value()); // unsafe: it is unclear whether the status of `x` is ok. } Use ``ABSL_CHECK_OK`` to signal that you knowingly want to crash on @@ -159,10 +159,10 @@ branches of the code. For example: .. code:: cpp - void f(absl::StatusOr<int> sor) { - if (sor.ok()) { + void f(absl::StatusOr<int> x) { + if (x.ok()) { } else { - use(*sor); // unsafe: it is clear that the status of `sor` is *not* ok. + use(*x); // unsafe: it is clear that the status of `x` is *not* ok. } } @@ -178,8 +178,8 @@ For example: .. code:: cpp void f(Foo foo) { - if (foo.sor().ok()) { - use(*foo.sor()); // unsafe: it is unclear whether the status of `foo.sor()` is ok. + if (foo.x().ok()) { + use(*foo.x()); // unsafe: it is unclear whether the status of `foo.x()` is ok. } } @@ -189,8 +189,8 @@ local variable and use it to access the value. For example: .. code:: cpp void f(Foo foo) { - if (const auto& foo_sor = foo.sor(); foo_sor.ok()) { - use(*foo_sor); + if (const auto& x = foo.x(); x.ok()) { + use(*x); } } @@ -225,7 +225,7 @@ assumes the return value of the accessor was mutated. void f(Foo foo) { if (foo.get().ok()) { foo.mutate(); - use(*foo.get()); // unsafe: mutate might have changed the state of the object + use(*foo.get()); // unsafe: `mutate()` might have changed the state of the object } } @@ -267,13 +267,13 @@ of the ``StatusOr<T>`` is ok. For example: .. code:: cpp - void g(absl::StatusOr<int> sor) { - use(*sor); // unsafe: it is unclear whether the status of `sor` is ok. + void g(absl::StatusOr<int> x) { + use(*x); // unsafe: it is unclear whether the status of `x` is ok. } - void f(absl::StatusOr<int> sor) { - if (sor.ok()) { - g(sor); + void f(absl::StatusOr<int> x) { + if (x.ok()) { + g(x); } } @@ -287,9 +287,9 @@ local scope of the callee. For example: use(val); } - void f(absl::StatusOr<int> sor) { - if (sor.ok()) { - g(*sor); + void f(absl::StatusOr<int> x) { + if (x.ok()) { + g(*x); } } @@ -303,8 +303,8 @@ via ``using`` declarations. For example: using StatusOrInt = absl::StatusOr<int>; - void f(StatusOrInt sor) { - use(*sor); // unsafe: it is unclear whether the status of `sor` is ok. + void f(StatusOrInt x) { + use(*x); // unsafe: it is unclear whether the status of `x` is ok. } Containers @@ -317,9 +317,9 @@ example: .. code:: cpp - void f(std::vector<absl::StatusOr<int>> sors) { - if (sors[0].ok()) { - use(*sors[0]); // unsafe: it is unclear whether the status of `sors[0]` is ok. + void f(std::vector<absl::StatusOr<int>> xs) { + if (xs[0].ok()) { + use(*xs[0]); // unsafe: it is unclear whether the status of `xs[0]` is ok. } } @@ -328,10 +328,10 @@ instead: .. code:: cpp - void f(std::vector<absl::StatusOr<int>> sors) { - absl::StatusOr<int>& sor0 = sors[0]; - if (sor0.ok()) { - use(*sor0); + void f(std::vector<absl::StatusOr<int>> xs) { + absl::StatusOr<int>& x0 = xs[0]; + if (x0.ok()) { + use(*x0); } } @@ -348,24 +348,24 @@ pattern will be reported as an unsafe access: .. code:: cpp - void f(absl::StatusOr<int> sor) { - if (sor.ok()) { - [&sor]() { - use(*sor); // unsafe: it is unclear whether the status of `sor` is ok. + void f(absl::StatusOr<int> x) { + if (x.ok()) { + [&x]() { + use(*x); // unsafe: it is unclear whether the status of `x` is ok. } } } -To avoid the issue, you should grab a reference to the contained object -and capture that instead +To avoid the issue, you should instead capture the contained object, +either by value or by reference. An init-capture is useful for this, +here capturing by reference: .. code:: cpp - void f(absl::StatusOr<int> sor) { - if (sor.ok()) { - auto& s = *sor; - [&s]() { - use(s); + void f(absl::StatusOr<int> x) { + if (x.ok()) { + [&x = *x]() { + use(x); } } } @@ -375,10 +375,10 @@ accessed: .. code:: cpp - void f(absl::StatusOr<int> sor) { - [&sor]() { - if (sor.ok()) { - use(*sor); + void f(absl::StatusOr<int> x) { + [&x]() { + if (x.ok()) { + use(*x); } } } `````````` </details> https://github.com/llvm/llvm-project/pull/176498 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
