uniconproject / unicon

http://www.unicon.org
Other
54 stars 28 forks source link

JSON package fails to reconstruct a new record #451

Open IanTrudel opened 2 weeks ago

IanTrudel commented 2 weeks ago

The JSON package fails to reconstruct a new record when the said record has never been used in the code. I suspect the compiler remove unused records and therefore cannot reconstruct the record. The current JSON implementation uses proc for both record and class. A potential solution would be to use constructor() instead.

A working example:

link ximage

import json

record a(b, c)

procedure main()
   ra := a(1, 2)
   ja := utoj(ra)
   jra := jtou(ja)
   jra.b := 3

   write(ja)
   write(ximage(jra))

   raa := jtou("{\"__unirecord__\":\"a\",\"b\":4,\"c\":5}")
   raa.c := 9
   write(ximage(raa))
end
{"__unirecord__":"a","b":1,"c":2}
R_a_3 := a()
   R_a_3.b := 3
   R_a_3.c := 2
R_a_5 := a()
   R_a_5.b := 4
   R_a_5.c := 9

Fails to create a new record n:

link ximage

import json

record a(b, c)
record n(m, o)

procedure main()
   ra := a(1, 2)
   ja := utoj(ra)
   jra := jtou(ja)
   jra.b := 3

   write(ja)
   write(ximage(jra))

   rn := jtou("{\"__unirecord__\":\"n\",\"m\":8,\"o\":7}")
   #rn.c := 9 # Would throw an error
   write(ximage(rn))
end
{"__unirecord__":"a","b":1,"c":2}
R_a_3 := a()
   R_a_3.b := 3
   R_a_3.c := 2
T4 := table(&null)
   T4["__unirecord__"] := "n"
   T4["m"] := 8
   T4["o"] := 7

It will not fail when at least one record has been created, anywhere in the code:

link ximage

import json

record a(b, c)
record n(m, o)

procedure main()
   ra := a(1, 2)
   ja := utoj(ra)
   jra := jtou(ja)
   jra.b := 3

   write(ja)
   write(ximage(jra))

   rn := jtou("{\"__unirecord__\":\"n\",\"m\":8,\"o\":7}")
   rn.o := 9
   write(ximage(rn))

   # Using the record anywhere works
   tn := n(0, 0)
end
{"__unirecord__":"a","b":1,"c":2}
R_a_3 := a()
   R_a_3.b := 3
   R_a_3.c := 2
R_n_2 := n()
   R_n_2.m := 8
   R_n_2.o := 9

A potential solution:

link ximage

import json

record a(b, c)
record n(m, o)

procedure main()
   ra := a(1, 2)
   ja := utoj(ra)
   jra := jtou(ja)
   jra.b := 3

   write(ja)
   write(ximage(jra))

   rn := constructor("n", "m", "o")(8, 7)
   rn.o := 9
   write(ximage(rn))
end
{"__unirecord__":"a","b":1,"c":2}
R_a_3 := a()
   R_a_3.b := 3
   R_a_3.c := 2
R_n_1 := n()
   R_n_1.m := 8
   R_n_1.o := 9
StephenWampler commented 2 weeks ago

Interesting problem - good catch!

The use of constructor() is really just hiding the creation of the record instance, however. So, invoking utoj() on the result still won't recreate the record unless a record instance is explicitly created somewhere in the code as in your previous example. Or am i missing something? Try your last example with the following lines added to the end of main():

   jn := utoj(rn)
   write("jn: ",jn)
   jrn := jtou(jn)
   write("jrn: ",ximage(jrn))
   jrn.m := 15

``

IanTrudel commented 2 weeks ago

Interesting problem - good catch!

The use of constructor() is really just hiding the creation of the record instance, however. So, invoking utoj() on the result still won't recreate the record unless a record instance is explicitly created somewhere in the code as in your previous example. Or am i missing something?

Exactly. The default behaviour should perhaps be the creation of a record instance, when it's not found, due to the nature of records — they are just a structure with fields.

Try your last example with the following lines added to the end of main():

   jn := utoj(rn)
   write("jn: ",jn)
   jrn := jtou(jn)
   write("jrn: ",ximage(jrn))
   jrn.m := 15

Mind blown. To be fair, the JSON package is unlikely to consider dynamically created records in its current implementation.

StephenWampler commented 2 weeks ago

To me, this is an example of a 'premature' optimization. Information about a record shouldn't be discarded just because there's no explicit creation of an instance of that record because of dynamic invocation (i.e. proc()). If it hadn't been discarded, JSON would [probably] have worked just fine because proc() would have worked.

The fact that proc() works or doesn't work because of something that is referenced later in the code is a flag that this optimization shouldn't have been done. Note that the statement: if 1 = 0 then n(12.13) is sufficient for proc() to work even though the record creation will never occur and could, conceivably, be optimized out.

Agree about dynamically created records - except that the JSON strings could be read in from a source created by a different program [which introduces other issues, mind you, such as both programs must have the same record declarations!].

Jafaral commented 2 weeks ago

FYI, those two json library functions were assume you want to encode/decode Unicon data structures. I started using the library recently and realized I didn't care about encoding the structures, instead I care about encoding the data. I added a new function tojson(). So So instead of doing utoj() -> jtou(), I do `tojson() -> jtou()

Try:

   jn := tojson(rn)
   write("jn: ",jn)
   jrn := jtou(jn)
   write("jrn: ",ximage(jrn))
StephenWampler commented 2 weeks ago

Of course, tojson() produces something simple that jtou() converts into a table [as I'm sure you know, Jafari], so the two operations are not inverses. Substituting tojson() for utoj() in the 'bad' example:

-> t1c
{"b":1,"c":2}
T3 := table(&null)
   T3["b"] := 1
   T3["c"] := 2

Run-time error 107
File t1c.icn; Line 14
record expected
offending value: table_3(2)
Traceback:
   main()
   {table_3(2) . b} from line 14 in t1c.icn

Interestingly, in the original 'bad' example from Ian, replacing the record declarations with class declarations shows the same 'missing' constructor if no instance of a class is referenced in the code. That's not really a surprise, since classes are records internally. What caught me surprise is the tojson() -> jtou() produces different problems depending on whether record or class declarations are used. For the class declaration version:

-> t1c
{"b":1"c":2}
[json] Unable to open file "{\"b\":1\"c\":2}"
&null

Run-time error 107
File t1c.icn; Line 14
record expected
offending value: &null
Traceback:
   main()
   {&null . b} from line 14 in t1c.icn
Jafaral commented 2 weeks ago

Of course, tojson() produces something simple that jtou() converts into a table [as I'm sure you know, Jafari], so the two operations are not inverses.

Yup! the intention is to encode data, not structures as I mentioned. In most practical use cases the other side isn't even a unicon program. A JSON object is dictionary (value field pairs), if you want to store that in a record in your application, then it is up to the application developer.

I also understand the utility of the other semantics to encode data structures if used between two unicon applications and only between two unicon applications. That json encoding semantics is wrong outside that use case.