Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infinite recursion in normal representation of type (Z1K1)? #4

Closed
lucaswerkmeister opened this issue May 1, 2020 · 5 comments
Closed

Comments

@lucaswerkmeister
Copy link

I’m trying to parse the normal JSON serialization, as described in the specification, and realized I don’t know what the type of an object is supposed to look like in that serialization.

In the JSON files shipped with eneyj, which are in canonical representation, the value of the Z1K1 key (the type) is a string literal like "Z2", which IIUC means that it’s a reference – if it was a string value, it would have to be represented as { "Z1K1": "Z6", "Z6K1": "Z2" }. And in the normal representation, Z1K1 is not a special key (only Z1K2 and Z6K1 are), so the reference (Z9) must be an object. Presumably, the reference ID (Z9K1) of that reference must be an object representing a string value… but what does the type of the reference look like?

{
  "Z1K1": {
    "Z1K1": {
      "Z1K1": {
        // ... infinite representation of a reference to Z9?
      },
      "Z9K1": {
        "Z1K1": /* reference to Z6 */,
        "Z6K1": "Z9"
      }
    },
    "Z9K1": {
      "Z1K1": /* reference to Z6 */,
      "Z6K1": "Z2" // actual type name goes here
    },
  // other keys of Z2 or other type go here
}

I suspect the specification needs to be amended to break this cycle; probably make Z1K1 another key that is serialized as a string literal.

I might be misunderstanding something, though. I tried to see if eneyj has a facility to show the normal serialization of a value, but couldn’t find it.

@vrandezo
Copy link
Contributor

vrandezo commented May 2, 2020

expand in src/utils/json.js should create the normal representation, and canonicalize the same place the canonical one. There is a script to do that, src/scripts/expander.js but that doesn't work for me right now.

(this is all IIRC, I haven't looked into the code myself for a while!)

I wrote the specification as a wishlist, it doesn't fit with the current kernel implementation. (That's obviously a bug)

The stable keys are currently 'Z1K1', 'Z1K2', 'Z6K1', 'Z11K2', 'Z16K1', 'Z16K2',
'Z18K1', 'Z60K1', 'Z70K1', 'Z180K2', none of these gets processed as described by the normalization / canonicalization. This list should become shorter - I think only Z1K1, Z1K2, and Z6K1 really need to be there.

But yes, as you say, Z1K1 indeed has to be there - that's an error in the Spec for exactly the reason you state and needs to be fixed.

@vrandezo
Copy link
Contributor

vrandezo commented May 2, 2020

Just FYI and as a comment, the reason why I didn't put Z1K1 there is because I haven't decided yet on how to deal with parametrised types. Should it just be a Z7 that creates a type on the fly from a given argument? Or something else? But this leads to the question what the identity of type is (and that's the reason for Z4K1 which is kinda weird), and down the whole rabbithole of type checking etc.

But really, that needs a proper solution, and until then I Z1K1 should probably just be treated as a stable key.

@vrandezo
Copy link
Contributor

vrandezo commented May 2, 2020

I fixed the expander script: fba8a4b

That at least should work now.

vrandezo added a commit that referenced this issue May 3, 2020
In particular, added Z1K1 to the stable keys  in 2.8, based on Issue #4
@vrandezo
Copy link
Contributor

vrandezo commented May 3, 2020

Fixed the specification to take that into account.

@vrandezo vrandezo closed this as completed May 3, 2020
@lucaswerkmeister
Copy link
Author

Thanks, and also thanks for explaining the x and * in the specification :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants