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

every symbol becomes 1st class; defines 0-cost lambda and aliases; generics/iterators/templates/etc can be passed to any routine #11992

Closed
wants to merge 23 commits into from

Conversation

timotheecour
Copy link
Member

@timotheecour timotheecour commented Aug 21, 2019

This PR makes every symbol 1st class, which enables these:

  • passing any symbol in scope (generic/proc/macro/template/iterator/module/etc) to any routine (proc/iterator/macro/etc). This works even if the symbol is overloaded, or if it's a template/macro with all optional params
  • aliases (const myecho = alias echo) that work with any symbol; can also return a tuple, eg (fun1: alias echo, fun2: alias doAssert)
  • 0-cost lambdas (eg: x~>x mod 2 == 0, (x,y) ~> x*y, lambdaIt: it*2 etc), which are defined in library code on top of alias
  • composable iterators, which allows define lazy functional programming primitives (eager toSeq and lazy map/filter/join etc; which can have speed benefit compared to sequtils); the resulting code is quite nice and compact, see tlambda_iterators.nim which uses library lambdaIter that builds on top of lambda; it allows defining primitives that work regardless we're passing a value (@[1,2,3]) or a lazy iterate (eg iota(3))

Extensive unittests are included, see tlambda.nim, tlambda_iterators.nim

fixed issues

speed benefit

  • can be used to improve speed and ease of use of procs that have callback (functor) parameters: the callback can now be a zero-cost lambda instead of a closure (which incurs overhead, eg due to lack of inlining once we have a closure);

for example the following code gives a 3X to 4X speedup (with both debug build and -d:danger) over algorithms.isSorted which uses a callback closure via
func isSorted*[T](a: openArray[T], cmp: proc(x, y: T): int {.closure.}, order = SortOrder.Ascending): bool =

func isSorted2*[T, Fun](a: openArray[T], cmp: Fun): bool =
  result = true
  for i in 0..<len(a)-1:
    if not cmp(a[i],a[i+1]):
      return false

# benchmark code:
  let n = 1_000
  let m = 100000
  var s = newSeq[int](n)
  for i in 0..<n: s[i] = i*2
  benchmarkDisp("isSorted2", m): doAssert isSorted2(s, (a,b)~>a<b)
  benchmarkDisp("isSorted", m): doAssert isSorted(s, cmp)

other benefits

