Bug 15594 – Make all of std.json @safe-friendly

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P3
Component
phobos
Product
D
Version
D2
Platform
All
OS
All
Creation time
2016-01-22T23:57:20Z
Last change time
2019-10-22T08:01:12Z
Keywords
safe
Assigned to
No Owner
Creator
Neia Neutuladh

Comments

Comment #0 by dhasenan — 2016-01-22T23:57:20Z
std.json has erroneously left several methods as @system. Ignoring everything else, those should be marked @safe. The code reads pointer fields from a union. This prevents it from marking several methods as @safe. Option 1: ========= Make it @safe by not using unions for reference types. This costs an extra 5 words per item. That's a non-trivial cost. Option 2: ========= Mark the following methods @trusted: * str() * object() * array() Then modify the rest of the code to use those accessors when applicable. (It usually goes through the union directly.) There will be a negligible cost from extra function calls, plus a few redundant checks, but on the whole it should be pretty much the same experience. The main problem is that it adds @trusted code. Option 3: ========= Switch from structs to classes. Use a class hierarchy to only store the data. This has a memory cost as well -- object overhead is at least one word, plus it adds indirection. I'm favoring option 2, since it's only got three @trusted methods.
Comment #1 by dhasenan — 2016-03-20T17:49:00Z
The .object() and .array() methods are not and cannot be @safe or @trusted because they return by reference. Observe: --- void main() { JSONValue v; v.type = JSON_TYPE.OBJECT; auto o = &v.object(); v.str = "overwrite"; (*o)["world"] = "i hope this works"; } --- This will, naturally, end in a segmentation fault. A different strategy is to create a @safe / @trusted DiscriminatedUnion template in std.typecons and use that. We'd have to expose @system methods to retrieve items by reference to make .object and .array continue to work, which is kind of ugly. Anyway, making str() @trusted and adding a couple @trusted methods, objectNoRef and arrayNoRef, mostly works. The main problem is iteration. The options are: * opApply is @system. That sucks. * opApply is @safe. That means you can only use it in @safe code. * Provide @safe and @system overloads. The compiler isn't smart enough to use the right one with foreach syntax. * Switch to ranges. I can only transparently do this with arrays *or* objects, not both, so this is also a breaking change. * opApply is @system. Use .array and .object in @safe code. Oh wait, they're @system because they return by reference. Expose .arrayNoRef and .objectNoRef instead. That's kind of crufty but works. * Expose .arrayNoRef and .objectNoRef, deprecate .array and .object, and eventually replace them with the NoRef variants.
Comment #2 by rschadek — 2019-10-22T08:01:12Z
this has been "fixed" by objectNoRef and arrayNoRef.