super_diff icon indicating copy to clipboard operation
super_diff copied to clipboard

A failing spec with expect(value).to eq 1.0 raises JSON::ParserError

Open maxnotarangelo opened this issue 2 years ago • 5 comments

I'm getting a JSON::ParserError with the following code:

it "lets you compare a big float with another float" do
  expect(100_000.1).to eq 1.0
end

It looks like it's happening only when both numbers are floats, and the first is at least 100,000. The error is happening here: https://github.com/mcmire/super_diff/blob/d0ccdf204a54da14bfecdbb306008527c52f1686/lib/super_diff/helpers.rb#L55

The proximate cause is that

ObjectSpace.dump(100_000.1)
=> "100000."

I think this is a similar issue to #144.

maxnotarangelo avatar Nov 19 '22 05:11 maxnotarangelo

Hi @maxnotarangelo, thanks for the report, makes sense to me. I'll have to think about how best to fix this as I sense it may be a tricky one although I'm not sure.

mcmire avatar Nov 21 '22 21:11 mcmire

Hi @maxnotarangelo, just re-reviewing the issues list and ran across this issue. A fix for #144 was shipped in 0.9.0. Not sure if you still use the gem, but does that fix this issue for you?

mcmire avatar Feb 11 '23 22:02 mcmire

Hmm, I'm already on 0.9.0 and I'm still getting the same error.

maxnotarangelo avatar Feb 13 '23 15:02 maxnotarangelo

This is still happening for me on 0.12.1

maxnotarangelo avatar Sep 16 '24 18:09 maxnotarangelo

The problem is basically that for CRuby, object_address_for is trying to reimplement Object#inspect in Ruby code, without the low-level information available in the C code.

Before Ruby 2.7.0, object_id was derived directly from the memory address[^1], so one could reverse that to get the actual memory address. 2.7.0 introduced compaction, which can change an object's memory address, so we switched to using ObjectSpace.dump, which is sketchy:

Generally, you SHOULD NOT use this library if you do not know about the MRI implementation. Mainly, this library is for (memory) profiler developers and MRI developers who need to know about MRI memory usage.

ObjectSpace.dump happens to return JSON data. But relying on that format is quite brittle, and evidently floats aren't dumped as JSON anyway.

So, to solve this particular issue, let's just rescue JSON parse errors.

In the long term, we should either A) add a C extension to get the object's address in CRuby (seems like overkill if all we're trying to do is make things look more like the built-in #inspect output), or B) change that identifier to just be the object ID, assuming people don't care if it's the literal memory address.

[^1]: allegedly; I haven't confirmed this myself

jas14 avatar Oct 11 '24 22:10 jas14