Fix/options and pools - #19
Merged
Merged
Conversation
…faults applyWithDefaults(o options, opts []Option) overwrote o with defaultOptions before reading it, so the parameter never carried a value. staticcheck SA4009 and unparam both reported that, and a //nolint directive silenced them. The parameter was meant to avoid an allocation that does not exist: options holds no pointer field, so the seed copy and the returned value travel in registers and never reach the heap. Escape analysis reports only "opts does not escape", with or without inlining. Seeding from defaultOptions unconditionally is load-bearing on the pooled path: it stops a borrowed L from inheriting the previous borrower's settings. The doc comment now records that. Both the JSON and the YAML lexer carried the same signature. TestBorrowWithOptionsAllocFree pins the zero-alloc property the parameter was supposed to protect, so an option that captures a slice — or a call site the compiler stops inlining — fails the suite instead of passing unnoticed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Mirrors the JSON lexer's pool: a pools.PoolRedeemable[YL] whose borrow yields a cached redeem closure, so borrowing costs no allocation. The pool amortizes the YL and the capacity of its toks slice, not the parse. build hands the source to goccy/go-yaml, which allocates a fresh AST every time, so a borrow→lex→redeem cycle lowers the allocation count of a repeated lex; it does not bring it to zero the way the JSON lexer's pool does. The doc comments on BorrowLexerWithBytes say so, and the alloc guard measures a borrow→redeem cycle rather than a full lex. YL.Reset dropped l.data but left l.toks holding token values that alias the same buffer, so a pooled lexer would have pinned the caller's source while idle. reset now clears the entries before shortening the slice. Clearing at len rather than cap is enough: every entry a build appends is cleared by the next reset. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Applies the two-line header from NOTICE to all 195 .go and .s files, which had none. The form and the blank line separating it from a package doc comment come from go-openapi/swag; as there, .yml, .sh and .md files are left alone. Generated files carry it from their generator, so it survives go generate: - lexgen prepends it to the banner it writes into scan_gen.go; - both avo programs prepend it to the .s named by avo's -out flag, since printer.Config exposes no hook for extra header lines and a header written into the .s by hand would be dropped on the next run. Regenerating scan_gen.go, validate_amd64.s and stringstop_amd64.s adds the three header lines and changes nothing else. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Frederic BIDON <fredbi@yahoo.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #19 +/- ##
==========================================
- Coverage 69.19% 69.06% -0.13%
==========================================
Files 90 91 +1
Lines 9681 9728 +47
==========================================
+ Hits 6699 6719 +20
- Misses 2973 3000 +27
Partials 9 9 ☔ View full report in Codecov by Harness. |
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
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.
Change type
Please select: 🆕 New feature or enhancement|🔧 Bug fix'|📃 Documentation update
Short description
Fixes
Full description
Checklist