From d12f63c6d2e69bfb6656361347757e90b134dfcd Mon Sep 17 00:00:00 2001 From: fanquake Date: Tue, 3 Oct 2023 10:43:41 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#28304: doc: Remove confusing assert linter fa6e6a3f03a38f8b431bf694268ed344d1815b3b doc: Remove confusing assert linter (MarcoFalke) Pull request description: The `assert()` documentation and linter are redundant and confusing: * The source code already refuses to compile with `assert()` disabled. * They violate the assumptions about `Assert()`, which *requires* side effects. * The existing linter doesn't enforce the guideline, only checking for `++` and `--` side effects. Fix all issues by removing the docs and the linter. See also https://github.com/bitcoin/bitcoin/pull/26684#discussion_r1287370102 Going forward everyone is free to use whatever code in this regard they think is the easiest to read. Also, everyone is still free to share style-nits, if they think it is a good use of their time and of the pull request author. Finally, the author is still free to dismiss or ignore this style-nit, or any other style-nit. ACKs for top commit: hebasto: ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b, I have reviewed the code and it looks OK. theStack: ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b Tree-SHA512: 686738d71e1316cc95e5d3f71869b55a02bfb137c795cc0875057f4410e564bc8eff03c985a2087b007fb08fc84551c7da1e8b30c7a9c3f2b14e5e44a5970236 --- doc/developer-notes.md | 6 ------ test/lint/lint-assertions.py | 12 +----------- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/doc/developer-notes.md b/doc/developer-notes.md index 5f5bcc129ecfe3..a6b19e2e7a5ec5 100644 --- a/doc/developer-notes.md +++ b/doc/developer-notes.md @@ -717,12 +717,6 @@ Common misconceptions are clarified in those sections: - Passing (non-)fundamental types in the [C++ Core Guideline](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rf-conventional). -- Assertions should not have side-effects. - - - *Rationale*: Even though the source code is set to refuse to compile - with assertions disabled, having side-effects in assertions is unexpected and - makes the code harder to understand. - - If you use the `.h`, you must link the `.cpp`. - *Rationale*: Include files define the interface for the code in implementation files. Including one but diff --git a/test/lint/lint-assertions.py b/test/lint/lint-assertions.py index 195ff33d1127d8..7731dc7901da4f 100755 --- a/test/lint/lint-assertions.py +++ b/test/lint/lint-assertions.py @@ -23,20 +23,10 @@ def git_grep(params: [], error_msg: ""): def main(): - # PRE31-C (SEI CERT C Coding Standard): - # "Assertions should not contain assignments, increment, or decrement operators." - exit_code = git_grep([ - "-E", - r"[^_]assert\(.*(\+\+|\-\-|[^=!<>]=[^=!<>]).*\);", - "--", - "*.cpp", - "*.h", - ], "Assertions should not have side effects:") - # Aborting the whole process is undesirable for RPC code. So nonfatal # checks should be used over assert. See: src/util/check.h # src/rpc/server.cpp is excluded from this check since it's mostly meta-code. - exit_code |= git_grep([ + exit_code = git_grep([ "-nE", r"\<(A|a)ss(ume|ert) *\(.*\);", "--",