ClojureScript

Can we get the ticket # in the commit message? Thanks!

Of course, sorry! Here is a new patch, if you please.

Thanks. I'm confused about the second aspect of the patch, what do you mean by bogus error messages?

Well, the repl uses a :statement context to generate the javascript which gets printed out as part of a stack trace. When setting :expr back to :statement in the repl, you can try the following scenario:

• enter into the repl (def val :worked) (if (> (Math/rand) 0.5) val (throw ""))
• half of the time you will get :worked
• half of the time you will get a stacktrace citing 'ns.val = ":worked"; if (Math.rand() > 0.5){}{throw ""}'
• that makes no sense

With the full patch, the printout will be smth like '(function(){ns.val = ":worked"; if (Math.rand() > 0.5){return ns.val}{throw ""}})()', which is noisier, but more like what actually got evaluated by the repl client.

##EDIT#APPEND##

The reason the repl still works, even when analyzing in a statement context with the patch is, that in order to get the return value, the repl statement has to be evaluated in an :expr context anyway.
This happens by unquoting it into a wrapper form. With the patch the wrapper form also gets analyzed in an :expr context, which doesn't hurt.
Only when an exception gets caught, the js of the unwrapped expr is used, which now always matches the its counterpart in the wrapper form.

I'm still confused. Does this additional modification have anything to do with the ticket? By that I mean does the patch require the second modification? If it does can you explain a further? Thanks.

The patch doesn't require the second modification in the sense that everything still works if it's left out.

The patch requires the modification in the sense that it would break stack traces on the REPL otherwise + the two changes should be reverted together if and when we move such optimizations out of the emitter into an optimization pass.

So the trade off is in always having the correct code in a REPL stacktrace in exchange for making it more verbose. Again, this doesn't influence behavior, so it's a matter of prioritizing requirements.

I still don't understand why/how the part of the patch that addresses the ticket breaks stack traces. Can you explain?

1) var read statements get omitted
2) typing "var" into the repl still works, because it's analyzed in a wrapper form
3) if it throws, the printed generated js is the original form in a statement context, you can observe this with above code sample.

Ok I understand now, I don't think the second part of the patch is relevant. The user entered two statements, not one combined in an implicit do which is what the other part of the patch does. Can we get a new patch that only includes the part that addresses the ticket? Thanks!

For the record: There was some talk in the chat, where I laid down my rationale for changing the repl specifically: "Technically repl inputs have to be in an expr context, to capture the return val. And they are, by virtue of the wrapping form. The change makes that fact explicit, thereby keeping stack traces sane."

I also discovered, that two other ops, beside a var read, don't emit in a statement context either and therefore have same issue. I created http://dev.clojure.org/jira/browse/CLJS-403 for the REPL change.

Updated attached patch contains just the optimization.

fixed, http://github.com/clojure/clojurescript/commit/97e5fbd1e1597d58be35fd8320c8044ccc9d3a3d