Move depth to generate_json_data

[etiennebarrie]

I want to remove the memcpy of generate_json_data and remove the FIXME by putting the mutating depth where it should be, in generate_json_data. The State object itself still has a depth, for configuration.

The idea is to have a new generate_json_data::depth field and stop mutating the old JSON_Generator_State::depth one, and then at the end, copy the depth back to it. This doesn't need to happen in all cases, e.g. generate_new doesn't need to copy it back, and this is what allows us not to do the memcpy it in the first place.

This still keeps generate_new to let it have different behaviors between JSON::Coder (calling dump does not result in the depth of the State changing if a JSON::NestingError occurs) and JSON::State (which does). I called that last behavior deprecated because I have another PR to start deprecating this behavior.

[byroot]

So this may be fine, but the reason I've put a TODO rather than a quick fix, is that until now depth did get transfered across to_json invocations, because the State with its depth is passed to it.

If we move the depth outside it, you can now end up with a stack overflow if you are calling to_json recursively. Or am I missing something?

[etiennebarrie]

Oh right there's definitely a missing case here:

  # This object is serialized to its depth in the JSON document.
  class DepthToJSON
    def to_json(state)
      state = JSON::State.from_state(state)
      state.depth.to_s
    end
  end

  def test_state_depth
    depth = DepthToJSON.new
    assert_equal "[1]", JSON.generate([depth])
end

This passes on main but not on my branch.

I think maybe through vstate_get we could copy back generate_json_data::depth into JSON_Generator_State::depth when necessary.

Can you share an example though? I'm not sure what you mean by calling to_json recursively.

[byroot]

Can you share an example though?

I was thinking of something like this:

require "json"

class Recur
  def to_json(state = nil, *)
    JSON::State.from_state(state).generate([self])
  end
end

JSON.generate(Recur.new, max_nesting: 10)

Right now it fails with:

/tmp/j.rb:5:in 'JSON::Ext::Generator::State#generate': nesting of 10 is too deep. Did you try to serialize objects with circular references? (JSON::NestingError)

Which is desirable. If we don't forward the depth, then I think it will end up in a much less nice SystemStackError

[etiennebarrie]

Correct, the branch as it was did turn the NestingError into a SystemStackError.
However, now that I copy the depth into the State before calling to_json, that part is fine too.
And that only happens when strict is not set, to_json is used so Coder won't be impacted.