Tuning soak compilation

[satyr]

After some struggling (#733, b0e34edf), I'm still unsure about some compilations with soak. So here's a public question:

What should these compile to?

### 1 ###
a?.b.c + 1

### 2 ###
a?.b.c++

### 3 ###
a?.b.c += 1

### 4 ###
a.b?().c += 1

[michaelficarra]

Alright, I'll be the first to embarrass myself and take a stab at it. For readabilty's sake, I'm going to write synonymous coffeescript instead of JS.

### 1 ###
a.b.c + 1 if a?

### 2 ###
a.b.c++ if a?

### 3 ###
a.b.c += 1 if a?

### 4 ###
a.b().c += 1 if a.b?

[StanAngeloff]

I think michaelficarra has a very good point. Instead of unfolding soaks and always trying to get them right, why not just turn them into If nodes? Every ? should break and append one more expression so a?.b.c?.d becomes a? and a.b.c? It should work for function invocations, property access, indexing, etc. Could be as simple as traversing the AST and building up the If nodes, no?

[michaelficarra]

Yes, that's exactly what I was trying to get across, Stan. Thanks for wording it so well for me.

[satyr]

a.b.c + 1 if a?

The problem with this is unintuitivity. When a of a?.b.c or d is null, What do you expect the expression return?

why not just turn them into If nodes

Yep, that's what it currently does (take a look at those unfoldSoaks in nodes.coffee).

FYI, the examples currently compile to:

/* 1 */
((typeof a !== "undefined" && a !== null) ? a.b.c : undefined) + 1;
/* 2 */
((typeof a !== "undefined" && a !== null) ? a.b.c : undefined)++;
/* 3 */
((typeof a !== "undefined" && a !== null) ? a.b.c : undefined) += 1;
/* 4 */
(typeof a.b !== "function" ? undefined : a.b()).c += 1;

[michaelficarra]

The problem with this is unintuitivity. When a of a?.b.c or d is null, What do you expect the expression return?

null, of course. Or undefined, whatever your preference. I prefer to stay away from undefined. The whole expression containing the soak is wrapped in an if with the condition being a conjunction of the soaked values.

[satyr]

What about 1 + a?.b.c?

The whole expression containing the soak is wrapped in an if with the condition being a conjunction of the soaked values

It was doing like that before b0e34edf. Here's the conversation that led to the commit.

(matyr) what should null?.p or 1 return?

(jashkenas) 1

(jashkenas) right?

(matyr) yep

(matyr) currently it returns undefined, which is counterintuitive

[michaelficarra]

I was hoping someone else would weigh in on this issue either in agreement or disagreement. In response to

What about 1 + a?.b.c?

I would say 1 + a.b.c if a?, but that should be obvious by now.

I do see the counterintuitive problem with the expression null?.p or true. With my proposed method, it would return null/undefined, but that is quite counterintuitive. Does anyone else have an opinion on this?

[satyr]

1 + a.b.c if a?

Propagating to upstream is more difficult to understand (and implement). You wouldn't expect b() in a() or b()?.c to be called before a().

It makes sense to unfold against mutating operations (=, +=, ++, delete), but not others.

[satyr]

As of 90a13bd7, the compilations are:

/* 1 */
((typeof a !== "undefined" && a !== null) ? a.b.c : undefined) + 1;
/* 2 */
((typeof a !== "undefined" && a !== null) ? a.b.c++ : undefined);
/* 3 */
((typeof a !== "undefined" && a !== null) ? (a.b.c += 1) : undefined);
/* 4 */
(typeof a.b === "function" ? (a.b().c += 1) : undefined);

[jashkenas]

Thanks for the match, satyr -- closing the ticket.

[jashkenas]

As mentioned here:

#1033 (comment)

I'd like to hear a few more comments about the desirability of unfoldSoak in general. I can't really decide if it's magical in a useful way, or magical in a way that's merely unexpected. In particular, would you ever find yourself writing code like this on purpose?

result = a?.b.c + 1

a?.b.c += 1

++a?.b.c

delete a?.b.c

[michaelficarra]

@jashkenas: I don't think I'd write code like that. It makes sense, but I believe it's unnecessary and ultimately adds confusion. @satyr: I know you're going to come up with a really convincing counter-example now.

[satyr]

@satyr: I know you're going to come up with a really convincing counter-example now.

I no longer mind Coffee removing them, but the point was they should be valid as LHS expressions.

Are you removing a?.b() → if a? then a.b() expansion as well?

[TrevorBurnham]

I'd call it magical in a useful way. I've certainly used

options.callback?()

as a common idiom. And I could easily see myself writing

ship.shields?.power -= 0.5 for ship in ships when ship.isInNebula()

I'd say it's an underused feature right now, but I don't see it as adding confusion.

[jashkenas]

Lets leave unfoldSoak in for now ... the painful memory of exponential compile times for long method chains has faded.