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

high(uint64) missing #725

Closed
Kooda opened this issue Dec 9, 2013 · 4 comments
Closed

high(uint64) missing #725

Kooda opened this issue Dec 9, 2013 · 4 comments

Comments

@Kooda
Copy link

Kooda commented Dec 9, 2013

var n: uint64 = high(uint64)

reports:

Error: invalid argument for 'high'

but this piece of code works for any other integer type, is this the expected behaviour?

@zah
Copy link
Member

zah commented Dec 9, 2013

This have been accepted as a minor drawback in order to keep the code in the compiler a bit more simple (signed int64 is used internally to represent all integer types). Araq have recently explained it here:

http://forum.nimrod-lang.org/t/313

@narimiran
Copy link
Member

This have been accepted as a minor drawback in order to keep the code in the compiler a bit more simple

I guess this won't be changed any time soon. won't fix label and close?

@ghost
Copy link

ghost commented Nov 19, 2018

Related: #6620

@narimiran
Copy link
Member

Technically, #6620 is a duplicate of this issue, but since it has more details and reactions, I'm closing this one.

Clyybber pushed a commit to Clyybber/Nim that referenced this issue Sep 16, 2023
## Summary

Simplify, refactor, and clean up the `lambdalifting` module and
associated passes. This makes the pass independent from `transf`,
which, besides making it more amendable to further changes, removes
the cyclic import dependency between `lambdalifting` and `transf`.

In addition, various aspects of closures are clarified and multiple bugs
are fixed:
- closing over locals inside for-loop bodies where the body is
duplicated
  works; the closed over locals are *not* duplicated
- using a closure closure iterator inside another closure iterator now
  works
- using an inner closure iterator inside another inner routine via the
  `for` syntax no longer causes an internal compiler error

Given the complexity of the rewrite, the above list is very likely not
exhaustive.

## Details

The summary of the rewrite is that the `transf`, lambda-lifting, and
closure iterator transformation passes now have a well-defined order in
which they're executed.

### Comparison

In the abstract, the lambda-lifting pass previously worked as follows:
```nim
proc innerOf(r) = # set of all closure routines defined directly in r

proc lambdalifting(root) =
  # ----- the 'detection' part
  var inner

  proc discover(x) =
    inner += x
    transform(x)
    for it in innerOf(x):
      discover(it)

  for x in innerOf(root):
    discover(x)

  var all = inner + root

  for x in all:
    let lifted # all locals of x closed over by some of its inner
               # routines
    env[x] = objectFrom(lifted)

    if x != root:
      setEnvParam x, env[x.owner]

  # ----- the 'lifting' part
  proc lift(x) =
    # for the body of `x`:
    # - (if `x` has lifted locals) inject the environment setup logic
    # - transform access to lifted locals
    # - transform closure routine usage into a closure construction
    #   expression

  for x in all:
    lift(x)

proc transform(x) =
  # note that the result of `transform` is cached
  if x is iterator or callConv(x) isnot closure: 
    lambdalifting(x)
  transf(x)
  closureiters(x)
```

The pass now works, and is integrated, as follows:
```nim
proc lambdalifting(root) =
  # ----- the 'detection' part
  let
    lifted = # all locals of `root` closed over by some of its inner
             # routines
    env = objectFrom(lifted)

  # ----- the 'lifting' part
  for x in innerOf(root):
    setEnvParam x, env

  lift(root)

proc transform(x) =
  # note that the result of `transform` is cached
  lambdalifting(x)
  transf(x)
  closureiters(x)
```

Note that transforming closure iterator symbol usage into a closure
construction expression no longer happens during `lift`, but is now
always the responsibility of `transf`.

The benefits of the new approach are:
- the passes now strictly happen in the order: `lambdalifting`,
  `transf`, `closureiters`
- the `lambdalifting` pass doesn't depend on `transf`, and each
  invocation only modifies symbols and AST directly owned by the
  provided root
- less recursion, and an overall more linear design

One downside is that nested inner closure routines now need to be
traversed multiple times, instead of each just once.

### Other internal changes:

- add the `names` iterators to `ast_query`
- use `typ.base` instead of `skipTypes` for accessing the environment
  parameters' underlying `object` type
- don't traverse the type slots of identdefs in `detectCapturedVars`
  and `liftCapturedVars`; they don't contain imperative code
- don't modify the bodies of inner routines when duplicating for-loop
  bodies

### Changes to the generated code:

- don't add an unnecessary `:state` field to the environment type for
  non-iterator routines
- don't use a temporary local during closure iterator environment
  setup. In addition to making the code easier to reason about for
  analysis passes (less assignments), no extra local is lifted into
  the environment when the setup code is emitted into a `.closure`
  iterator
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

4 participants