carp-lang / Carp

A statically typed lisp, without a GC, for real-time applications.
Apache License 2.0
5.47k stars 173 forks source link

[Bug] Control.iterate-until double-uses a variable, and thus is broken #1437

Open AZMCode opened 1 year ago

AZMCode commented 1 year ago

The current definition of Control.iterate-until is as such:

(sig iterate-until (Fn [(Ref (Fn [b] b c) d), (Ref (Fn [b] Bool c) e), b] b))
  (defn iterate-until [f pred start]
    (let-do [result start]
      (while (not (~pred result))
        (set! result (~f result)))
      result))

This definition hands the ownership of the result variable to the pred function, then reuses it for f, meaning that the variable result is undefined by the time an attempt to transform it is made.

The compiler gladly warns us about this, but only when used:

You’re using a given-away value `result` at line 20, column 26 in '<$HOME>/.carp/core/Control.carp'.

You’ll have to copy the value using `@`. at <Irrelevant file where I used it>

As per fixing this there are two options, to my understanding:

  1. Handing a reference to pred. This avoids requiring the result be able to be copied, but at the expense of changing the function signature. I have made this alteration locally to core, and can confirm it works as expected.

    (doc iterate-until "Like `iterate`, but f is applied repeatedly until the predicate `pred` is true.")
    (sig iterate-until (Fn [(Ref (Fn [b] b c) d), (Ref (Fn [(Ref b f)] Bool c) e), b] b))
    (defn iterate-until [f pred start]
    (let-do [result start]
      (while (not (~pred &result))
        (set! result (~f result)))
      result))
  2. Copying the result internally. This avoids changing the function signature, at the expense of requiring the result be copyable, and doing so every iteration.

    (doc iterate-until "Like `iterate`, but f is applied repeatedly until the predicate `pred` is true.")
    (sig iterate-until (Fn [(Ref (Fn [b] b c) d), (Ref (Fn [b] Bool c) e), b] b))
    (defn iterate-until [f pred start]
    (let-do [result start]
      (while (not (~pred @&result))
        (set! result (~f result)))
      result))

Given the goals of the language I'd strongly suggest The first option, but that's up to you people.

AZMCode commented 1 year ago

I haven't checked as of yet, but similar bugs could be hidden in the other control functions.

scolsen commented 1 year ago

this is indeed a bug when b is a linear type -- thanks for catching this! Your suggested fix (1.) looks good to me, if you want, you can open a PR with it and the team can review it and if it's good to go we can merge.

AZMCode commented 1 year ago

Got it. I'll test it a little bit before submitting the PR