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

Nullable compound assignments are inconvenient. #1113

Open
lrhn opened this issue Jul 23, 2020 · 19 comments
Open

Nullable compound assignments are inconvenient. #1113

lrhn opened this issue Jul 23, 2020 · 19 comments
Labels
nnbd NNBD related issues request Requests to resolve a particular developer problem

Comments

@lrhn
Copy link
Member

lrhn commented Jul 23, 2020

With the current Null Safety behavior, a nullable variable like int? x; cannot be incremented as easily as a non-nullable variable, even when you know that it currently contains a non-null value.

We have ++x and x += 1 as easy composite operations on non-nullable types, but for the nullable type, when you know it's not null, the only option is x = x! + 1 because we need to insert the ! in the middle of the expression that ++x or x += 1 expands to.

It would be nice to have something like x!+=1 which expands to x = x! + 1.

Draft writeup: https://github.com/dart-lang/language/blob/9a83a94c0278180674dfdd09d97c9eab3a624221/working/1113%20-%20null-asserting%20compound%20assignment/proposal.md

@lrhn lrhn added the request Requests to resolve a particular developer problem label Jul 23, 2020
@creativecreatorormaybenot
Copy link
Contributor

I would say x!++ or ++x! seem decent too.

@munificent munificent changed the title Nullable compound assignments are invoncenient. Nullable compound assignments are inconvenient. Jul 24, 2020
@leafpetersen
Copy link
Member

@lrhn are you still planning to push on this?

@Hixie
Copy link

Hixie commented Aug 21, 2020

FWIW I haven't seen much of a need for this in practice yet.

@creativecreatorormaybenot
Copy link
Contributor

creativecreatorormaybenot commented Aug 21, 2020

@Hixie 👍 me neither

Have written >100k loc in NNBD and not encountered it.

Theory: ++ is rarely used anyway, but when it is used, it is used mostly for loops. When you loop and have an index for your loop, that index will be non-nullable to begin with.

Examples:

for (var i = 0; i < n; i++)
var i = n;

while (i > 0) {
  i--;
}

(same roughly goes for the other compound assignments)

@Hixie
Copy link

Hixie commented Aug 21, 2020

For ++ specifically I'd be fine with just removing it from the language. :-D

@rrousselGit
Copy link

For ++ specifically I'd be fine with just removing it from the language. :-D

Haha, off topic but any reason?

@Hixie

This comment has been minimized.

@lrhn
Copy link
Member Author

lrhn commented Aug 21, 2020

I don't think it's worth pushing it right now. We have enough on our plate for the release. It's a feature that can be safely added later since it introduces new syntax which is currently invalid.

I think it is worth keeping the issue open in case users end up wanting to use null-aware compound assignments. Then they have a place to report that.

@alextekartik
Copy link

As I was experimenting NNBD, I ran into a similar issue. I post here a sample code in case it is relevant. I think the lint warning should not happen here.

class A {
  String? value;
}

class B {
  A? a;
}

void main() {
  var b = B();
  b.a = A();
 
  // Ok
  b.a!.value = '123';

  // Lint error!
  // An expression whose value can be 'null' must be null-checked before it can be dereferenced.
  b.a!.value += '456';

}

@creativecreatorormaybenot
Copy link
Contributor

@alextekartik The lint is correct because the class member could change from the outside. See dart-lang/sdk#42626.

So the flow analysis cannot be sure that a!.value was modified somewhere else, e.g. when you have a custom setter.

@alextekartik
Copy link

@creativecreatorormaybenot Ok I understand. I guess it was just puzzling but you are right it is correct.

@ravenblackx
Copy link

I think it might make sense to simplify this issue to "the non-null assertion operator ! should not remove assignability".

If that could be achieved it would solve for all the cases x!++ (where x is a nullable int), someMap[someKey]!++, etc.

It's clear that the syntax is parsing appropriately for this already, since the error on trying to do it that way right now is Illegal assignment to non-assignable expression

Note, changing it this way would not introduce any new syntax, it would just make the existing syntax work the way a user would expect.

@munificent
Copy link
Member

Note, changing it this way would not introduce any new syntax, it would just make the existing syntax work the way a user would expect.

(Puts on pedantic hat.) It is technically a change in syntax, which is why the current syntax yells at you. The thing to the left of ++ or = is not an expression, it's an assignment target. In C/C++ terms, it's the difference between an lvalue and an rvalue. It happens to be the case that all lvalues are a subset of the expression syntax, but there are many expressions that are not valid assignment targets, like:

1 + 2 = 3;
[4, 5, 6] = 7;
(parentheses) = value;
-negative = positive;

Assignment targets are also the things allowed to the left of an increment/decrement operator. The ! null-assertion operator is yet another expression that has no corresponding assignment target sibling. We would have to add new syntax (and semantics) to allow a ! there.

@ravenblackx
Copy link

(Puts on pedantic hat.) It is technically a change in syntax, which is why the current syntax yells at you.

Semantic perspective difference I think. From the user perspective the syntax isn't new with that change, it's the existing syntax with different (better) behavior. But fair enough if from the dart developer perspective it's an addition of a different operator. :)

@jaumard
Copy link

jaumard commented Mar 18, 2021

I've encounter this use case and was wondering why it didn't work:

enum Stat {
  strength,
  agility,
  wisdom,
  charisma,
}

 @observable
  ObservableMap<Stat, int> stats = ObservableMap.of({
    Stat.strength: 1,
    Stat.agility: 1,
    Stat.charisma: 1,
    Stat.wisdom: 1,
  });

  @observable
  ObservableMap<Stat, int> updatedStats = ObservableMap.of({
    Stat.strength: 0,
    Stat.agility: 0,
    Stat.charisma: 0,
    Stat.wisdom: 0,
  });

  @action
  void levelUp() {
//this works fine
    stats[Stat.strength] = stats[Stat.strength]! + updatedStats[Stat.strength]!;
    stats[Stat.agility] = stats[Stat.agility]! + updatedStats[Stat.agility]!;
    stats[Stat.charisma] = stats[Stat.charisma]! + updatedStats[Stat.charisma]!;
    stats[Stat.wisdom] = stats[Stat.wisdom]! + updatedStats[Stat.wisdom]!;

// but wanted to write this at first but doesn't compile ^^ I was a bit frustrated lol  
    stats[Stat.strength]! += updatedStats[Stat.strength]!;
    ....
  }

Just wanted to share :)

@leafpetersen leafpetersen added the nnbd NNBD related issues label Mar 24, 2021
@stuartmorgan
Copy link

Capturing from a chat discussion, I've seen this come up in a few places now specifically for counting maps, where the pattern is often to initialize everything to zero, then do a lot of foo[bar]++ (or foo[bar] += 1) for the counting. Having to put the lookup on both sides hurts maintainability since it creates the possibility of a bug where the key is changed in only one of the two places, and is presumably worse for efficiency as well (unless in efficiency terms it was actually expanding out to two lookup under the hood before?)

@bkonyi
Copy link

bkonyi commented Jun 25, 2021

I've started running into this as well when working with package:vm_service. We have lots of nullable fields since the package needs to be able to support potentially missing properties, so where I could previously do:

function.inclusiveTicks++;

I now need to do:

function.inclusiveTicks = function.inclusiveTicks! + 1;

Which is significantly more verbose and doesn't feel very Dart-y :-(

@nyck33
Copy link

nyck33 commented Aug 20, 2021

Yes this is ugly. Should be able to do x!++;
I'm a novice coming mostly from Python so not sure if this even makes sense but another option could be (x==null) ? 0 : x++; or x ? x++ : 0;

@Cyp
Copy link

Cyp commented Jan 17, 2022

Also ought to be able to do x?++, I think.

// x = x! + 42;
x! += 42;

// x == null ? null : (x = x! + 42);
x? += 42;

// x == null ? null : (x = x! + doSomethingWithSideEffects());
x? += doSomethingWithSideEffects();

// x = y == null || z == null ? null : y! + z!;
x = y? + z?;

// x == null ? null : (x = x! * y + z);
x? = x * y + z;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
nnbd NNBD related issues request Requests to resolve a particular developer problem
Projects
None yet
Development

No branches or pull requests