Compilation error when "import libp2p/multicodec" is added to sync_manager.nim about nimbus-eth2 HOT 12 CLOSED

nim-lang/Nim#23680

from nimbus-eth2.

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.

from nimbus-eth2.

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)

from nimbus-eth2.

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.

from nimbus-eth2.

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.

from nimbus-eth2.

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; and
• multicodec's two lines (in that example) are imported, not in-line in constants.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.

from nimbus-eth2.

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 an immediate 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.

from nimbus-eth2.

Might be related to nim-lang/Nim#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)

from nimbus-eth2.

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)

from nimbus-eth2.

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.

from nimbus-eth2.

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

from nimbus-eth2.

template m(a: RootObj, p: int) = discard is enough.

from nimbus-eth2.