status-im / nim-toml-serialization

Flexible TOML serialization [not] relying on run-time type information.
Apache License 2.0
36 stars 7 forks source link

fix failure with devel after stricteffects enabled, fix #56 #57

Closed pietroppeter closed 1 year ago

pietroppeter commented 1 year ago

see #56

pietroppeter commented 1 year ago

I am expecting error reported in #56 to show when CI runs. not sure how to proceed with fixing the new error

jangko commented 1 year ago

instead of using nosideeffect pragma, it will be cleaner and shorter to use func to replace the proc.

the unlisted exception indeed annoying because we are not sure where to look into for the source of exception.

pietroppeter commented 1 year ago
arnetheduck commented 1 year ago

nimble test fails

what about running nim cpp --threads:off -d:release --outdir:build -r --hints:off --skipParentCfg --styleCheck:usages --styleCheck:error tests/test_all manually?

pietroppeter commented 1 year ago

nimble test fails

what about running nim cpp --threads:off -d:release --outdir:build -r --hints:off --skipParentCfg --styleCheck:usages --styleCheck:error tests/test_all manually?

I did but it does not provides additional info

pietroppeter commented 1 year ago

ok, looking at some of the failing checks I can see that in some cases (e.g. windows-amd64-c-(Nim-version-1-6), but also linux and possibly others) an error is reported during compilation:

/home/runner/work/nim-toml-serialization/nim-toml-serialization/nim-toml-serialization/toml_serialization/private/array_reader.nim(40, 12) Error: 'readArray' is not GC-safe as it calls 'readValue'

in other cases (in particular at least some of those using devel, which include my local setup of windows-amd64), the error is not reported during compilation and running fails without a stacktrace.

Menduist commented 1 year ago

This should help: https://github.com/status-im/nim-serialization/pull/44 Managed to compile with the linked fix + {.push gcsafe.} at the beginning of lexer.nim & reader.nim (or one brave soul could add gcsafe to each readValue & parseValue)

Then the test segfaults, probably a orc bug / incompatibility.

pietroppeter commented 1 year ago

I see this has been merged (while still failing), assuming maintainers will take over from here (fine with me! ;))

jangko commented 1 year ago

until issue in nim-serialization solved, https://github.com/status-im/nim-serialization/issues/45, we cannot do much to remove the segfaults.