Fix #4586: parser cache collides on closed generics#4587
Merged
Conversation
LinqParsing.FindMethodParser cached the result of resolving a custom IMethodCallParser by (Module, MetadataToken). For generic methods like Enumerable.Contains<T> the MetadataToken is the SAME regardless of the closed-over T — the token points to the generic *definition* in the metadata table, not the reified closed method. Consequence: the first parser that matched any T was returned for every subsequent T on the same generic method, even when the new T didn't satisfy the parser's Matches() guard. Concrete repro from the issue: a custom parser gated on expression.Method.GetGenericArguments()[0] == typeof(StrongId) matched Enumerable.Contains<StrongId> as designed — but a later Enumerable.Contains<string> query then hit the *same* cached StrongId-only parser instead of falling through to the standard handling. Fix: key the cache by MethodInfo. MethodInfo.Equals / GetHashCode on the closed reified method include the generic arguments, so each closed generic gets its own cache slot. Simplifies the cache to a single ImHashMap<MethodInfo, IMethodCallParser> — drops the (Module → int → parser) two-level map from the #4374 optimization since the per-module keying was only there to keep the int slot small and the int slot was the collision source. Test: LinqTests/Bugs/Bug_4586_parser_cache_collides_on_closed_generics mirrors the issue's repro. Inserts a custom Enumerable.Contains parser gated on T == Bug4586StrongId at position 0, runs a query that should hit it (asserts a controlled throw), then runs a second query with Enumerable.Contains<string> that must NOT hit the custom parser. Both before/after assertions on the parser's HitCount pin the contract. Regression sweep: custom_linq_extensions.query_with_custom_parser + 44 other Contains / IsOneOf / IsNotOneOf tests still pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This was referenced May 30, 2026
Merged
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.
Closes #4586.
Bug
LinqParsing.FindMethodParsercached the result of resolving a customIMethodCallParserby(Module, MetadataToken). For generic methods likeEnumerable.Contains<T>, theMetadataTokenis the same regardless of the closed-overT— the token points to the generic definition in the metadata table, not the reified closed method.Consequence: the first parser that matched any
Twas returned for every subsequentTon the same generic method, even when the newTdidn't satisfy the parser'sMatches()guard. From the issue's repro:Fix
Key the cache by
MethodInfo.MethodInfo.Equals/GetHashCodeon the closed reified method include the generic arguments, so each closed generic gets its own cache slot:FindMethodParsersimplifies to a single map lookup + AddOrUpdate. Drops the two-level(Module → int → parser)map from #4374 — that nesting only existed to keep the inner int slot small, and the int slot was the collision source.Test
LinqTests/Bugs/Bug_4586_parser_cache_collides_on_closed_generics.csmirrors the issue's repro:Enumerable.Containsparser gated onT == Bug4586StrongIdat position 0.HitCount == 1.Enumerable.Contains<string>— must NOT hit the custom parser; assertHitCountstays at 1 + the query round-trips normally.Regression check
custom_linq_extensions.query_with_custom_parser(the existing custom-parser path) + 44 otherContains/IsOneOf/IsNotOneOftests still pass locally.🤖 Generated with Claude Code