From cdba47becf622ff342f1f64ec708d5f99dafb60c Mon Sep 17 00:00:00 2001 From: Eric <5089238+emizzle@users.noreply.github.com> Date: Tue, 21 May 2024 12:39:47 +1000 Subject: [PATCH] fix: force symbol resolution for types that serde de/serializes (#24) * fix: force symbol resolution for types that serde de/serializes Force symbols into scope when mixins are used with generic overloads. When mixins are used with generic overloads, the overloaded symbols in scope of the mixin are evaluated from the perspective of the mixin. This creates issues in downstream modules that may inadvertantly dispatch *only* to the symbols in the scope of the mixin, even when the module with the wrong symbol overloads is not imported. By forcing the compiler to use symbols for types handled by serde, we can be sure that these symbols are available to downstream modules. We can also be sure that these `fromJson` symbols can be overloaded where needed. * remove enum forced scoping Forcing a scoping for a particular enum type would only resolve that type and not all enum types. * Add mixin + generic overloads as known issue to README * try to fix URL reference to deserializer.nim --- README.md | 22 ++++++++++++++++++++++ serde/json/deserializer.nim | 27 +++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/README.md b/README.md index 128a946..1b0d338 100644 --- a/README.md +++ b/README.md @@ -388,3 +388,25 @@ proc parseMe(me: string): JsonNode {.raises: [MyAppError].} = assert """{"hello":"world"}""".parseMe == %* { "hello": "world" } ``` + +## Known issues + +There is a known issue when using mixins with generic overloaded procs like +`fromJson`. At the time of mixin call, only the `fromJson` overloads in scope of +the called mixin are available to be dispatched at runtime. There could be other +`fromJson` overloads declared in other modules, but are not in scope at the time +the mixin was called. Therefore, anytime `fromJson` is called targeting a +declared overload, it may or may not be dispatchable. This can be worked around +by forcing the `fromJson` overload into scope at compile time. For example, in +your application where the `fromJson` overload is defined, at the bottom of the +module add: +```nim +static: MyType.fromJson("") +``` +This will ensure that the `MyType.fromJson` overload is dispatchable. + +The basic types that serde supports should already have their overloads forced +in scope in [the `deserializer` +module](./serde/json/deserializer.nim#L340-L356). + +For an illustration of the problem, please see this [narrow example](https://github.com/gmega/serialization-bug/tree/main/narrow) by [@gmega](https://github.com/gmega). diff --git a/serde/json/deserializer.nim b/serde/json/deserializer.nim index f63dcaa..8f00d5a 100644 --- a/serde/json/deserializer.nim +++ b/serde/json/deserializer.nim @@ -327,3 +327,30 @@ proc fromJson*[T: ref object or object](_: type ?T, json: string): ?!Option[T] = else: let jsn = ?JsonNode.parse(json) # full qualification required in-module only Option[T].fromJson(jsn) + +# Force symbols into scope when mixins are used with generic overloads. When +# mixins are used with generic overloads, the overloaded symbols in scope of the +# mixin are evaluated from the perspective of the mixin. This creates issues in +# downstream modules that may inadvertantly dispatch *only* to the symbols in +# the scope of the mixin, even when the module with the wrong symbol overloads +# is not imported. By forcing the compiler to use symbols for types handled by +# serde, we can be sure that these symbols are available to downstream modules. +# We can also be sure that these `fromJson` symbols can be overloaded where +# needed. +static: + discard bool.fromJson("") + discard Option[bool].fromJson("") + discard seq[bool].fromJson("") + discard uint.fromJson("") + discard Option[uint].fromJson("") + discard seq[uint].fromJson("") + discard int.fromJson("") + discard Option[int].fromJson("") + discard seq[int].fromJson("") + discard UInt256.fromJson("") + discard Option[UInt256].fromJson("") + discard seq[UInt256].fromJson("") + discard JsonNode.fromJson("") + type DistinctType = distinct int + discard DistinctType.fromJson(newJString("")) + discard UInt256.fromJson(newSeq[byte](0))