Short circuiting with and/or breaks in some cases

[6cdh]

description

In some cases, the semantic of let is wrong. I expect it always works like a function, but it isn't.

reproduce

(and false 
     (not (let []
            (print 1)
            true)))

compiled lua

local _1_
do
  print(1)
  _1_ = true
end
return (false and not _1_)

expect

local function _1_(...)
  print(1)
  return true
end
return (false and not _1_(...))

[andreyorst]

I think, this is actually bug in the and special (and possibly or too). We had a bunch of these before: https://todo.sr.ht/~technomancy/fennel/111

[technomancy]

Yes, definitely a bug in the operator special; you can replace let with do and it still triggers. But it won't trigger without the not.

Thanks for finding this; good catch.

[technomancy]

The bug is in this function: https://git.sr.ht/~technomancy/fennel/tree/1.1.0/item/src/fennel/specials.fnl#L773

It has a list of "dangerous" specials, but honestly as you've demonstrated here even a "pure" special like not can contain "dangerous" side-effects. So I think the criteria should probably just be any list.

[technomancy]

I have a fix for this, but it breaks some of the tests: https://git.sr.ht/~technomancy/fennel/commit/4f192a50bb94011ffea28c1c1e114e687d0c9186

(and (values)) => was true, becomes nil
(and true true (values)) => was true, becomes nil
(and true (values true false)) => was false, becomes true

Essentially what happens with this fix is that any values call is replaced with its first value, which may be nil in the case of (values).

I think this is more consistent/predictable because the current behavior breaks the rule that wrapping something in an IIFE should have no effect on the result. Any value of x y z should be treated the same as ((fn [] x y z)) as a general rule, and that's not true currently with and/or because of the special-casing of values.

It would certainly be possible to extend the special-casing detection, but I'm not sure whether it's a good idea to do so.

[technomancy]

It would certainly be possible to extend the special-casing detection

It's easy to detect a direct call to values inside an and/or:

  (if (and (or (= :or name) (= :and name)) (utils.list? ast) (< 1 i)
           (not= :values (tostring (. ast 1))))
      (utils.list (utils.sym :do) ast)
      ast)

However, direct calls to values here are more or less pointless; no one writes code like this. I think the only reason to support values here is when the macroexpansion results in values.

However, this produces a chicken-or-egg problem; we can't tell if the macroexpansion gives multiple values without compiling the arguments to and/or, but we need to know whether an IIFE is necessary before the macroexpansion happens.

This makes me think that "proper" handling of values that we used to have in the previous release is actually fundamentally incompatible with short-circuiting.

[andreyorst]

just a thought — maybe we can splice values inte current form unless they're in return position? AFAIU values is a special case to return multiple values from a function, or to assign them to a set of variables. This is easy to detect statically, as the compiler already does it. And probably all other uses of values can be safely replaced with no values call:

(and true false (values true false) false)
;; becomes
(and true false true false false)

Then there's no difference in compiling, and no need for detection, because it'll happen early enough?

[6cdh]

I know nothing about compiler, but I guess its hard to get values work right on logical operators at compile time.
For example, this code

(fn f [...]
  (values ...))

(and true (f true false)) ; => ?

How should it works? Or even more runtime

(fn f [...]
  (values ...))

(fn input []
  (let [s (io.read "*n")]
    (if (= s 1) 
        (f true)
        (f true false))))

(print (and true (input)))

[6cdh]

It's a bigger bug than I thought. Consider this example:

(fn f [...]
  (values ...))

(fn input []
  (let [s (io.read "*n")]
    (if (= s 1)
      (f "a")
      (f "a" "b"))))

(print (.. "=" (input)))

[andreyorst]

The last two examples are not a bug. This is documented behavior: https://fennel-lang.org/reference#operators

[6cdh]

Thanks. I didn't know it, my mistake.

[technomancy]

just a thought — maybe we can splice values into current form unless they're in return position?

That's basically what happens today; we compile the final argument to and/or, and each of the values it gives us gets spliced in as if it were directly in the arglist. The problem is that the compilation process can put side-effects before the actual operator:

(and false (values :a (let [] (error :noo)) :c))

local _1_
do
  _1_ = error("noo")
end
return (false and "a" and _1_ and "c")

I think the problem is that right now the macroexpansion and the compilation happen at the same time. What we really need to do is macroexpand the arguments, look for multiple values in the final position, splice them into a single arglist, and then add IIFE to anything we can't prove is safe.

[XeroOl]

This is fixed in the upcoming 1.5.0 release