ClojureScript

broken let cases

Details

  • Type: Defect Defect
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Completed
  • Affects Version/s: None
  • Fix Version/s: None
  • Component/s: None
  • Labels:
    None

Description

(assert (= "original" (let [x "original"
                            oce (fn [] x)
                            x "overwritten"]
                        (oce))))


(letfn [(x [] "original")
        (y [] (x))]
  (let [x (fn [] "overwritten")]
    (assert (= "original" (y)))))

Activity

Hide
Brandon Bloom added a comment - - edited

What's the problem? This output seems fine to me:

$ rlwrap ./script/repljs
"Type: " :cljs/quit " to quit"
#(assert (= "original" (let [x "original" 
                            oce (fn [] x) 
                            x "overwritten"] 
                        (oce))))
#<
function () {
    if (cljs.core._EQ_.call(null, "original", (function () {
        var x = "original";
        var oce = (function () {
            return x;
        });
        var x__$1 = "overwritten";
        return oce.call(null);
    })())) {
        return null;
    } else {
        throw (new Error([cljs.core.str("Assert failed: "), cljs.core.str(cljs.core.pr_str.call(null, cljs.core.with_meta(cljs.core.list("\ufdd1'=", "original", cljs.core.with_meta(cljs.core.list("\ufdd1'let", cljs.core.vec(["\ufdd1'x", "original", "\ufdd1'oce", cljs.core.with_meta(cljs.core.list("\ufdd1'fn", cljs.core.vec([]), "\ufdd1'x"), cljs.core.hash_map("\ufdd0'line", 14)), "\ufdd1'x", "overwritten"]), cljs.core.with_meta(cljs.core.list("\ufdd1'oce"), cljs.core.hash_map("\ufdd0'line", 16))), cljs.core.hash_map("\ufdd0'line", 13))), cljs.core.hash_map("\ufdd0'line", 13))))].join("")));
    }
}
>
#(letfn [(x [] "original") 
        (y [] (x))] 
  (let [x (fn [] "overwritten")] 
    (assert (= "original" (y)))))
#<
function () {
    var x = (function x() {
        return "original";
    });
    var y = (function y() {
        return x.call(null);
    });
    var x__$1 = (function () {
        return "overwritten";
    });
    if (cljs.core._EQ_.call(null, "original", y.call(null))) {
        return null;
    } else {
        throw (new Error([cljs.core.str("Assert failed: "), cljs.core.str(cljs.core.pr_str.call(null, cljs.core.with_meta(cljs.core.list("\ufdd1'=", "original", cljs.core.with_meta(cljs.core.list("\ufdd1'y"), cljs.core.hash_map("\ufdd0'line", 12))), cljs.core.hash_map("\ufdd0'line", 12))))].join("")));
    }
}
>

As an aside. The less-mangled names in the output are much more pleasant to look at

Show
Brandon Bloom added a comment - - edited What's the problem? This output seems fine to me:
$ rlwrap ./script/repljs
"Type: " :cljs/quit " to quit"
#(assert (= "original" (let [x "original" 
                            oce (fn [] x) 
                            x "overwritten"] 
                        (oce))))
#<
function () {
    if (cljs.core._EQ_.call(null, "original", (function () {
        var x = "original";
        var oce = (function () {
            return x;
        });
        var x__$1 = "overwritten";
        return oce.call(null);
    })())) {
        return null;
    } else {
        throw (new Error([cljs.core.str("Assert failed: "), cljs.core.str(cljs.core.pr_str.call(null, cljs.core.with_meta(cljs.core.list("\ufdd1'=", "original", cljs.core.with_meta(cljs.core.list("\ufdd1'let", cljs.core.vec(["\ufdd1'x", "original", "\ufdd1'oce", cljs.core.with_meta(cljs.core.list("\ufdd1'fn", cljs.core.vec([]), "\ufdd1'x"), cljs.core.hash_map("\ufdd0'line", 14)), "\ufdd1'x", "overwritten"]), cljs.core.with_meta(cljs.core.list("\ufdd1'oce"), cljs.core.hash_map("\ufdd0'line", 16))), cljs.core.hash_map("\ufdd0'line", 13))), cljs.core.hash_map("\ufdd0'line", 13))))].join("")));
    }
}
>
#(letfn [(x [] "original") 
        (y [] (x))] 
  (let [x (fn [] "overwritten")] 
    (assert (= "original" (y)))))
