Skip to content

Include receiver and complete options in the instance cache key - #369

Open
OskarEichler wants to merge 1 commit into
ruby:mainfrom
OskarEichler:codex/csv-instance_cache
Open

Include receiver and complete options in the instance cache key#369
OskarEichler wants to merge 1 commit into
ruby:mainfrom
OskarEichler:codex/csv-instance_cache

Conversation

@OskarEichler

Copy link
Copy Markdown

Summary

Key cached CSV objects by their receiver class, input identity, and full option hash. This also distinguishes explicitly supplied nil values from omitted options.

Reproduction

On the same string, calls with encoding: "UTF-8" and encoding: "ISO-8859-1" incorrectly return one object. Calling CSV.instance before CSV::TSV.instance returns a CSV object for the TSV call. Unknown options can also bypass validation on a cache hit.

Verification

10 focused checks: six failures on the release, zero with the patch, covering encodings, subclass separation, option order, explicit nil, unknown options, cache reuse and block returns.

This isolated patch passes the unchanged upstream suite in both normal and CSV_PARSER_SCANNER_TEST=yes modes: 527 tests / 4,050 assertions in each mode, zero failures/errors. Commands use Ruby 4.0.6 through rbenv (bundle exec ruby run-test.rb). Syntax and git diff --check pass. The combined release-based consumer branch also passes gem build and rake warning:error rdoc.

No tests were added or changed, per the requesting project's explicit no-new-tests policy. Focused checks and measurements live outside this repository. The upstream no-RuboCop policy is respected. Verification was local on macOS/Ruby 4.0.6; the supported OS/Ruby matrix was not run locally. Current main and installed 3.3.6 share base commit 0873ab362d996f12796c3c3e8998b5be657a9b12; no unrelated changes are included.

Breaking changes and limitations

Calls with different receiver/options now get different objects. Code depending on accidental cache collisions will observe corrected identity behavior. Cache lifetime and the documented lack of Ractor safety are unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant