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

Raise an unlisted exception #130

Closed
StefanSalewski opened this issue Mar 17, 2023 · 13 comments
Closed

Raise an unlisted exception #130

StefanSalewski opened this issue Mar 17, 2023 · 13 comments

Comments

@StefanSalewski
Copy link

Dear Sir,

sorry for bothering you again. Recently I saw, that you updated a few files of your great Nim YAML package. So my guess is, that the package is still maintained? If not, please just ignore this issue.

References:

#99
#97

Some years ago, I considered mentioning your great library in the book, and using it also in my SDT tool. But due to the above old issues, I only used std/json for the book and for the SDT tool.

Today, I had again some tiny motivation to do some Nim work again, and so tried your lib again. First I tried the plain first JSON example from https://ssalewski.de/nimprogramming.html#_serializationstoring_data_permanently_on_external_storage which was working fine. But testing your lib in my SDT tool, lead instantly to error messages like:

home/salewski/.nimble/pkgs2/yaml-1.1.0-69800cc68996ea75203b1d3aa706fb37909dada1/yaml/serialization.nim(1328, 19) Error: representChild(value[], childTagStyle, c) can raise an unlisted exception: Exception

Unfortunately, I have not enough knowledge and motivation to investigate that issue in more detail. So my question: Is this an issue with the latest devel compiler? May a "nimble install yaml@#head" help? I did a plain "nimble install yaml" for now. May it be an issue of default ORC in latest devil compiler?

Of course I can try to extract the failing code from SDT to reproduce the issue, but as I said above, I would understand well if you gave up Nim, so we may just ignore the issue. I liked the {.transient.} and {.defaultVal.} of your lib very much, as that was missing from current std/json. So with std/json whenever I added a field to an ref object, it was impossible to import old data files. That is really not fun when developing. But I know that other JSON implementations may exists.

Best regards,

Dr. Stefan Salewski

@StefanSalewski
Copy link
Author

OK, here is a fast copy and paste from SDT:

import yaml/serialization
import std/streams

type
  LineCap* {.size: sizeof(cint), pure.} = enum
    butt = 0
    round = 1
    square = 2

type
  Styles {.pure.} = enum
    medium, silk, thin, thick, fat, pin, sym, pad, hole, net, none

type
  LineCaps {.pure.} = enum
    butt, round, square

type
  V2 = array[2, float]

type
  Element = ref object of RootRef
    layer: int
    style: Styles
    p: seq[V2]
    at: seq[Text] # attached text
    hover: bool
    selected: bool
    gx, gy: int # text grab
    isNew: bool

# type
  Text = ref object of Element
    name, val: string # attribute
    nameVis, valVis: bool
    text: string
    parent: Element # new, for easy reattaching, and maybe a graphical parent indication (arrow)
    detached: bool # maybe with new parent field this boolean is redundant. 
    sizeInPixel: bool

type
  PathLike = ref object of Element

type
  Line = ref object of PathLike # Element

type
  Trace = ref object of PathLike # Element

type
  Net = ref object of PathLike # Element

type
  Rect = ref object of Element

type
  Circ = ref object of Element

type
  Pad = ref object of Element
    cornerRadius: float

type
  GerberPad = ref object of Element
    width: float
    cap: LineCap

type
  GerberSilk = ref object of Element
    width: float
    cap: LineCap

type
  Path = ref object of PathLike # Element

type
  Pin = ref object of PathLike # Element
    sym: string # Symbolname, extracted from at of group

type
  Hole = ref object of Element
    drill: float
    dia: float
    xext, yext: bool

type
  Group = ref object of Element
    silks: seq[GerberSilk]
    lines: seq[Line]
    circs: seq[Circ]
    texts: seq[Text]
    rects: seq[Rect]
    pads: seq[Pad]
    gerberPads: seq[GerberPad]
    holes: seq[Hole]
    paths: seq[Path]
    pins: seq[Pin]
    traces: seq[Trace]
    nets: seq[Net]
    groups: seq[Group]
    origin: V2 # for inserting, origin is aligned with grid

var x, x2: Group
x = Group()

var s = newFileStream("file.yaml", fmWrite)
#var str: string = dump(d1)
#echo str
dump(x, s)
s.close()

var s2 = newFileStream("file.yaml", fmRead)
load(x2, d2)
s2.close()