notes

  • [parser] a => counter+=1 parsed as (a => counter) += 1, violating the spec, and causing issues with => #8759 is a minor bug that affects both sugar.=> and this PR's ~> in some cases; should be easy to fix

  • alias2 and aliassym are placeholder names; I'll change these once we agree on final names

  • I'd prefer the syntax: alias x = foo.bar instead of const x = alias foo.bar; this requires a minor parser change; it would simplify usage in general. The code in this PR wouldn't change much with this change, but user code would become simpler. If there's agreement, I'll switch to alias x = foo.bar and proc bar(fun: alias)

  • compared to sugar.=>, the library code alias defined in this PR on top of alias are easier to use (doesn't suffer from type inference with lambdas doesn't work (requires explicit types) #7435 as shown in unittests), and more efficient (no function call overheard that would prevent inlining)

  • will hide behind a feature flag (eg: {.experimental: aliassym.}) once we agree on name

@timotheecour timotheecour changed the title every symbol becomes first class; 0-cost lambda; aliases; passing iterators/generics/etc to any routine every symbol becomes 1st class; 0-cost lambda; aliases; passing iterators/generics/etc to any routine Aug 21, 2019
@timotheecour timotheecour changed the title every symbol becomes 1st class; 0-cost lambda; aliases; passing iterators/generics/etc to any routine every symbol becomes 1st class; 0-cost lambda; aliases; iterators/generics/etc can be passed to any routine Aug 21, 2019
@timotheecour timotheecour marked this pull request as ready for review August 21, 2019 09:06
@timotheecour timotheecour changed the title every symbol becomes 1st class; 0-cost lambda; aliases; iterators/generics/etc can be passed to any routine every symbol becomes 1st class; defines 0-cost lambda; defines aliases; iterators/generics/etc can be passed to any routine Aug 21, 2019
@timotheecour timotheecour changed the title every symbol becomes 1st class; defines 0-cost lambda; defines aliases; iterators/generics/etc can be passed to any routine every symbol becomes 1st class; defines 0-cost lambda and aliases; iterators/generics/etc can be passed to any routine Aug 21, 2019
@timotheecour timotheecour changed the title every symbol becomes 1st class; defines 0-cost lambda and aliases; iterators/generics/etc can be passed to any routine every symbol becomes 1st class; defines 0-cost lambda and aliases; inline iterators/templates/etc can be passed to any routine Aug 21, 2019
@timotheecour
Copy link
Member Author

just rebased (it was previously green but had started to bitrot) /cc @Araq

@timotheecour timotheecour force-pushed the pr_aliassym branch 5 times, most recently from effe211 to 21c9ad3 Compare September 8, 2019 20:35
@timotheecour
Copy link
Member Author

timotheecour commented Sep 8, 2019

rebased and workaround after new gensym handling
friendly ping /cc @Araq

@timotheecour
Copy link
Member Author

rebased again against latest devel /cc @Araq ; this PR (which garnered a lot of +1's) would fill an important gap in nim IMO;
I've been benefitting from it in my own code but it's unpleasant to have to keep rebasing a patched nim against latest devel :)

@kristianmandrup
Copy link

Looks fantastic!! Please merge this ASAP, perhaps release it for 1.1.0?

@Araq
Copy link
Member

Araq commented Mar 24, 2020

This really needs a RFC process. Negative sides:

  • I need to constantly think about this feature when adding new code to the compiler, comparable to skipTypes which is burdensome.
  • It probably impacts the compiler's speed.

@timotheecour
Copy link
Member Author

timotheecour commented Nov 3, 2020

For now it doesn't even subsume typedesc[T]. How to infer the T?

here:

when true:
  import std/lambdas
  template typedesc2[T](a): bool = a is type and a is T

  # this currently works:
  proc bar1(T: aliassym): auto = T.default
  doAssert bar1(alias2 float) == 0.0
  proc bar2(T: aliassym): auto {.enableif: typedesc2[SomeInteger](T).} = T.default
  doAssert bar2(alias2 uint8) == 0'u8

  when false:
    # this hasn't yet been implemented but will work:
    proc bar3(T: aliassym): seq[T] = discard
    # pending future enableif syntax sugar:
    proc bar2(T: typedesc2[SomeInteger]): seq[T] = discard

I see concepts as our ticket to get typechecked generics and I don't see how "enableif" can get us there.

{.enableif.} is much simpler and more powerful, it can express constraints that can't be expressed with concepts (isIterable, constraints involving more than 1 arguments, constraints involving untyped, etc) without all the bugs.

just look at how much easier it is to write a correct+efficient toSeq with this PR.
As the author of #8711, I can tell you how refreshing it is to be able to use proc instead of template and not have to worry about evalOnceAs (or all the gymnastics in toSeq, toSeq1, toSeq2).

Also note that sequtils.toSeq is still incorrect for openArray (multiple evaluation bug, and it's really hard if not impossible to fix once you understand the problem), whereas with this PR, it just works:

import std/lambdas
template isIterable(a): bool = typeof(block: (for ai in a: ai)) is type
proc toSeqImpl(a: auto): seq {.enableif: isIterable(a).} =
  type T = typeof(block: (for ai in a: ai))
  when compiles(a.len):
    result = newSeq[T](a.len)
    var i=0
    for ai in a: (result[i] = ai; i.inc)
  else:
    result = newSeq[T]()
    for ai in a: result.add ai

template toSeq(a: untyped): untyped = toSeqImpl(lambdaIter a) # sugar to avoid calling code having to write `lambdaIter`

doAssert toSeq(10..13) == @[10,11,12,13]
iterator iota(n: int): int = (for i in 0..<n: yield i)
doAssert toSeq(iota(3)) == @[0,1,2]
doAssert @[10,11].toSeq == @[10,11]
doAssert [10,11].toSeq == @[10,11]
doAssert not compiles toSeq 10

# works with openArray, safe wrt multiple evaluation bug, unlike sequtils.toSeq!
from sequtils import nil
proc fn3(a: openArray[int]) =
  template fn(x: int): untyped = (echo "in fn"; x)
  echo "this PR toSeq: no bug"
  doAssert toSeq(toOpenArray(a,0,fn(1))) == @[10,11]
  echo "sequtils.toSeq: multiple evaluation bug and hard/impossible to fix"
  doAssert sequtils.toSeq(toOpenArray(a,0,fn(1))) == @[10,11]
fn3 @[10,11]

prints:

this PR toSeq: no bug
in fn
sequtils.toSeq: multiple evaluation bug and hard/impossible to fix
in fn
in fn
in fn
in fn
in fn
in fn

@Araq
Copy link
Member

Araq commented Nov 3, 2020

As long as your return type remains auto you don't have a solution that is satisfying. Also, how can you "enableif" constraints for the return type? Remember that the documentation generator cares about proc headers, not about proc bodies.

@SixteNim
Copy link

SixteNim commented Dec 1, 2020

aliassym simulates a #define name (....) well known from C, but locally scoped instead of C's rather undefined (global) scopes. If the (...) stands for a name itself, alpha-conversion will happen - the most basic (trivial) substitution. Therefore, a proc that annotates a a : aliassym b can be read as a substitution a for b , where the proc body uses an "a" that stands for a "b" and the "b" itself stands for a witness, for both a prototypic implementation and a pattern for other substitutions as well. Logically, aliassym is sound - as long as a witness is presented. But what about substitutions? Substitutions should be made possible with an attribute, e.g. {. witness 'name' .} where "name" stands for the name of the witness. The attribute is sufficient enough to get a sound aliassym , noenableif etc. needed. As long as scope borders are respected, the implementer of a particular module will have full control about the module's intended behaviour. This is important for safety reasons. If scope borders are not respected, the compiler should give a warning (at least).
aliassym is crucial at every place where structural substitution is needed (iterators) ore desired (lambdas to be inlined). It would improve Nim's expressiveness, safety and speed at the same time. Why don't other languages (except D) have this feature? Well, they are not flexible enough at the AST level.

This was referenced Mar 8, 2021
@timotheecour timotheecour mentioned this pull request Apr 8, 2021
8 tasks
@timotheecour timotheecour changed the title every symbol becomes 1st class; defines 0-cost lambda and aliases; inline iterators/templates/etc can be passed to any routine every symbol becomes 1st class; defines 0-cost lambda and aliases; generics/iterators/templates/etc can be passed to any routine Sep 16, 2021
@mwerezak
Copy link

Notwithstanding the technical discussion in the PR, having first class symbols and composable iterators would be a huge boost to expressiveness of the language.

@stale
Copy link

stale bot commented Feb 5, 2023

This pull request has been automatically marked as stale because it has not had recent activity. If you think it is still a valid PR, please rebase it on the latest devel; otherwise it will be closed. Thank you for your contributions.

@stale stale bot added the stale Staled PR/issues; remove the label after fixing them label Feb 5, 2023
@stale stale bot removed the stale Staled PR/issues; remove the label after fixing them label Feb 27, 2023
@metagn metagn mentioned this pull request Apr 13, 2023
Copy link
Contributor

github-actions bot commented Mar 4, 2024

This pull request is stale because it has been open for 1 year with no activity. Contribute more commits on the pull request and rebase it on the latest devel, or it will be closed in 30 days. Thank you for your contributions.

@github-actions github-actions bot added the stale Staled PR/issues; remove the label after fixing them label Mar 4, 2024
Copy link
Contributor

github-actions bot commented Apr 5, 2024

This pull request has been marked as stale and closed due to inactivity after 395 days.

@github-actions github-actions bot closed this Apr 5, 2024
@mwerezak
Copy link

mwerezak commented Apr 6, 2024

rip

@ringabout
Copy link
Member

Although this PR is incredible. It has been a struggle to maintain the existing features of the Nim language. What really is needed for now is to refactor existing features and make it accessible among backends. Anyway the stale label means eveyone could pick it up and complete it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Staled PR/issues; remove the label after fixing them
Projects
None yet
10 participants