Skip to content
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

Generic template parameter order determines whether compilation succeeds #17253

Open
tersec opened this issue Mar 4, 2021 · 1 comment
Open

Comments

@tersec
Copy link
Contributor

tersec commented Mar 4, 2021

A template can't bind to generic types in a specific combination involving array and int64 when parameter are orders in one way, but not when they're reversed.

Example

type
  Foo*[maxLen: static int64; T] = object
    data*: array[maxLen, T]

  Baz*[maxLen: static int64; T] = object
    data*: array[maxLen, T]

# test.nim(12, 42) Error: cannot instantiate Foo
# got: <static[type T](T), type U>  (Nim 1.4)
# got: <typedesc[T], typedesc[U]>   (Nim current head)
# but expected: <maxLen, T>
template bar[T, U](x: array[T, U], y: Foo[T, U]) = discard # comment this line and it compiles

template bar[T, U](x: array[T, U], y: array[T, U]) = discard
template bar[T, U](x: Foo[T, U], y: array[T, U]) = discard
template bar[T, U](x: Foo[T, U], y: Foo[T, U]) = discard

# These all compile fine; it appears to relate to taking array before Foo
template bar[T, U](x: Baz[T, U], y: Foo[T, U]) = discard
template bar[T, U](x: Baz[T, U], y: Baz[T, U]) = discard
template bar[T, U](x: Foo[T, U], y: Baz[T, U]) = discard
template bar[T, U](x: Foo[T, U], y: Foo[T, U]) = discard

Current Output

$ Nim/bin/nim --version
Nim Compiler Version 1.5.1 [Linux: amd64]
Compiled at 2021-03-04
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: c180de60a8ac5459d1e553152132fcfc6cd2fe6f
active boot switches: -d:release
$ Nim/bin/nim c test.nim
Hint: used config file 'Nim/config/nim.cfg' [Conf]
Hint: used config file 'Nim/config/config.nims' [Conf]
....
test.nim(12, 42) Error: cannot instantiate Foo
got: <typedesc[T], typedesc[U]>
but expected: <maxLen, T>

Expected Output

A successful compilation, or at least some error message explaining why the apparently inconsistent situation makes sense.

Additional Information

I've checked that this also occurs in

$ nim --version
Nim Compiler Version 1.4.2 [Linux: amd64]
Compiled at 2020-12-02
Copyright (c) 2006-2020 by Andreas Rumpf

active boot switches: -d:release
$ nim c test.nim
Hint: used config file '/etc/nim/nim.cfg' [Conf]
Hint: used config file '/etc/nim/config.nims' [Conf]
....
test.nim(12, 42) Error: cannot instantiate Foo
got: <type T, type U>
but expected: <maxLen, T>

and

$ ./env.sh nim --version
Nim Compiler Version 1.2.6 [Linux: amd64]
Compiled at 2021-02-27
Copyright (c) 2006-2020 by Andreas Rumpf

git hash: bf320ed172f74f60fd274338e82bdc9ce3520dd9
active boot switches: -d:release
$ ./env.sh nim c ../test.nim 
test.nim(12, 42) Error: cannot instantiate Foo
got: <static[type T](T), type U>
but expected: <maxLen, T>
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Mar 4, 2021
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Mar 4, 2021
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Mar 4, 2021
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Mar 5, 2021
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Mar 6, 2021
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Mar 11, 2021
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Mar 12, 2021
tersec added a commit to status-im/nimbus-eth2 that referenced this issue Mar 15, 2021
* initial immutable validator database factoring

* remove changes from chain_dag: this abstraction properly belongs in beacon_chain_db

* add merging mutable/immutable validator portions; individually test database roundtripping of immutable validators and states-sans-immutable-validators

* update test summaries

* use stew/assign2 instead of Nim assignment

* add reading/writing of immutable validators in chaindag

* remove unused import

* replace chunked k/v store of immutable validators with per-row SQL table storage

* use List instead of HashList

* un-stub some ncli_db code so that it uses

* switch HashArray to array; move BeaconStateNoImmutableValidators from datatypes to beacon_chain_db

* begin only-mutable-part state storage

* uncomment some assigns

* work around nim-lang/Nim#17253

* fix most of the issues/oversights; local sim runs again

* fix test suite by adding missing beaconstate field to copy function

* have ncli bench also store immutable validators

* extract some immutable-validator-specific code from the beacon chain db module

* add more rigorous database state roundtripping, with changing validator sets

* adjust ncli_db to use new schema

* simplify putState/getState by moving all immutable validator accounting into beacon state DB

* remove redundant test case and move code to immutable-beacon-chain module

* more efficient, but still brute-force, mutable+immutable validator merging

* reuse BeaconState in getState

* ensure HashList/HashArray caches are cleared when reusing getState buffers; add ncli_db and a unit test to verify this

* HashList.clear() -> HashList.clearCache()

* only copy incrementally necessary immutable validators

* increase strictness of test cases and fix/work around resulting HashList cache invalidation issues

* remove explanatory scaffolding

* allow for storage of full (with all validators) states for backwards/forwards-compatibility

* adjust DbSeq type usage

* store full, with-validators, state every 64 epochs to enable reverting versions

* reduce memory allocation and intermediate objects in state storage codepath

* eliminate allocation/copying through intermediate BeaconStateNoImmutableValidators objects

* skip benchmarking initial genesis-validator-heavy state store

* always store new-style state and sometimes old-style state

* document intent behind BeaconState/Validator type-punnery

* more accurate failure message on SQLite in-memory database initialization failure
@metagn
Copy link
Collaborator

metagn commented Sep 5, 2024

Still errors but since #24005 you can change Foo[T, U] to Foo[x.len, U] for it to work. The T inferred in array[T, U] is a range type, not a static type, so it can't be applied to Foo, but array types still accept static ints for their first parameter. So another workaround would be a wrapper type for Foo that turns the first parameter from a range to a static int which should also work in 2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants