-
Notifications
You must be signed in to change notification settings - Fork 259
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Revert "stop testing with broken upstream/version-2-0" #6573
Conversation
still nope:
at least it's different than the first two |
That one is arguably a not-great template in Nimbus, fixed by b7f3f0c but that allowed finding nim-lang/Nim#24150 in its place, which shows up as e.g., https://github.com/status-im/nimbus-eth2/actions/runs/10972715518/job/30469294056?pr=6573
|
|
It's related to type U = distinct uint64
template toSszType(v: U): auto = uint64(v)
echo $hash_tree_root(default(List[U, 2])) which on respectively 2.0.8,
|
This is causing https://github.com/status-im/nim-ssz-serialization/blob/cc09635ff06a337087ffeb83f51b8ee1e70a105c/ssz_serialization/types.nim#L67-L78 to return 1, since |
Requires status-im/nim-ssz-serialization#95 |
I'm not sure if this should be necessary though. nim-lang/Nim#22022 is the Nim commit/PR which triggers this particular CI failure. A simplified form is that: proc k(_: int | int): int {.compileTime.} =
doAssert false
0
doAssert compiles(k 0) compiles before that PR and does not compile after it. proc k(_: int | int): int {.compileTime.} =
doAssert false
0
doAssert compiles(typeof k(0)) compiles both before and after it. For non-generic proc k(_: int): int {.compileTime.} =
doAssert false
0
doAssert compiles(typeof k(0)) does not compile before, but it does compile after, even with the proc k(_: int): int {.compileTime.} =
doAssert false
0
doAssert compiles(k(0)) With neither So while status-im/nim-ssz-serialization#95 does work around this in general, I'm not sure what the design intention is, or if its documented what should happen. template supports*(_: type SSZ, T: type): bool =
mixin toSszType
anonConst compiles(fixedPortionSize toSszType(declval T)) offers another example, which it's also not clear to me even whether it should work. |
Is this caused by the
|
b7f3f0c
to
ecd1e0b
Compare
To begin with, there should be no difference between the generic and non-generic cases - in both Now, as to which behavior to pick .. this is harder - if However, this is also very hard to teach - when reading code, |
Well, it doesn't |
https://nim-lang.org/docs/manual.html#pragmas-compiletime-pragma claims:
It seems inconsistent even on this point though. Just checking proc r(): int {.compileTime.} = discard
discard r() compiles, but proc r() {.compileTime.} = discard
r() does not. It seems like either they should both compile or neither compile. |
The I think |
Reverts #6554
If upstream Nim has been fixed, can test
version-2-0
again