Skip to content

Conversation

@nitely
Copy link
Contributor

@nitely nitely commented Oct 7, 2025

Remove unittest2EvalOnceIter(var T): var T to work around nim-lang/Nim#25210 .

Alternatively the template could be defined when not in nimvm, but is it needed?

@nitely nitely requested a review from arnetheduck October 9, 2025 18:32
@nitely nitely marked this pull request as ready for review October 9, 2025 18:33
@arnetheduck
Copy link
Member

it's needed to forward the "varness" of something through evalonce which ut2 needs to work with functions that take var params - try removing it from here and you'll see:

iterator unittest2EvalOnceIter[T](x: T): auto =
  yield x
iterator unittest2EvalOnceIter[T](x: var T): var T =
  yield x

template unittest2EvalOnce(name: untyped, param: typed, blk: untyped) =
  for name in unittest2EvalOnceIter(param):
    blk

proc x(v: var int) = echo v
template testit(xxx: typed) =
  unittest2EvalOnce(ff, xxx):
    x(ff)

var v: int
testit(v)

In general, I'm pretty sure that if you try this change with nimbus-eth2 or similar which has a large test suite, it'll show a few breaking examples.

@nitely
Copy link
Contributor Author

nitely commented Oct 31, 2025

The way it's used in ut2, it would require for the operator proc to only accept var params which seems iffy, but valid I guess:

import ./unittest2

suite "test foo":
  test "test bar":
    type DisInt = distinct int

    proc `==`(a, b: var DisInt): bool =
      a.int == b.int

    proc x(v: var DisInt): var DisInt = v

    var a = 1.DisInt
    var b = a
    check x(a) == x(b)

The check generates:

unittest2EvalOnce(:c2, x(b), unittest2EvalOnce(:c1, x(a)) do:
  block:
    if not(:c1 == :c2):
      checkpoint("/home/esteban/AtomProjects/nim-unittest2/test2.nim(14, 15)" &
          ": Check failed: " &
          "x(a) == x(b)")
      when compiles(string($:c1)):
        checkpoint("x(a)" & " was " & $:c1)
      when compiles(string($:c2)):
        checkpoint("x(b)" & " was " & $:c2)
      fail())

The nimbus-eth2 tests do pass. I'll just define it when not in nimvm to be safe it's not that trivial.

@nitely nitely marked this pull request as draft October 31, 2025 22:15
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

Successfully merging this pull request may close these issues.

2 participants