Skip to content
This repository has been archived by the owner on Jan 28, 2023. It is now read-only.

Disallow mixing nullish coalescing with logical OR via early errors #38

Closed

Conversation

DanielRosenwasser
Copy link
Member

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@ljharb
Copy link
Member

ljharb commented Jul 10, 2019

Are there issues this closes or relates to?

@rkirsling
Copy link
Member

#26 and #15 are the relevant ones.

@ljharb
Copy link
Member

ljharb commented Jul 10, 2019

Why not give them the same precedence, assuming #15 (comment) isn't wrong?

@rkirsling
Copy link
Member

Same precedence is the status quo, but it's problematic for the reasons laid out in #26 (comment).

Of the solutions discussed there, my personal vote would be to lower ??'s precedence, but I don't object to this approach either.

@claudepache
Copy link

claudepache commented Jul 10, 2019

The solution proposed in this PR is wrong, because it is treating the symptom (mixing ?? and || does the wrong thing) and not the root of the disease (the precedence of ?? is poorly chosen).

One of the problems provoked by the wrong precedence and not solved by this PR is apparent when comparing !(maybeA ?? b && c) and (maybeNotA ?? notB || notC).

@DanielRosenwasser
Copy link
Member Author

From the last TC39 meeting, we had some general agreement that disallowing the mix of both would be acceptable and would permit us to revisit at a later time. So the rationale to disallow the mix of both is:

  1. We did the same for exponentiation and unary +/-.
  2. It leaves room for design open if someone cares to do so later on.

My preference would probably be to align with other lower-precedence languages because same precedence feels like it would rarely be what I would want. I also get the sense that it's the behavior I've heard most others (both inside and outside of the committee) favor.

If there are no hard objections to lower precedence, I'm willing to bring that up at the next meeting and survey the room. However, I think that forbidding mixing the two is probably the best way to move forward and make progress.

@DanielRosenwasser
Copy link
Member Author

Any objections to the contents of the spec text? @codehag and @rkirsling?

@jridgewell
Copy link
Member

I think we should ban mixing ?? and && as well. It's not enough that && has always been tighter than || (and so of course it's tighter than ??), because people still get that precedence incorrect.

Kevin Gibbons created a Grammar Flag parsing at the last meeting that would work perfectly: https://gist.github.com/bakkot/332ddeb26978c41613abc092120448b9

@ljharb
Copy link
Member

ljharb commented Jul 12, 2019

@jridgewell would that mean that parens are always syntactically required to mix ?? with ||/&&?

@jridgewell
Copy link
Member

Yes

@DanielRosenwasser
Copy link
Member Author

I can add the nullish grammar flag, but I wonder if it's not just cleaner to issue the early error.

@DanielRosenwasser DanielRosenwasser changed the title Disallow mixing nullish coalescing with logical OR. Disallow mixing nullish coalescing with logical OR via early errors Jul 12, 2019
@DanielRosenwasser
Copy link
Member Author

#39 changes the grammar directly, but I'm not sure 4 grammar parameters is the prettiest thing I've seen in this spec.

@rkirsling
Copy link
Member

I believe the spec text here says what it intends to; it's merely a question of what we want the state of the proposal to be during the upcoming meeting:

  • I think this is a reasonable state to have things be in if we're aiming to "pick up from where we left off", as the discussion last time seemed to center around ?? being ||-like.

  • One could also argue for leaving things as they are for now, since many of us are hoping to lower precedence instead, in which case the change here would be unnecessary.

  • I don't feel comfortable making a change that affects && so hastily—?? is not &&-like, so this ought to require a discussion of its own. (Although I guess it's taken me long enough to write this that Add grammar flags to disallow mixing ?? with || and &&. #39 already exists, so I can express my thoughts over there.)

Overall though, it would be nice to allow @DanielRosenwasser to try for stage 3 with the status quo reflecting some solution to #26. 😂

@bakkot
Copy link

bakkot commented Jul 12, 2019

@claudepache

One of the problems provoked by the wrong precedence and not solved by this PR is apparent when comparing !(maybeA ?? b && c) and (maybeNotA ?? notB || notC).

I don't understand the problem this is pointing at; can you expand? The thing that I get when comparing those under this PR is "the first one behaves reasonably" and "the second one is not valid JS", which, while it is perhaps slightly surprising while performing that transformation, doesn't seem terrible.

@claudepache
Copy link

claudepache commented Jul 12, 2019

@bakkot Consider some expression mixing operators of different categories:

A && B == C 

You have to learn whether it means (A && B) == C or A && (B == C), but once you know that, you have strong expectations about the semantics of the following expression:

A || B == C

It would be surprising if the latter triggers a syntax error.

But it does work as expected, because every comparison operators (==, <=, etc.) have a strictly higher precedence level than every logical operator (||, &&).


Returning to the original issue. The root of the problem is that the following is currently not true: “Logical operators (||, &&) have either a strictly higher, or a strictly lower precedence level than nullish-coalescing.” (EDIT: “... or it is completely prohibited to mix any logical operator with nullish-coalescing”, in which case it is meaningless to speak about their relative precedence level)

@bakkot
Copy link

bakkot commented Jul 12, 2019

@claudepache Thanks for expanding, that makes sense to me. Your last statement applies equally to #39, strictly speaking (which bans mixing with both || and &&); do you have the same issue with it?

@bakkot
Copy link

bakkot commented Jul 12, 2019

@DanielRosenwasser FWIW I find the grammar flags much cleaner than prose Early Errors, but that's an editorial question which can be resolved at a later stage.

@claudepache
Copy link

claudepache commented Jul 12, 2019

@bakkot Your last statement applies equally to #39, strictly speaking

My last statement is incomplete, because it overlooked the possibility of completely disallowing the mixing of operators of two distinct categories (in which case, speaking of their relative precedence level is meaningless). — So, no, #39 has not the same issue.

@ljharb
Copy link
Member

ljharb commented Jul 12, 2019

?? is in the same category as || imo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants