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

RT vs CT #91

Closed
timotheecour opened this issue Nov 14, 2020 · 3 comments
Closed

RT vs CT #91

timotheecour opened this issue Nov 14, 2020 · 3 comments

Comments

@timotheecour
Copy link
Contributor

timotheecour commented Nov 14, 2020

example 1: works at RT(?), fails at CT

wrote works at RT with a ? because I'm not sure whether the output (A, ) is correct or not.

when defined case4n:
  import yaml
  type
    Node = ref object
      name1: string
      name2: string
  proc main =
    let input = """
name1: A,
name2: B
"""
    let root = loadAs[Node](input)
    echo root.name1
  static: main()
  main()

RT:
prints:

A,

CT (uncomment static: main()):
(with nim-lang/Nim#15947)
errors:

stack trace: (most recent call last)
/Users/timothee/git_clone/nim/timn/tests/nim/all/t11261.nim(294, 15) t11261
/Users/timothee/git_clone/nim/timn/tests/nim/all/t11261.nim(292, 28) main
/Users/timothee/git_clone/nim/NimYAML/yaml/serialization.nim(1382, 7) loadAs
/Users/timothee/git_clone/nim/NimYAML/yaml/serialization.nim(1365, 14) load
/Users/timothee/git_clone/nim/NimYAML/yaml/serialization.nim(1344, 5) construct
/Users/timothee/git_clone/nim/timn/tests/nim/all/t11261.nim(6, 1) template/generic instantiation from here
/Users/timothee/git_clone/nim/NimYAML/yaml/serialization.nim(1344, 5) Error: unhandled exception: Expected field name, got yamlStartMap [YamlConstructionError]
      raise (ref YamlConstructionError)(getCurrentException())

example 2: works at RT and CT

adding {} makes it work

when defined case4o:
  #[
  ]#
  import yaml
  type
    Node = ref object
      name1: string
      name2: string
  proc main =
    let input = """
{
  name1: A,
  name2: B
}
"""
    let root = loadAs[Node](input)
    echo root.name1
  static: main()
  main()

example 3: works at RT and CT

when defined case5e:
  #[
  works
  ]#
  import yaml
  type
    Node = ref object
      name1: string
      lhs: Node
      rhs: Node
  proc main =
    let input = """
{
  name1: A,
  lhs:{
    name1: B,
    lhs: ~,
    rhs: ~
  },
  rhs:{
    name1: B,
    lhs: ~,
    rhs: ~
  }
}
"""
    let root = loadAs[Node](input)
    echo root.name1
    echo root.lhs == nil
  static: main()
  main()
@flyx
Copy link
Owner

flyx commented Nov 14, 2020

The cause of this is the keyCache which seems to be broken at compile time.

The quick rundown is: When name1 is encountered, a scalar event is created. However, before it is emitted, the parser checks whether a : follows – if so, name1 starts a map. The created scalar event is pushed into the keyCache and a mapping start event is created and emitted. When querying the next event, the parser retrieves the scalar event from the keyCache.

However, at compile time, when the mapping start is generated, this seems to also update the pushed event in the key cache (which it shouldn't!). This is a VM bug. This is the parser code where it occurs.

This explains the difference in the output you showed in your comment on the other issue: Instead of the first key of the map, you get another +MAP.

Is this enough info to fix it in the VM? I can try writing up a minimal example to file a Nim bug, but I am afraid the time slot I allocated for rewriting the NimYAML parser has expired and I don't have much time pushing this forward so things will take more time.

@timotheecour
Copy link
Contributor Author

@flyx after a painful reduction I've found the root causes:

flyx added a commit that referenced this issue Aug 30, 2023
 * dumping doesn't work at CT
 * timestamps don't work at CT
 * aliases don't work at CT
 * renamed tserialization -> tnative to mirror rename of native.nim file
 * added comptime tests to tnative, currently only execute when doing
   nim nativeTests, because of compiler bug
 * Fixes #70, #91
@flyx
Copy link
Owner

flyx commented Aug 30, 2023

Compile time loading now works well enough. The linked Nim issue should be re-evaluated as I don't see the problem occurring with NimYAML anymore.

Dumping doesn't work but I think that's not a priority. Some other things don't work at CT:

  • timestamps (needs C API)
  • aliases (due to the casting issue)

Apart from that, all loading tests for loading into native types succeed at comptime, which is good enough for me to close this and the other issue.

@flyx flyx closed this as completed Aug 30, 2023
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