bug: -if-let mismatch between documentation and behavior

[ethan-leba]

The documentation for -if-let reads as follows: If VAL evaluates to non-nil, bind it to VAR and do THEN, otherwise do ELSE.

However, -if-let's behavior does not match this, and instead appears to match
against (part of) var:

(-if-let ((foo . bar) '(3)) 'true 'false) ;; evals to 'false
(if '(3) 'true 'false) ;; evals to 'true

;; if-let macroexpands to:
(let ((--dash-source-98-- '(3)))
  (if --dash-source-98--
      (let ((foo (pop --dash-source-98--)))
        (if foo ;; should be --dash-source-98--
            (let ((bar --dash-source-98--))
              (if bar 'true 'false))
          'false))
    'false))

Thanks for the great package!

[Fuco1]

The VAL here is '(3) is non-nil, which passes. The list matching was done on purpose like this, where you can match prefixes or extra keys. That happened before if-let was created though so the logic is a bit weird maybe. Consider these examples

(-let (((a b c d) nil))                         ;; only this one would fail -if-let
  (message "%s" (list a b c d)))

(-let (((a b c d) (list 1)))
  (message "%s" (list a b c d)))

(-let (((a b c d) (list 1 2 3 4 5)))
  (message "%s" (list a b c d)))

[ethan-leba]

The VAL here is '(3) is non-nil, which passes.

By passes do you mean (-if-let ((foo . bar) '(3)) 'true 'false) evaluates to 'true? It evaluates to 'false in my testing -- sorry if the code comments in my original post were unclear!

(-let (((a b c d) nil))                         ;; only this one would fail -if-let
  (message "%s" (list a b c d)))

(-let (((a b c d) (list 1)))
  (message "%s" (list a b c d)))

(-let (((a b c d) (list 1 2 3 4 5)))
  (message "%s" (list a b c d)))

This fails -if-let:

(-if-let ((a b c d) (list 1))
    (message "%s" (list a b c d)))

So -if-let appears to be performing it's conditional against some part of the destructured variables. This shouldn't be the case, correct?

[Fuco1]

Oh yea, so I got myself confused.

What you say is correct. The -if-let requires all the destructured variables to be non-nil, not the "source". What I said was not wrong but unrelated :D

We should fix the documentation.

There are cases where for example (list nil nil) is actually truthy but destructuring it to (a b) would be false with -if-let. So this needs to be taken into consideration by people using it.

[ethan-leba]

Gotcha, so that's the expected behavior then. Maybe something like: If all VARs bound by evaluating VAL are non-nil, do THEN, otherwise do ELSE.? -when-let seems to have the same behavior as well.

[yuhan0]

This was actually confusing coming from Clojure, where if-let does indeed work the way the current docstring implies.

Here's the equivalent example (note: gensyms cleaned up for readability)

(if-let [[foo bar] '(3)] :true :false) ;; => :true

;; macroexpands to:
(let [temp# '(3)]
  (if temp#
    (let* [vec# temp#
           foo  (nth vec# 0 nil)
           bar  (nth vec# 1 nil)]
      :true)
    :false))

Clojure also has a related set of macros if-some and when-some for checking that the value is strictly non-nil (since it has both false and nil to be falsey values).
Given that we shouldn't make any breaking changes, maybe we could repurpose those names here in Dash for macros that actually behave this way? And add a warning to the docstring of -if-let, pointing out the difference.

(defmacro -if-some (var-val then &rest else)
  "If VAL evaluates to non-nil, bind it to VAR and do THEN,
otherwise do ELSE.

Note: binding is done according to `-let'.

\(fn (VAR VAL) THEN &rest ELSE)"
  (declare (debug ((sexp form) form body))
           (indent 2))
  (let* ((sym (make-symbol "input0"))
         (bindings (dash--match (car var-val) sym)))
    `(let ((,sym ,(cadr var-val)))
       (if ,sym
           (let* ,bindings
             ,then)
         ,@else))))

(-if-some ((a b) '(3))
    (list a b)
  'false)
;; => (3 nil)

;; =macroexpand=>
(let ((input0 '(3)))
  (if input0
      (let* ((--dash-source-97-- input0))
             (a (pop --dash-source-97--))
             (b (car --dash-source-97--)))
        (list a b))
    'false))