Fix ValueLazy caching being defeated by readonly fields#706
Merged
tannergooding merged 1 commit intoJul 12, 2026
Merged
Conversation
ValueLazy<T> is a mutable struct: its Value getter computes the value on first access and clears the factory so subsequent accesses return the cached value. Every field storing one was declared readonly, so each access operated on a defensive copy and the cached result was discarded -- the factory ran again on every access, defeating the memoization the type exists to provide. Drop readonly from all ValueLazy<T> fields so the cached value persists. The factories are idempotent, so behavior is unchanged; access to lazy members (spellings, parents, translation units, etc.) no longer recomputes each time. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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.
ValueLazy<T>is a mutable struct: itsValuegetter computes the value on first access and clears the factory so subsequent accesses return the cached value. Every field storing one was declaredreadonly, so each access operated on a defensive copy and the cached result was discarded -- the factory ran again on every access, defeating the memoization the type exists to provide. This is the classic mutable-struct-in-a-readonly-field bug.Dropping
readonlyfrom all 326ValueLazy<T>fields (170 files) lets the cached value persist. The factories are idempotent, so behavior is unchanged; access to lazy members (spellings, parents, translation units, etc.) simply no longer recomputes each time -- which avoids repeated nativeclang_*calls,GetOrCreatelookups, and string marshaling.Measured on a representative real-world workload -- the
d3d12generation from terrafx/terrafx.interop.windows against the Windows SDK:I audited the rest of the runtime, generator, and interop layers for the same pattern: the descriptor structs (
StructDesc,ValueDesc,FunctionOrDelegateDesc, etc.) are mutable but only ever used as locals or by-value parameters, andLazyListis a sealed class --ValueLazywas the only type caught in this trap.All 3706 tests pass with a 0-warning build.