doc: Remove confusing assert linter #28304

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2308-doc-assert- changing 2 files +1 −17

maflcko commented at 12:56 PM on August 21, 2023: member
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 #26684 (review)

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.
DrahtBot commented at 12:56 PM on August 21, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK hebasto, theStack

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Docs on Aug 21, 2023
maflcko added the label DrahtBot Guix build requested on Aug 22, 2023

Type	Reviewers
ACK	hebasto, theStack

DrahtBot commented at 12:39 AM on August 23, 2023: contributor

Guix builds

File	commit 38db2bd4e144065c515a222e1ae3d4ca7677e428<br>(master)	commit cbec0b14bdd4574087dea6ed5e9e8ad85d1bbd9b<br>(master and this pull)
SHA256SUMS.part	`740e6c5d9b4e167e...`	`42d4f41ff2fb8286...`
*-aarch64-linux-gnu-debug.tar.gz	`2eb6065bd57f1ce7...`	`1aebe12130d123f4...`
*-aarch64-linux-gnu.tar.gz	`52ed1288635185a1...`	`b0b4fb1d6ef39c6c...`
*-arm-linux-gnueabihf-debug.tar.gz	`0924c3d536c8f04a...`	`f05ac3f5c5933955...`
*-arm-linux-gnueabihf.tar.gz	`b9680d0dbad2e035...`	`b18506fc5b061c30...`
*-arm64-apple-darwin-unsigned.dmg	`eb09a41c7c60b4c7...`	`980252c0a7164cc5...`
*-arm64-apple-darwin-unsigned.tar.gz	`d0d1a44623583cfd...`	`3441acd18c653d6e...`
*-arm64-apple-darwin.tar.gz	`16e3ec6cc1b015a9...`	`0810ea82ab5b4e8b...`
*-powerpc64-linux-gnu-debug.tar.gz	`91f2a9ddb00e6e82...`	`760995df20345879...`
*-powerpc64-linux-gnu.tar.gz	`8adbb5294a61b01c...`	`0e714aad7b4b9b1b...`
*-powerpc64le-linux-gnu-debug.tar.gz	`f54633c2f9a6f7d0...`	`fa665e0f4c76c6a0...`
*-powerpc64le-linux-gnu.tar.gz	`a2ddb98e2d9985c3...`	`26071588767d47b5...`
*-riscv64-linux-gnu-debug.tar.gz	`53782894fd1d6b68...`	`c0193cbf61077516...`
*-riscv64-linux-gnu.tar.gz	`6d760fd88b76b69d...`	`0cde117605afaf24...`
*-x86_64-apple-darwin-unsigned.dmg	`f210299e59ed3553...`	`fb130a24cb2eca4d...`
*-x86_64-apple-darwin-unsigned.tar.gz	`9c7933f084d3fce3...`	`7f26c48ca06b0588...`
*-x86_64-apple-darwin.tar.gz	`9f545141b34e4bb2...`	`dc0f194f7f3911a0...`
*-x86_64-linux-gnu-debug.tar.gz	`b46979488366d6da...`	`67d3c77076a1358f...`
*-x86_64-linux-gnu.tar.gz	`de82001dabf95800...`	`97d14924de553f78...`
*.tar.gz	`bb6d491db6c6088c...`	`4dd59004af50f508...`
guix_build.log	`25a587482b817be2...`	`d443ec23c8d7fe76...`
guix_build.log.diff		`9cf88b432416a539...`

DrahtBot removed the label DrahtBot Guix build requested on Aug 23, 2023
maflcko requested review from theStack on Aug 27, 2023
DrahtBot added the label CI failed on Sep 3, 2023
DrahtBot removed the label CI failed on Sep 5, 2023
doc: Remove confusing assert linter fa6e6a3f03
maflcko force-pushed on Sep 14, 2023
hebasto commented at 10:53 AM on September 15, 2023: member

Concept ACK.
DrahtBot added the label CI failed on Sep 19, 2023
DrahtBot removed the label CI failed on Sep 20, 2023
maflcko requested review from hebasto on Sep 21, 2023
hebasto approved
hebasto commented at 1:37 PM on September 21, 2023: member

ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b, I have reviewed the code and it looks OK.

I agree with all three points mentioned in the PR description.
theStack approved
theStack commented at 12:11 AM on October 3, 2023: contributor

ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b
maflcko requested review from fanquake on Oct 3, 2023
fanquake merged this on Oct 3, 2023
fanquake closed this on Oct 3, 2023
fanquake commented at 9:45 AM on October 3, 2023: member

If we ever wanted to reintroduce something similar in future, using https://clang.llvm.org/extra/clang-tidy/checks/bugprone/assert-side-effect.html#bugprone-assert-side-effect is also likely more appropriate.
maflcko deleted the branch on Oct 3, 2023
Frank-GER referenced this in commit 87e1b7ee41 on Oct 13, 2023
PastaPastaPasta referenced this in commit 82091da3dc on Oct 24, 2024
PastaPastaPasta referenced this in commit d12f63c6d2 on Oct 24, 2024
PastaPastaPasta referenced this in commit 4101fea620 on Oct 25, 2024
PastaPastaPasta referenced this in commit bef82c4f03 on Oct 25, 2024
bitcoin locked this on Dec 3, 2024