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

template generics don't bind generic params on instantiation; causing wrong runtime values or crashes #13527

Closed
timotheecour opened this issue Feb 28, 2020 · 2 comments · Fixed by #22535

Comments

@timotheecour
Copy link
Member

timotheecour commented Feb 28, 2020

template generics don't bind generic params on instantiation; causing wrong runtime values or crashes

Example

when true:
  template getTypStr[T](a: T): string = $T
  echo(getTypStr(1))   # 170141183460469231731687303715884105727; should be: int
  # echo(getTypStr(true))   # true ; should be: bool
  # echo(getTypStr(1.0)) # Error: unhandled exception: cannot extract number from invalid AST node 
  # echo(getTypStr(1u8))   # Error: internal error: expr(skType); unknown symbol

Current Output

170141183460469231731687303715884105727

Expected Output

int

other example: default

  template isDefault[T](a: T): bool = a == default(T) # BUG
  doAssert isDefault(0.0)

gives: Error: type mismatch: got <int> but expected one of: proc default(T: typedesc): T:type

workarounds are not good

use type(a) instead of T works in example above but won't work in more general cases eg:

  type Foo[M, P] = object
  template getTypStr[T1, T2](a: Foo[T1, T2]): untyped =
    # $T1 # doesn't work because of this issue
    $type(a).M # bad: you have to "know" that `T1` corresponds to `M`
  let a = Foo[int8, bool]()
  let b=getTypStr(a)
  echo (b, $type(b))

but then (despite being bad), that doesn't even work with special cases like seq[T], giving Error: undeclared field: 'T'

  template getTypStr[X](a: seq[X]): untyped =
    $type(a).T # doesnt' work for seq[T] even though it's declared like seq[T] !
  var a: seq[int]
  let b=getTypStr(a)
  echo (b, $type(b))

So you're forced to use the newly introduced typetraits.genericParams (see #13433), which seems overkill for these specific cases.

note

interestingly, static[T] generic params do seem to bind correctly, unlike non-static generic params eg:

  type Foo[N: static float, T: Ordinal] = object
  template getTypStr[N, T](a: Foo[N, T]): auto = N
  let a = Foo[3.1, bool]()
  doAssert getTypStr(a) == 3.1

root cause for:

Additional Information

@krux02
Copy link
Contributor

krux02 commented Mar 11, 2020

Well I have to say the output is pretty bad. But still, the example does not produce the output of the Current Output section.

@timotheecour
Copy link
Member Author

timotheecour commented Mar 11, 2020

I fixed the top snippet by removing the # i had

# echo(getTypStr(1))   # 170141183460469231731687303715884105727; should be: int
=>
echo(getTypStr(1))   # 170141183460469231731687303715884105727; should be: int

@Araq Araq added the Severe label Mar 23, 2021
metagn added a commit to metagn/Nim that referenced this issue Aug 23, 2023
metagn added a commit to metagn/Nim that referenced this issue Aug 25, 2023
Araq pushed a commit that referenced this issue Aug 25, 2023
* fix generic param substitution in templates

fixes #13527, fixes #17240, fixes #6340, fixes #20033, fixes #19576, fixes #19076

* fix bare except in test, test updated packages in CI
narimiran pushed a commit that referenced this issue Dec 1, 2023
* fix generic param substitution in templates

fixes #13527, fixes #17240, fixes #6340, fixes #20033, fixes #19576, fixes #19076

* fix bare except in test, test updated packages in CI

(cherry picked from commit 1cc4d3f)
narimiran pushed a commit that referenced this issue Dec 1, 2023
* fix generic param substitution in templates

fixes #13527, fixes #17240, fixes #6340, fixes #20033, fixes #19576, fixes #19076

* fix bare except in test, test updated packages in CI

(cherry picked from commit 1cc4d3f)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants