Minor redundant code or possible bug in instruction (not (not %))?

[erp12]

I can't be exactly sure what this instruction is meant to do based on its name alone, but something seems off. Seems wrong to have (not (not ...)) anywhere.

(define-registered 
  code_null
  ^{:stack-types [:code :boolean]}
  (fn [state]
    (if (not (empty? (:code state)))
      (push-item (let [item (first (:code state))]
                   (not (not (and (seq? item) (empty? item))))) ;; <-- Seems redundant.
                 :boolean
                 (pop-item :code state))
      state)))

Link to full code: https://github.com/lspector/Clojush/blob/master/src/clojush/instructions/code.clj#L386

[lspector]

(not (not ...)) can be useful to turn a truthy value into an actual true and a falsey value into an actual false:

  (not (not 'truthy!))
=> true
   (not (not nil))
=> false

I haven't investigated whether seq? or empty? can return something that is truthy/falsey rather than actual true/false, but if it can then the and here could as well, and the double not will ensure that we're only pushing an actual true or false onto the :boolean stack.

The code_null instruction should push true onto the :boolean stack if () is on top of the :code stack, and false otherwise.

[erp12]

Gotcha. I am not sure either about seq? and empty?, but I do think that boolean is probably a more clear way to express this.

=> (boolean (and :a :b))
true
=> (boolean (and :a '()))
false

Not crucial to make any changes because behavior is correct.

[thelmuth]

I'm reopening this, since it seems at the least this could use a comment to explain the strange code for future generations, and it wouldn't be a bad idea to just change to boolean as @erp12 suggests.