Skip to content

Add specs for Kernel.Float with hex strings #1251

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

Merged
merged 1 commit into from
May 29, 2025

Conversation

herwinw
Copy link
Member

@herwinw herwinw commented Feb 28, 2025

They are very much driven by the current (inconsistent) behaviour of MRI. I'm not sure if these should be seen as a specification or as MRI specific implementation quirks.

@herwinw
Copy link
Member Author

herwinw commented Feb 28, 2025

https://bugs.ruby-lang.org/issues/21163 is a related upstream case where I pointed out the inconsistencies.

@herwinw
Copy link
Member Author

herwinw commented Apr 26, 2025

I've updated the PR to reflect the changes in Ruby 3.4.3. It now contains checks like this:

ruby_version_is ""..."3.4.3" do
  it "does not accept embedded _ if the number contains a-f" do
    -> { @object.send(:Float, "0x1_0a") }.should raise_error(ArgumentError)
  end
end

ruby_version_is "3.4.3" do
  it "accepts embedded _ if the number contains a-f" do
    @object.send(:Float, "0x1_0a").should == 0x10a.to_f
  end
end

I'm not sure if this should be checked with a version guard, or with a ruby_bug. It's not the kind of fix I would expect in a patchlevel release.

@herwinw herwinw force-pushed the kernel_float_hex branch from 64d18ba to a9595b6 Compare May 29, 2025 08:52
@eregon
Copy link
Member

eregon commented May 29, 2025

I'm not sure if this should be checked with a version guard, or with a ruby_bug. It's not the kind of fix I would expect in a patchlevel release.

If not likely to be backported then yes a version guard is correct.

I'm not sure if these should be seen as a specification or as MRI specific implementation quirks.

Regardless it's good to test it, and indeed report weird behavior upstream.
If something looks strange in the specs adding a code comment with a link to the issue is helpful.

@eregon
Copy link
Member

eregon commented May 29, 2025

Ah actually it got backport, but only to 3.4.3. Yeah, what you did is right.

@eregon eregon merged commit 8732759 into ruby:master May 29, 2025
12 checks passed
@herwinw herwinw deleted the kernel_float_hex branch May 30, 2025 05:50
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.

3 participants