Closed diegomrsantos closed 1 month ago
Removing
proc `!=`*(a, b: MultiCodec): bool =
## Returns ``true`` if MultiCodecs ``a`` and ``b`` are not equal.
int(a) != int(b)
is a correct, useful change anyway, so it should be done regardless.
https://nim-lang.org/docs/manual.html#templates documents that
template `!=` (a, b: untyped): untyped =
# this definition exists in the system module
not (a == b)
assert(5 != 6) # the compiler rewrites that to: assert(not (5 == 6))
The
!=
,>
,>=
,in
,notin
,isnot
operators are in fact templates
and https://github.com/nim-lang/Nim/blob/version-1-6/lib/system/comparisons.nim#L128 shows this in the Nim 1.6 stdlib exactly:
template `!=`*(x, y: untyped): untyped =
## Unequals operator. This is a shorthand for `not (x == y)`.
not (x == y)
So as https://github.com/vacp2p/nim-libp2p/blob/368c9765f7c48b76d3f79b47a5cf210c84ff857c/libp2p/multicodec.nim#L288-L294 already defines both the ==
operator:
proc `==`*(a, b: MultiCodec): bool =
## Returns ``true`` if MultiCodecs ``a`` and ``b`` are equal.
int(a) == int(b)
proc `!=`*(a, b: MultiCodec): bool =
## Returns ``true`` if MultiCodecs ``a`` and ``b`` are not equal.
int(a) != int(b)
the !=
version should be removed as the syntax sugar it is. Nim's behavior isn't well-defined if one tries to define a !=
which isn't the logical negation of ==
regardless, so in general !=
should not be overloaded.
If this compilation error persists/returns, it can be investigated further.
I couldn't reproduce the error with a simpler code when overloading !=
.
Adding the code below instead of the import
doesn't reproduce the error either:
type
MultiCodec* = distinct int
proc `!=`*(a, b: MultiCodec): bool =
## Returns ``true`` if MultiCodecs ``a`` and ``b`` are not equal.
int(a) != int(b)
type Slot = distinct uint64
func `==`(x: Slot, y: Slot): bool {.borrow.}
import libp2p/multicodec
proc syncStep(index: int | int) = # ensure this is generic
let acc = default(seq[Slot])
discard acc[acc.len-1] != 0.Slot
syncStep(0)
where libp2p/multicodec
consists solely of
type MultiCodec* = distinct int
proc `!=`*(a, b: MultiCodec): bool =
## Returns ``true`` if MultiCodecs ``a`` and ``b`` are not equal.
int(a) != int(b)
so, no external imports left.
This reproduces it. Import libp2p/multicodec
and this error appears. Don't import libp2p/multicodec
and it compiles fine.
But fundamentally if it depends on libp2p's erroneous overloading of !=
it's not something the Nim upstream is likely to accept as a real bug, even if it's strange behavior.
So this is what I'm getting at, unless, without the !=
overloading which isn't even really well-defined in Nim, it's not a useful potential Nim bug report, so if it resolves the immediate issue, why not just do that.
Sure, I'll do it. But I'd argue that the error happening only when the operator is imported from another file and disappears when copy[^1]
is used is strange enough and deserves a deeper investigation as it might be a symptom of a bigger issue.
Additionally, the overload isn't even for Slot
.
Well, one can show the structure of this issue without any special Nim symbols:
import "."/multicodec
template m(x: untyped) = discard x == x
proc j(n: int | int) = m(@[0'u][0])
j(0)
and
type D = object
proc m*(a: D) = discard
i.e. it's not a manifestation of anything special about !=
anymore.
It also occurs in Nim 1.6, Nim 2.0, and Nim devel.
It does seem odd that the m
template isn't being called on non-D
types in this context. There is in general a matching priority Nim has from specific to non-specific types, and untyped
is sort of the least specific possible. I don't actually know if by design nothing can coexist with a template of some name taking only untyped
symbols, that at that point Nim doesn't even try to fit it in the overload-matching order.
It also occurs only when
j
is generic; andconstants.nim
So, yes, it's a bit strange. Will leave here for potential further investigation.
But the fundamental fix to libp2p you've already at this point made remains the same, and it's not some workaround.
Ah, indeed, https://nim-lang.org/docs/manual.html#templates-typed-vs-untyped-parameters documents that:
A template where every parameter is
untyped
is called an immediate template. For historical reasons, templates can be explicitly annotated with animmediate
pragma and then these templates do not take part in overloading resolution and the parameters' types are ignored by the compiler.
So, this is already documented, and is a specific reason not to try to overload !=
, that the existing !=
template does not participate in overload resolution by design because all its parameters are untyped
and therefore it's immediate
.
Might be related to https://github.com/nim-lang/Nim/issues/7803
This gives the same error:
type D = object
proc m*(a: D) = discard
block:
template m(x: untyped) = discard x == x
proc j(n: int | int) = m(@[0'u][0])
j(0)
I think it's different, because it also happens the same way if all m
are templates:
type D = object
template m(a: D) = discard
block:
template m(x: untyped) = discard x == x
proc j(n: int | int) = m([0][0])
j(0)
The other weird part is, if it's really because the immediate template m
isn't participating in the overloading resolution, why is the error message:
a.nim(5, 38) Error: type mismatch: got <T, T>
but expected one of:
proc `==`(x, y: bool): bool
first type mismatch at position: 1
required type for x: bool
but expression '[0][0]' is of type: T
proc `==`(x, y: char): bool
first type mismatch at position: 1
required type for x: char
but expression '[0][0]' is of type: T
proc `==`(x, y: cstring): bool
first type mismatch at position: 1
required type for x: cstring
but expression '[0][0]' is of type: T
proc `==`(x, y: float): bool
first type mismatch at position: 1
required type for x: float
but expression '[0][0]' is of type: T
proc `==`(x, y: float32): bool
first type mismatch at position: 1
required type for x: float32
but expression '[0][0]' is of type: T
it can only access that if it's trying to compile discard x == x
, i.e. has in fact attempted to resolve to using the immediate template m
.
type D = object
template m(a: D, p: int) = discard
block:
template m(x: untyped, i: int) = echo x
proc j(n: int | int) = m([0][0], 0)
j(0)
No immediate template, same issue
template m(a: RootObj, p: int) = discard
is enough.
Describe the bug
When
import libp2p/multicodec
is added tosync_manager.nim
, runningmake -j10 nimbus_beacon_node
fails with error:To Reproduce Steps to reproduce the behavior:
make -j10 nimbus_beacon_node
Additional context
copy[len(copy) - 1]
bycopy[^1]
fixes the problem.