echo x2
/home/salewski/.nimble/pkgs2/yaml-1.1.0-69800cc68996ea75203b1d3aa706fb37909dada1/yaml/serialization.nim(79, 15) template/generic instantiation from here
/home/salewski/.nimble/pkgs2/yaml-1.1.0-69800cc68996ea75203b1d3aa706fb37909dada1/yaml/serialization.nim(1328, 19) Error: representChild(value[], childTagStyle, c) can raise an unlisted exception: Exception
nim -v
Nim Compiler Version 1.9.1 [Linux: amd64]
Compiled at 2023-03-17
Copyright (c) 2006-2023 by Andreas Rumpf

git hash: fd4e3ae3e4564525f901f2517711a1243535d2a2
active boot switches: -d:release

@StefanSalewski
Copy link
Author

And this makes no difference:

nimble install yaml@#head

nim c --gc:refc bug.nim

@flyx
Copy link
Owner

flyx commented Mar 17, 2023

In your code there's an undefined variable d2. Did you mean load(s2, x2) at line 113?

I got this error with that change:

/Users/flyx/Projects/NimYAML/test1.nim:113:5 template/generic instantiation of `load` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:1386:14 template/generic instantiation of `construct` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:1362:19 template/generic instantiation of `constructChild` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:1266:22 template/generic instantiation of `constructChild` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:1179:20 template/generic instantiation of `constructObject` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:927:25 template/generic instantiation of `constructObjectDefault` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:907:28 template/generic instantiation of `constructFieldValue` from here
/Users/flyx/Projects/NimYAML/test1.nim:89:5 template/generic instantiation of `constructChild` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:1200:18 template/generic instantiation of `constructObject` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:440:19 template/generic instantiation of `constructChild` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:1266:22 template/generic instantiation of `constructChild` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:1179:20 template/generic instantiation of `constructObject` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:927:25 template/generic instantiation of `constructObjectDefault` from here
/Users/flyx/Projects/NimYAML/yaml/serialization.nim:860:28 Error: cannot infer the type of the array

which does seem like a bug.

Then again, I'm still on

Nim Compiler Version 1.6.8

I will investigate a bit and report back.

@StefanSalewski
Copy link
Author

This is my latest version:

import yaml/serialization
import std/streams

type
  Element = ref object of RootRef
    layer: int
    style: int#Styles
    p: seq[int]
    at: seq[Text] # attached text
    hover: bool
    selected: bool
    gx, gy: int # text grab
    isNew: bool

# type
  Text = ref object of Element
    name, val: string # attribute
    nameVis, valVis: bool
    text: string
    parent: Element # new, for easy reattaching, and maybe a graphical parent indication (arrow)
    detached: bool # maybe with new parent field this boolean is redundant. 
    sizeInPixel: bool

type
  PathLike = ref object of Element

type
  Line = ref object of PathLike # Element

type
  Trace = ref object of PathLike # Element

type
  Net = ref object of PathLike # Element

type
  Rect = ref object of Element

type
  Circ = ref object of Element

type
  Pad = ref object of Element
    cornerRadius: float

type
  GerberPad = ref object of Element
    width: float
    cap: int#LineCap

type
  GerberSilk = ref object of Element
    width: float
    cap: int#LineCap

type
  Path = ref object of PathLike # Element

type
  Pin = ref object of PathLike # Element
    sym: string # Symbolname, extracted from at of group

type
  Hole = ref object of Element
    drill: float
    dia: float
    xext, yext: bool

type
  Group = ref object of Element
    silks: seq[GerberSilk]
    #lines: seq[Line]
    #circs: seq[Circ]
    texts: seq[Text]
    #rects: seq[Rect]
    #pads: seq[Pad]
    #gerberPads: seq[GerberPad]
    #holes: seq[Hole]
    #paths: seq[Path]
    #pins: seq[Pin]
    #traces: seq[Trace]
    #nets: seq[Net]
    groups: seq[Group]
    origin: int#V2 # for inserting, origin is aligned with grid

var x, x2: Group
x = Group()

var s = newFileStream("file.yaml", fmWrite)
#var str: string = dump(d1)
#echo str
dump(x, s)
s.close()

var s2 = newFileStream("file.yaml", fmRead)
load(s2, x2)
s2.close()

#echo x2.repr

Giving