#<
function () {
    var x = (function x() {
        return "original";
    });
    var y = (function y() {
        return x.call(null);
    });
    var x__$1 = (function () {
        return "overwritten";
    });
    if (cljs.core._EQ_.call(null, "original", y.call(null))) {
        return null;
    } else {
        throw (new Error([cljs.core.str("Assert failed: "), cljs.core.str(cljs.core.pr_str.call(null, cljs.core.with_meta(cljs.core.list("\ufdd1'=", "original", cljs.core.with_meta(cljs.core.list("\ufdd1'y"), cljs.core.hash_map("\ufdd0'line", 12))), cljs.core.hash_map("\ufdd0'line", 12))))].join("")));
    }
}
>
As an aside. The less-mangled names in the output are much more pleasant to look at
Hide
David Nolen added a comment -
Show
David Nolen added a comment - nothing is broken, added test cases, http://github.com/clojure/clojurescript/commit/a3d2270480d5d4774ba20b587cdb7544546f4200
Hide
Herwig Hochleitner added a comment - - edited
(do
  (let [x "original"]
    (defn original-closure-stmt [] x))

  (let [x "overwritten"]
    (assert (= "original" (original-closure-stmt))))

is still broken, patch forthcoming

Show
Herwig Hochleitner added a comment - - edited
(do
  (let [x "original"]
    (defn original-closure-stmt [] x))

  (let [x "overwritten"]
    (assert (= "original" (original-closure-stmt))))
is still broken, patch forthcoming
Hide
Herwig Hochleitner added a comment -

should i make a new ticket?

Show
Herwig Hochleitner added a comment - should i make a new ticket?
Hide
Herwig Hochleitner added a comment - - edited

OK, I tried to patch the issue by wrapping every statement let in a function.
Unfortunately, because of continue; et al, that approach yields problems with lets inside a loop.
Maybe the original patches for http://dev.clojure.org/jira/browse/CLJS-401 should be reconsidered?

    1. EDIT: failing test case applying to current trunk is appended as of two comments before
Show
Herwig Hochleitner added a comment - - edited OK, I tried to patch the issue by wrapping every statement let in a function. Unfortunately, because of continue; et al, that approach yields problems with lets inside a loop. Maybe the original patches for http://dev.clojure.org/jira/browse/CLJS-401 should be reconsidered?
    1. EDIT: failing test case applying to current trunk is appended as of two comments before
Hide
David Nolen added a comment - - edited

I think we can probably make the current approach work with some more thought.

Show
David Nolen added a comment - - edited I think we can probably make the current approach work with some more thought.
Hide
David Nolen added a comment -

Reopening

Show
David Nolen added a comment - Reopening
Hide
Herwig Hochleitner added a comment -

The problem are recurs across a nested let, so wrapping with fns isn't an option, unless we want to special case lets containing a recur form.
Still thinking ...

Show
Herwig Hochleitner added a comment - The problem are recurs across a nested let, so wrapping with fns isn't an option, unless we want to special case lets containing a recur form. Still thinking ...
Hide
Herwig Hochleitner added a comment - - edited

Tracking let vars in the lexical scope won't solve it, the shadowing patch already does that. IMO that leaves globally tracking or gensyming let vars.

A design for compilation units would encompass global transformations like that. For now we can implement el-cheapo gensyming in the emitter, which I have done in previous ticket.

For reference, my considerations on compilation units here: http://dev.clojure.org/display/~bendlas/Design+for+compilation+units+in+ClojureScript

Show
Herwig Hochleitner added a comment - - edited Tracking let vars in the lexical scope won't solve it, the shadowing patch already does that. IMO that leaves globally tracking or gensyming let vars. A design for compilation units would encompass global transformations like that. For now we can implement el-cheapo gensyming in the emitter, which I have done in previous ticket. For reference, my considerations on compilation units here: http://dev.clojure.org/display/~bendlas/Design+for+compilation+units+in+ClojureScript
Hide
David Nolen added a comment -

subsumed by CLJS-401

Show
David Nolen added a comment - subsumed by CLJS-401

People

Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: