Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support "&&=" and "||=" operators #26996

Closed
1 of 7 tasks
munificent opened this issue Aug 1, 2016 · 23 comments
Closed
1 of 7 tasks

Support "&&=" and "||=" operators #26996

munificent opened this issue Aug 1, 2016 · 23 comments
Assignees
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). customer-flutter type-enhancement A request for a change that isn't a bug

Comments

@munificent
Copy link
Member

munificent commented Aug 1, 2016

Dart defines the following "regular" binary operators *, /, %, ~/, +, -, <<, >>, >=, >, <=, <, ==, &, ˆ, |. By "regular", I mean that they do no short-circuiting and are just syntactic sugar for a regular method call. These can all be implemented by user-defined classes.

It also defines three short-circuiting binary operators ||, &&, and ??. These cannot be overridden and do not desugar to method calls.

The regular operators also have self-assignment forms (except for the comparison operators), +=, -=, etc. It used to be the case that none of the short-circuiting operators did. However, when ?? was added, ??= was too. Given that, there is no good reason to omit ||= and &&=. They are consistent with ??= and—more importantly!—are sometimes useful.

So, to harmonize this, we intend to add ||= and &&= to Dart.

Syntax

We add &&= and ||= to compoundAssignmentOperator in the grammar.

Semantics

The semantics are pretty straightforward. The general vibe is that evaluation is "maximally lazy". A subexpression is evaluated at most once, and only if its result is needed.

To describe the desugaring, we need something akin to a "template language". It's basically Dart syntax, with a couple of extra bits:

  • C is any identifier that resolves to a type name.
  • A name starting with e is a placeholder for an arbitrary expression (except for a typename). It gets inserted as-is in the result.
  • name can be any identifier and is substituted as-is.
  • bool_convert() invokes the "boolean conversion" procedure in the language spec.

&&=

// An expression like `name &&= e3` desugars to:
(() {
  if (!name) return false;
  return name = bool_convert(e3); 
})()

// An expression like `C.name &&= e3` desugars to:
(() {
  if (!C.name) return false;
  return C.name = bool_convert(e3); 
})()

// An expression like `e1.name &&= e3` desugars to:
(() {
  var x = e1; // Don't double evaluate e1.
  if (!x.name) return false;
  return x.name = bool_convert(e3);
}())

// An expression like `e1?.name &&= e3` desugars to:
(() {
  var x = e1; // Don't double evaluate e1.
  if (x == null) return null;
  if (!x.name) return false;
  return x.name = bool_convert(e3);
}())

// An expression like `e1[e2] &&= e3` desugars to:
(() {
  var x = e1, y = e2; // Don't double evaluate e1 and e2.
  if (!x[y]) return false;
  return x[y] = bool_convert(e3);
}())

||=

// An expression like `name ||= e3` desugars to:
(() {
  if (name) return true;
  return name = bool_convert(e3); 
})()

// An expression like `C.name ||= e3` desugars to:
(() {
  if (C.name) return true;
  return C.name = bool_convert(e3); 
})()

// An expression like `e1.name ||= e3` desugars to:
(() {
  var x = e1; // Don't double evaluate e1.
  if (x.name) return true;
  return x.name = bool_convert(e3);
}())

// An expression like `e1?.name ||= e3` desugars to:
(() {
  var x = e1; // Don't double evaluate e1.
  if (x == null) return null;
  if (x.name) return true;
  return x.name = bool_convert(e3);
}())

// An expression like `e1[e2] ||= e3` desugars to:
(() {
  var x = e1, y = e2; // Don't double evaluate e1 and e2.
  if (x[y]) return true;
  return x[y] = bool_convert(e3);
}())

This desugaring has one wrinkle. It doesn't handle cases where the surrounding code is async or a generator since the function literals don't correctly propagate the "async-ness" or "generator-ness". Instead of actual functions, imagine them to be "block expressions" that let you define variables scoped to the expression instead of actually declaring a function, and you'll have the right idea.

Flag

While this is being implemented, to ensure we don't expose users to inconsistency in our tools, it should be put behind a flag named logical-self-assign.

Tracking issues

@munificent munificent added area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). area-multi type-enhancement A request for a change that isn't a bug labels Aug 1, 2016
@scheglov
Copy link
Contributor

scheglov commented Aug 1, 2016

In "An expression likee1[e2] &&= e3desugars to:" and "An expression likee1[e2] ||= e3desugars to:" it seems that e2 is evaluated twice.

@munificent
Copy link
Member Author

Oops, fixed!

@jmesserly
Copy link

do we want a DDC bug too? if so it's here: dart-archive/dev_compiler#617

