Skip to content

Commit

Permalink
Soften some language in styleguide/c++/checks.md
Browse files Browse the repository at this point in the history
A paragraph saying that we reserve the right to upgrade DCHECKs to
CHECKs is removed as it suggests that changes will be made to DCHECKs
under the hood bypassing code reviewers. This may have been discouraging
the addition of DCHECKs in the first place.

The intent here is to suggest CHECKs over DCHECKs, it should never be
interpreted as discouraging the addition of CHECKs in the first place.

Bug: None
Change-Id: Ic81114de969e6a9c33c0f4e43a01d3aa49a1f3b0
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4363704
Reviewed-by: Peter Kasting <pkasting@chromium.org>
Commit-Queue: Peter Boström <pbos@chromium.org>
Reviewed-by: Nasko Oskov <nasko@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1120887}
  • Loading branch information
pbos authored and Chromium LUCI CQ committed Mar 22, 2023
1 parent c572653 commit 83b08a7
Showing 1 changed file with 3 additions and 6 deletions.
9 changes: 3 additions & 6 deletions styleguide/c++/checks.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ being violated. All invariant failures should be seen as P1 bugs, regardless of
their crash rate. Continuing past an invariant failure can cause crashes and
incorrect behaviour for our users, but also frequently presents security
vulnerabilities as attackers may leverage the unexpected state to take control
of the program. We also reserve the right to let the compiler assume and
optimize around `DCHECK()`s holding true in non-DCHECK builds using
`__builtin_assume()`, which even further formalizes undefined behavior.
of the program. In the future we may let the compiler assume and optimize around
`DCHECK()`s holding true in non-DCHECK builds using `__builtin_assume()`, which
further formalizes undefined behavior.

Prefer `CHECK()` and `NOTREACHED_NORETURN()` as they ensure that if an invariant
fails, the program does not continue in an unexpected state, and we hear about
Expand Down Expand Up @@ -47,9 +47,6 @@ that these are in fact commonly reached. Prefer `NOTREACHED_NORETURN()` in new
code, while we migrate the preexisting cases to it with care. See
https://crbug.com/851128.

We reserve the right to make any `DCHECK()` or `NOTREACHED()` statements fatal
in production builds, subject to performance and code-size constraints.

Below are some examples to explore the choice of `CHECK()` and its variants:

```c++
Expand Down

0 comments on commit 83b08a7

Please sign in to comment.