/home/salewski/.nimble/pkgs2/yaml-1.1.0-7cde4658e6fd00b498580c25b5b3a90f513cd16c/yaml/serialization.nim(1328, 19) Error: representChild(value[], childTagStyle, c) can raise an unlisted exception: Exception

First version contained a typo, but compiler stopped earlier due to this issue. I had to comment most fields of Group to get it to compile. As I had no idea to debug, I already asked Jason B., he replied

Likely they use Exception instead of CatchableError like here

except Exception:
.
Which means Nim adds Exception to the raises list. Searching "raise", "except", and "Exception" are going to give the most fruitful response.
Finding where incorrectly annotated exceptions are should not be too difficult with that.

But I think I am not smart enough to understand what is going on.

@StefanSalewski
Copy link
Author

My feeling is, that the issue is related to all the messages

Warning: The bare except clause is deprecated; use `except CatchableError:` instead [BareExcept]

I think that was changed in Nim v1.4. But I can not remember all the details.

@flyx
Copy link
Owner

flyx commented Mar 18, 2023

I did push a commit that consolidates exception code to avoid Exception and except:. Not sure it fixes your error though since I cannot reproduce it with a stable Nim version.

I didn't find anything about the Nim version 1.9.1 you seem to be using on the nightly builds so I'm unsure how to obtain it. Generally while I do maintain this library to ensure it works with released Nim versions, I don't keep up with current development. Comments from maintainers and people more active are always welcome if the development version changed something that breaks NimYAML.

@StefanSalewski
Copy link
Author

Thanks. You must have been really bright in 2016 when you wrote this great library, I hope you are still that bright! I have to admit that I do understand absolutely nothing about it.

I will test your latest commit soon, I guess I have to do "nimble install yaml@#head" for that?

Unfortunately, if people should really use your lib, it is highly recommended to make it work with latest devel. Note that we had already Nim 2.0 release candidate end of 2022. I have to do the same for my packages, e.g. for gintro Arch Linux people always used latest GTK, so I actually had to install an Arch Linux system to fix a recent bug. I know that this is no fun for one or too users only.

Maybe an alternative for your yaml would be status-im serialisation? I just started reading their docs, it is not really easy. They seem to support optional fields, which is in practice very important. And I have seen a JSON implementation of threeform, but that seems to have no optional fields. Mr. R,. had a packaged JSON, but I would reject to use that. And maybe Disruptek had one, but I guess he fully retired. Actually I would prefer YALM, as its human readebility is nice, but YAML is known to be very complicated. I think I should advertice YAML only in the book, when it is sure that it will work still in a few years, and that may be really hard to guarantee. For my SDT tool, I may use your yaml when its works, but I would create a set of when statements to support JSON as a fallback.

For devel compiler, most of us do just:

#Nim install
git clone https://github.com/nim-lang/Nim.git
cd Nim
./build_all.sh
#Add to .bashrc when not already done:
PATH=$PATH:~/Nim/bin
PATH=$PATH:~/.nimble/bin

Later, for an update this is sufficient:

#Nim update:
cd Nim
git pull
./koch boot -d:release
./koch tools

Best regards,

Stefan Salewski

@StefanSalewski
Copy link
Author

No, your latest commit seems not to help. I tried similar except fixes myself with your code, and totally failed. Currently I get errors like

/home/salewski/.nimble/pkgs2/yaml-1.1.0-402e28900438ec8b905af628d37e2adf4965de94/yaml/serialization.nim(1326, 19) Error: representChild(value[], childTagStyle, c) can raise an unlisted exception: Exception
home/salewski/.nimble/pkgs2/yaml-1.1.0-402e28900438ec8b905af628d37e2adf4965de94/yaml/serialization.nim(860, 28) Error: cannot infer the type of the array

You may also care for warnings like

/home/salewski/.nimble/pkgs2/yaml-1.1.0-402e28900438ec8b905af628d37e2adf4965de94/yaml/serialization.nim(989, 9) Warning: template 'serializeImpl' is implicitly redefined; this is deprecated, add an explicit .redefine pragma [ImplicitTemplateRedefinition]

But well, when there are no other potential users beside me, then you may just ignore these issues for now. I can use other JSON libs.

flyx added a commit that referenced this issue Mar 18, 2023
 * can now be loaded properly
 * ref #130
@StefanSalewski
Copy link
Author

