core.async

(CLJS) or evaluation not stopped when exp nests take

Details

  • Type: Defect Defect
  • Status: Reopened Reopened
  • Priority: Major Major
  • Resolution: Unresolved
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None
  • Environment:
    CLJS

Description

(go (or true
        (= (<! (do (js/console.log "this should not happen") (chan)))
           :doesnt-matter)))

Activity

Hide
Alex Miller added a comment -

That <! is missing a channel so this seems to be broken code to start with - marking not reproducible till example fixed.

Show
Alex Miller added a comment - That <! is missing a channel so this seems to be broken code to start with - marking not reproducible till example fixed.
Hide
Leon Grapenthin added a comment -

Hi Alex, the invalid arg to <! was intended.

The point is that it should never be evaluated according to defined behavior of `or` and thus is correct.

You can also substitute line 2 with
(= (<! (do (js/console.log "this should not happen") (chan)))
or invoke a function there that logs and returns a channel.

IMO that should not make a difference to label this a bug.

The bug still exists in 0.2.395

Show
Leon Grapenthin added a comment - Hi Alex, the invalid arg to <! was intended. The point is that it should never be evaluated according to defined behavior of `or` and thus is correct. You can also substitute line 2 with (= (<! (do (js/console.log "this should not happen") (chan))) or invoke a function there that logs and returns a channel. IMO that should not make a difference to label this a bug. The bug still exists in 0.2.395
Hide
Kevin Downey added a comment - - edited

For some weird reason (maybe as an optimization of some sort?), the or macro in cljs some times expands as calls to if (like clojure's does) and some times expands to `(js* "({}) || ({})" a b)`. the ioc machinery has no special handling for `js*` so it does its normal ANF like transform:

(js* "(~{}) || (~{})" A B)

to

(let [x A
      y B]
  (js* "(~{}) || (~{})" x y))

so of course you lose short circuiting.

General support in the ioc machinery for js* would require parsing the rewriting the javascript, which seems unlikely to happen. Specific cases of js* could be handled by matching the exact string of javascript, but that may be kind of brittle.

The best thing to do is likely to change the clojurescript definition of or to always emit ifs (like clojure's) and move an optimizations in to the compiler.

Show
Kevin Downey added a comment - - edited For some weird reason (maybe as an optimization of some sort?), the or macro in cljs some times expands as calls to if (like clojure's does) and some times expands to `(js* "({}) || ({})" a b)`. the ioc machinery has no special handling for `js*` so it does its normal ANF like transform:
(js* "(~{}) || (~{})" A B)
to
(let [x A
      y B]
  (js* "(~{}) || (~{})" x y))
so of course you lose short circuiting. General support in the ioc machinery for js* would require parsing the rewriting the javascript, which seems unlikely to happen. Specific cases of js* could be handled by matching the exact string of javascript, but that may be kind of brittle. The best thing to do is likely to change the clojurescript definition of or to always emit ifs (like clojure's) and move an optimizations in to the compiler.
Hide
Kevin Downey added a comment -

also `or` and `and` in clojurescript both run analysis as part of the macro expansion, which can lead to macros be expanded multiple times

Show
Kevin Downey added a comment - also `or` and `and` in clojurescript both run analysis as part of the macro expansion, which can lead to macros be expanded multiple times

People

Vote (1)
Watch (0)

Dates

  • Created:
    Updated: