Document argument evaluation cost and add generated-stub CI check - #2
Merged
Conversation
Document that the zero-overhead guarantee applies to the assertion machinery, not to the expression passed in. Go evaluates arguments before the no-op call, so a computed or side-effecting condition still runs in release builds. Point users at TrueFn to defer expensive work. Add a check-generate Makefile target and a CI step so the build fails if bumert_release.go is out of date with respect to the debug source.
Pass the value being asserted to Should() and reference it inside the closure, instead of the confusing Should(nil) idiom.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two small things that came out of a closer read of the zero-overhead claim.
1. Document that arguments are still evaluated in release builds
The "zero overhead" guarantee is real for the assertion machinery: in a release build the calls inline away to nothing and there are no allocations. I confirmed this by disassembling a release binary,
Should(ptr).NotBeNil()andShould(n).BeGreaterThan(0)compile to zero instructions.What is not removed is the expression you pass in. Go evaluates function arguments before the call, and a no-op cannot un-evaluate them. This differs from a C-style
assert()macro, which the preprocessor deletes entirely underNDEBUG. So a computed or side-effecting condition still runs in production:Verified empirically: with this line in a release build the side effect still fires, and the disassembly shows the only instruction left on that line is the
CALLto the argument function.The README now spells this out and points readers at
TrueFn, which defers the work behind a closure that is never called in release builds. Also verified: theTrueFnbody runs only under-tags debug.2. Add a CI guard for the generated release stubs
bumert_release.gois generated frombumert_debug.go. If someone adds a debug assertion and forgets to runmake generate, the release build can reference a missing stub. CI already checks formatting and tidiness but not stub drift.Added a
check-generateMakefile target and aCheck Generated StubsCI step that runsmake generateand fails ifbumert_release.gochanged.Changes
README.md: new section explaining the check-is-free-but-argument-is-not distinction, with theTrueFnworkaround.Makefile: newcheck-generatetarget..github/workflows/ci.yml: new step runningmake check-generatebefore the test jobs.No functional changes to the library.