There is some progress, with your latest commit the "cannot infer the type of the array" issue seems to be gone. But still "/home/salewski/.nimble/pkgs2/yaml-1.1.0-5d4adf2617b4340d1b83566028d2cfbdb4c3ac6b/yaml/serialization.nim(1344, 19) Error: representChild(value[], childTagStyle, c) can raise an unlisted exception: Exception"

For install Nim 2.0 RC see also https://nim-lang.org/blog/2022/12/21/version-20-rc.html.

Have to leave the computer for two hours now...

@flyx
Copy link
Owner

flyx commented Mar 18, 2023

I pushed a fix for the cannot infer the type of the array error.

You use inheritance in your types, this is currently not supported in NimYAML, i.e. loading a Rect currently cannot load fields of Element etc. I do not plan to implement this feature myself but welcome contributions. Even if I fixed the other error, you still won't be able to load your structure from YAML using NimYAML, as long as this feature is not implemented. If you depend on inherited fields working, which I assume you do, I suggest looking elsewhere.

status-im serialisation is welcome to implement YAML support on top of NimYAML's low-level API. Another library I know of is yanyl which afaik does support inheritance so you're welcome to try that.

I think I should advertice YAML only in the book, when it is sure that it will work still in a few years

I maintain this project in my spare time and I can't and won't give any guarantee that I'll still do it in two weeks. I do try my best to keep it working using the time I am willing to spend on it. This does not include continuously testing latest Nim devel.

I will tackle the exception problem eventually if it breaks supported features, but it's not high priority if it only occurs with code that wouldn't work anyway.

@flyx
Copy link
Owner

flyx commented Mar 18, 2023

One more note: YAML is a decent format when your use-case is to have a human-readable serialization format. It is, however, quite horrible for humans to write due to many strange edge cases in the language, and missing support for text processing in the language.

I know this from answering YAML questions on StackOverflow. Lots of large YAML users have bolted some templating engine onto YAML to deal with its lacking expressiveness (see for example: Ansible with Jinja, Kubernetes Helm with go templates, GitHub actions with their own, homebrew, lacking engine, …). So depending on what exactly you're doing, I would possibly be hesitant to promote YAML.

There are newer approaches for human-writable configuration languages like Dhall which do a better job with providing the user with tools to apply concepts like DRY on data files. Dhall specifically doesn't currently support Nim directly but can output JSON which you can then load with a JSON implementation.

You probably have a better understanding of whether this would be relevant to your work than I have.

@StefanSalewski
Copy link
Author

You use inheritance in your types, this is currently not supported in NimYAML, i.e. loading a Rect currently cannot load fields of Element etc.

Actually I am using only static data types, no runtime dispatch. So it works with std/json, and I had the hope that it would work with your yaml as well. The problem of dynamic dispatch is discussed in my book at the second half of https://ssalewski.de/nimprogramming.html#_serializationstoring_data_permanently_on_external_storage. My solution is that I copy heterougenous container content to homogeneous container before JSON storing. I think I could avoid inheritance completely, so you yaml may work then. But actually I have to understand exceptions and the effect system better. My feeling is, that exceptions and effects are not the bests parts of Nim, they are difficult in practice. I should really expand the sections in my book. I wonder if Rumpf may explain them well in his new book, but I will not buy it, I donated 500$ for the Nim project 18 months ago, that should be enough.

@flyx
Copy link
Owner

flyx commented Mar 18, 2023

My feeling is, that exceptions and effects are not the bests parts of Nim, they are difficult in practice.

I agree. They are also buggy; there's at least one place in NimYAML where the compiler tells me that an exception can never be raised, but when I remove it from the raises list it throws an error saying this exception can be raised.

Actually I am using only static data types, no runtime dispatch

The Readme states that polymorphic objects are not supported, which is to say that you can't load a Rect into a field that has the declared type Element. But it also means that inherited fields are not supported. These are two separate problems though.

The problem with inherited fields is that NimYAML currently walks through the AST of the type for which it loads fields, and therefore only sees the fields directly declared on this type. This can and should be done differently, but since I haven't used Nim for a long time apart from managing this library, this is not an easy fix I can do, less so since the AST documentation is not the best part of the Nim docs. There is fieldPairs which could be the solution but it'd mean to rewrite larger parts of the serialization code.

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