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

default should be public instead of redefined everywhere; + proposal for a better implementation #103

Closed
timotheecour opened this issue Jul 31, 2018 · 3 comments

Comments

@timotheecour
Copy link
Member

Currently template default[T](t: typedesc[T]): T = var v: T; v is defined in multiple places:

lib/core/seqs.nim:91:10:template default[T](t: typedesc[T]): T =
lib/pure/collections/queues.nim:157:10:template default[T](t: typedesc[T]): T =
lib/pure/collections/sets.nim:44:10:template default[T](t: typedesc[T]): T =
lib/pure/collections/sets.nim:284:10:template default[T](t: typedesc[T]): T =
lib/pure/collections/tableimpl.nim:119:10:template default[T](t: typedesc[T]): T =
tests/destructor/tcustomseqs.nim:101:10:template default[T](t: typedesc[T]): T =
tests/vm/tvmmisc.nim:16:8:  proc default[T](t: typedesc[T]): T {.inline.} = discard

instead of being defined once and exported via * (it's also useful in library and user code).

how about defining it (once) in system.nim, but using the following better definition which allows usage at compile time:

# NOTE: `defaultImpl` needed because of https://github.com/nim-lang/Nim/issues/8486
proc defaultImpl[T]():T=
  var a: T
  return a
template default*[T](t: typedesc[T]): T = defaultImpl[T]()
when defined(case_default_old_temp):
  template default*[T](t: typedesc[T]): T =
    # Error: cannot evaluate at compile time: v116407 var v: T
    var v: T
    v
when defined(case_default_workaround):
  proc defaultImpl[T]():T=
    var a: T
    return a
  # Works!
  template default*[T](t: typedesc[T]): T = defaultImpl[T]()
else:
  proc default*[T]():T=
    var a: T
    return a

proc len(T: typedesc[tuple]):auto=
  when defined(case_default_old) or defined(case_default_workaround):
    const t=T.default
  else:
    const t=default[T]()

  var n=0
  for _ in t.fields:
    inc n
  return n

proc test()=
  var a=(1,2,3)
  const length = a.type.len
  echo length

test()

EDIT /cc @LemonBoy see my edited proposal above.

NOTE: if nim-lang/Nim#8486 is fixed in the future, we can use the friendlier syntax default(T) / T.default

EDIT: added template default*[T](t: typedesc[T]): T = defaultImpl[T]()

@timotheecour timotheecour changed the title default should be public instead of redefined everywhere default should be public instead of redefined everywhere; should use default[T] Jul 31, 2018
@timotheecour timotheecour changed the title default should be public instead of redefined everywhere; should use default[T] default should be public instead of redefined everywhere; should use default[T] instead of default(T) Jul 31, 2018
@timotheecour timotheecour changed the title default should be public instead of redefined everywhere; should use default[T] instead of default(T) default should be public instead of redefined everywhere; should use default[T]() instead of default(T) Jul 31, 2018
@Araq
Copy link
Member

Araq commented Jul 31, 2018

I agree, we can do that once the compiler bugs are fixed.

@timotheecour timotheecour changed the title default should be public instead of redefined everywhere; should use default[T]() instead of default(T) default should be public instead of redefined everywhere; + proposal for a better implementation Jul 31, 2018
@timotheecour
Copy link
Member Author

timotheecour commented Jul 31, 2018

@Araq I found a way to use the friendlier T.default using intermediate defaultImpl, see above.
PR accepted for that? (with that friendlier version, no need to wait until nim-lang/Nim#8486 is fixed since user code using default won't be affected by a change of implementation of default)

Which module, system.nim?

timotheecour referenced this issue in timotheecour/Nim Jul 31, 2018
timotheecour referenced this issue in timotheecour/Nim Aug 1, 2018
timotheecour referenced this issue in timotheecour/Nim Aug 1, 2018
timotheecour referenced this issue in timotheecour/Nim Aug 1, 2018
@narimiran narimiran transferred this issue from nim-lang/Nim Jan 13, 2019
@Araq
Copy link
Member

Araq commented Mar 10, 2019

system.default has been added. Closing.

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

2 participants