@a14n
Copy link
Contributor

a14n commented Aug 1, 2016

The desugar of e1?.name &&= e3 and e1?.name ||= e3 looks strange to me. Shouldn't it be :

// An expression like `e1?.name &&= e3` desugars to:
(() {
  var x = e1;
  if (x == null) return null;
  var y = x.name;
  if (!y) return y;
  return x.name = e3;
}())

// An expression like `e1?.name ||= e3` desugars to:
(() {
  var x = e1;
  if (x == null) return null;
  var y = x.name;
  if (y) return y;
  return x.name = e3;
}())

@a14n
Copy link
Contributor

a14n commented Aug 1, 2016

I also notice that if e1 = null and e3 = true then e1 = e1 && e3 leads to e1 equals to false but e1 &&= e3 leads to e1 equals to null. Is it intended ?

@munificent
Copy link
Member Author

do we want a DDC bug too? if so it's here: dart-archive/dev_compiler#617

Yes, added it to the list, thanks!

Is it intended ?

Oof, no. Thought it through better and updated the above. This will teach me to put out a proposal before I write tests. :)

@lrhn
Copy link
Member

lrhn commented Aug 2, 2016

I would also expect the rules for &&= to be:

// An expression like `v &&= e3` desugars to:
(() {
  var x = v;
  if (x) return true;
  return v = boolean_conversion(e3); 
})()

// An expression like `e1.name &&= e3` where `e1` is not a type name, desugars to:
(() {
  var x = e1;
  var y = x.name;
  if (y) return true;
  return x.name = boolean_conversion(e3);
}())

// An expression like `e1?.name &&= e3` desugars to:
(() {
  var x = e1;
  if (x == null) return null;
  var y = x.name;
  if (y) return true;
  return x.name = boolean_conversion(e3);
}())

// An expression like `e1[e2] &&= e3` desugars to:
(() {
  var x = e1;
  var y = e2;
  var z = x[y];
  if (z) return true;
  return x[y] = boolean_conversion(e3);
}())

and similar for ||=.
The boolean_conversion function refers to boolean conversion (16.4.1) in the spec, but there isn't currently a syntax for performing that in code.

We also need a specail case for C.name &&= e3 where C is a type name. Otherwise the operation can't be used on static variables because the e1.name case evaluates e1 to a Type object.

(As usual, the rewrite using an applied function expression isn't sound (#25858), but it's consistent with the other unsound rewrites in the spec).

@jmesserly
Copy link

The boolean_conversion function refers to boolean conversion (16.4.1) in the spec, but there isn't currently a syntax for performing that in code.

you could write x ? x : x ... although that's a bit of a hacky way to express it :)

@munificent
Copy link
Member Author

munificent commented Aug 2, 2016

The boolean_conversion function refers to boolean conversion (16.4.1) in the spec, but there isn't currently a syntax for performing that in code.

Ah, right. I didn't realize && and || return the boolean conversion of the RHS. Changed the desugaring to use && and || which I think addresses this.

We also need a special case for C.name &&= e3 where C is a type name.

Done.

@eernstg
Copy link
Member

eernstg commented Aug 3, 2016

We do have a bunch of ways to say this, I think we're just converging on the level of obscurity of presentation that we are ready to accept. See this gist for some variants of the semantic specification. A tricky point is, for instance, how many times can we call the relevant getter and setter? If we wish to fix that to "call the getter exactly once" and "call the setter at most once, and do not call it in the case that corresponds to shortcut evaluation of the underlying operator && and ||" then the presentation gets a bit less readable.

@eernstg
Copy link
Member

eernstg commented Aug 3, 2016

PS: x ? x : x evaluates to the value of x, not the boolean-converted value. ;-)

@jmesserly
Copy link

PS: x ? x : x evaluates to the value of x, not the boolean-converted value. ;-)

(haha, oops. In my head I've already moved on to strong mode/dart2 where a "bool" is actually true, false, or null, so I was just trying to get the cast+assert-non-null ... yeah, in Dart 1 you'd need x ? true : false to get both the assert(x != null) and the identical(x, true) part... anyway, I'll stop sidetracking this bug with my silly comments :) )

@lrhn
Copy link
Member

lrhn commented Aug 6, 2016

One shorthand that could work is, say for v.name &&= e:

  (v.name && (v.name = boolean_convert(e)))

You still need the temporary variables when the LHS isn't simple, like e1[e2] &&= e3:

  (x = e1)[y = e2] && x[y] = boolean_convert(e3)

(where x and y are fresh variables and boolean_convert might just be !!).

@fsc8000
Copy link
Contributor

fsc8000 commented Aug 18, 2016

Why not just have the following rule:

e1 op= e2 is equivalent to e1 = e1 op e2 except that e1 is only evaluated once.

I think this is true for all existing compound assignment operators.

@munificent
Copy link
Member Author

except that e1 is only evaluated once.

That's more or less what the proposal specifies. It's just that saying "e1 is only evaluated once" doesn't precisely capture that fact that it appears in both lvalue and rvalue position and the distinctions between the two.

@lrhn
Copy link
Member

lrhn commented Aug 19, 2016

You will never get x ? true : false through a code review with me 😈
Alternatives are x == true which doesn't throw in checked mode, or !!x which does.

That latter is idiomatic in JavaScript, but Dart has rarely needed it because most programs are checked-mode correct, so non-boolean values almost never end up in a boolean situation anyway.

This specification has to account for it since it is defining a new boolean context, but everybody else should generally just not worry.

@lrhn
Copy link
Member

lrhn commented Aug 19, 2016

The reason for lhs &&= rhs not being equivalent to lhs = lhs && rhs is that it's unnecessary to do the assignment when the short-circuit operator bails out. In that case, it will be equivalent to lhs = lhs, and then it's easier to just not do the assignment.
We did the same thing for ??= - if the left-hand operand is null, we don't assign null back to it again.

It gets slightly more complicated because unchecked mode can allow a non-bool left operand to evaluate to false, so lhs = lhs && rhs is bailing out but actually assigning a different (now bool) value back.
Still, the program is bad and the author should feel bad. It would throw in checked mode, and I'm not particularly worried about not assigning a boolean back to the variable in this case.

@eernstg
Copy link
Member

eernstg commented Aug 19, 2016

Executing the setter in the case where shortcutting is supposed to occur could have arbitrary side-effects, so it does matter.

@fsc8000
Copy link
Contributor

fsc8000 commented Aug 19, 2016

Correct @eernstg . The question is if the assignment is executed or not. I think it is important that the programmer, who refactors code from E1 = E1 op E2 into E1 op= E2 does not get caught by surprise, because this refactoring is a breaking change for the short-circuit operators?

@lrhn If the assignment boils down to assigning back the value itself (i.e. the setter is a simple assignment), the compiler would figure this out and not perform the store. - If it does not do this right now, it would be easy to make sure it does.

I would rather change ??= to be consistent with the rule I proposed than adding these new operators with surprising semantics.

@munificent
Copy link
Member Author

because this refactoring is a breaking change for the short-circuit operators?

It's potentially a breaking change but is rarely one in practice.

the compiler would figure this out and not perform the store

That's hard to do in the general case since setters can have arbitrary side effects.

I would rather change ??= to be consistent with the rule I proposed than adding these new operators with surprising semantics.

That would be a breaking change to the language.

I don't personally have a strong opinion here (in fact, I don't personally care about this feature at all), I just tried to come up with semantics that are consistent with ??= and had a coherent principle behind it.

The other logical operators seem to have a rule of being "maximally lazy"—they only evaluate a subexpression if needed, so I did the same thing here.

@floitschG floitschG mentioned this issue Aug 25, 2016
16 tasks
@munificent munificent added area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. web-dart2js area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. web-dev-compiler and removed area-multi labels Sep 2, 2016
@munificent munificent removed area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. web-dart2js web-dev-compiler area-vm Use area-vm for VM related issues, including code coverage, FFI, and the AOT and JIT backends. labels Dec 13, 2016
@srawlins
Copy link
Member

Most of the "Tracking issues" have been closed as "Will implement in CFE". I don't know if there is an issue for CFE because GitHub search does not like &&= or ||=...

@jmesserly
Copy link

Most of the "Tracking issues" have been closed as "Will implement in CFE". I don't know if there is an issue for CFE because GitHub search does not like &&= or ||=...

it looks like it is at least partially implemented, but disabled in CFE (LAZY_ASSIGNMENT_ENABLED). I wasn't able to find any open CFE bugs tracking this. I don't know whether it is implemented beyond parsing.

Analyzer has some support as well, and is also disabled (scanLazyAssignmentOperators).

@munificent munificent removed the area-language Dart language related items (some items might be better tracked at github.com/dart-lang/language). label Jul 11, 2018
@vsmenon
Copy link
Member

vsmenon commented Jul 29, 2020

Closing this in favor of dart-lang/language#23

@vsmenon vsmenon closed this as completed Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-meta Cross-cutting, high-level issues (for tracking many other implementation issues, ...). customer-flutter type-enhancement A request for a change that isn't a bug
Projects
No open projects
2.0 language
  
Awaiting triage
Development

No branches or pull requests

9 participants