Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix support for dynamic keys. #8273

Merged
merged 4 commits into from
Aug 24, 2023

Conversation

ioquatix
Copy link
Member

@ioquatix ioquatix commented Aug 24, 2023

Unfortunately there is a bug in Fiber[key] = value when key is dynamically defined. I didn't fully understand the implementation of rb_check_id.

https://bugs.ruby-lang.org/issues/19845

@ioquatix ioquatix requested a review from nobu August 24, 2023 01:33
@ioquatix ioquatix merged commit d4c720a into ruby:master Aug 24, 2023
90 checks passed
@ioquatix ioquatix deleted the fiber-storage-dynamic-keys branch August 24, 2023 03:19
@eregon
Copy link
Member

eregon commented Aug 24, 2023

👍 it should only allow Symbols, I think that's how we discussed it in the past, and so much simpler.

@ioquatix
Copy link
Member Author

Yeah, totally agree in retrospect. I monkey patched a fix in my fiber-storage shim: ioquatix/fiber-storage@c98221b

nagachika added a commit to nagachika/ruby that referenced this pull request Sep 30, 2023
	Fix support for dynamic keys. (ruby#8273)

	* Skip RBS test.
	---
	 cont.c                               |  8 +++-----
	 spec/ruby/core/fiber/storage_spec.rb | 14 ++++++++++++++
	 tool/rbs_skip_tests                  |  2 ++
	 3 files changed, 19 insertions(+), 5 deletions(-)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants