Corefxlab: JsonDynamicObject and JsonLazyDynamicObject need to be re-designed

[ahsonkhan]

JsonDynamicObject wraps around Dictionary<JsonProperty, JsonValue> and JsonProperty & JsonValue both contain Utf8String fields, which are stack-only, and hence cannot be used as a generic type in Dictionary<T>.

Also JsonProperty implements IEquatable<JsonProperty> which is not allowed for the same reason.

JsonLazyDynamicObject is a class and has a JsonObject field which contains a ReadOnlySpan<byte> field. A value type containing a by-ref instance field, such as Span, cannot be used as the type for a class instance field.

Edit: With the new compiler that enforces correct Span usage, incorrect uses have been commented out. From https://github.com/dotnet/corefxlab/pull/1608:
https://github.com/dotnet/corefxlab/pull/1608/files#diff-d7c8e1ff012098966b3cdaa243f77ae0R229
C# //TODO: no spans on the heap

These should be resolved as part of this issue.

[ahsonkhan]

cc @KrzysztofCwalina, @shiftylogic, @joshfree

[KodrAus]

I'd be interested in getting on board with this.

[ahsonkhan]

I'd be interested in getting on board with this.

Awesome :)

[KodrAus]

So currently JsonObject doesn't own its values, which is reasonable, but comes with those caveats. Maybe we could layer it so there's a lower-level borrowed layer that requires an input Span for building up the db and for reading values etc, and a wrapper around that that requires owned buffers and has an ergonomic API for clients?

[KodrAus]

So I guess the design here could depend on the design for Buffer that comes out of #1437, but in the case where Buffer is just a 'fat span' I think changing Spans in JsonObject to Buffers is a bit sketchy. Storing Buffers in fields would be problematic because you can't guarantee their lifetime and if that storage is opaque to API users then they might not even know you're doing it and leave you with a dangling pointer.

1447 may also affect this if the design for utf8-strings changes.

I'm still keen to play with the idea of an owned and borrowed variant for json objects.

[KodrAus]

Does anybody else have any thoughts on this? @shiftylogic?

[shiftylogic]

I am definitely interested in exploring this with other folks. However, it is lower on my priority list at the moment.

[ahsonkhan]

Updated issue with the TODOs added when using the new compiler.