language icon indicating copy to clipboard operation
language copied to clipboard

[wildcards] Late, wildcarded variables

Open eernstg opened this issue 1 month ago • 5 comments

We could make it an error, or we could lint it, but an initializing expression in a late variable declaration should presumably be recognized as dead code if the variable is wildcarded (that is, it is local and has the name _):

int f() {
  print('Work really hard on finding that int!');
  return 42; // got'yer
}

void main() {
  late int _ = f();
  late final int _ = f();
  // ... no matter what, we will not find that int ...
}

Similarly, a wildcarded late variable with no initializing expression can not be evaluated, and it can not have a new value assigned. So the variable itself would be "dead memory". Error? Lint? OK?

void main() {
  late int _;
  late final int _;
  // ... that memory is wasted ...
}

Perhaps we should just say that it is an error for a late variable to be wildcarded.

eernstg avatar May 13 '24 17:05 eernstg

Minimum a warning. It's dead code. That can be enough.

Same for any other late _-named variable. (I wouldn't assume that any memory is wasted, a halfway decent compiler should be able to just drop all the dead code.)

Personally, I'd warn about every local variable named _. The only case where they might have any effect is by giving a context type to some expression: Foo _ = (expression)..cascade(); where the type of the expression is affected by the context type. But still, just don't! Find a more readable way to get the same effect.

lrhn avatar May 13 '24 22:05 lrhn

Personally, I'd warn about every local variable named _ ... only ... giviing a context type

Right, it's a straightforward way to provide a context type. However, it could also be useful with some switches which are more conveniently expressed as switch expressions (for the syntax, or in order to enforce exhaustiveness).

eernstg avatar May 14 '24 07:05 eernstg

If you want a switch expression (switch (...) { ... }); is shorter than var _ = switch (...) { ... };.

Also, don't do that for brevity. If it's for exhaustiveness, we really should consider allowing switch!(expression) { ... } to be an exhaustive switch statement.

lrhn avatar May 14 '24 09:05 lrhn

OK, but if local variables that aren't formal parameters are so bad then perhaps just drop the support for such variables being wildcards?

eernstg avatar May 14 '24 09:05 eernstg

I'd be fine with that. It's consistent to make _ non-binding in var _ = ...;, but it's not particularly helpful. It's more breaking to disallow var _ = ...;, which is valid today, but the fix (to ...;) is so trivial that it should be easy to automate.

If we introduce a warning for var _ = ...; which are being referenced, "don't refer to variables named _" (with a quick-fix of renaming to a random fresh local name like x1), and we have the current "unused variable" warning when you don't refer to a local variable (with a quick-fix of removing the var _ = ), then we are effectivey guiding people away from ever writing that.

If we can get the first warning out soon, I'd be fine with completely disallowing local variable declarations named _ with the "wildcard-variable" feature.

The only worry is that it's inconsistent with patterns, where we allow you to write _:

var _ = e; // disallowed!
var (_) = e; // allowed?
var (_, x) = (42, e); // pretty much necessary.

The var _ = is no more or less unnecessary than the var (_) =, so it feels inconsistent if we treat them differently.

I'd warn about both, if the analyzer is up for it. Both are introducing unused local variable declarations, even if they are no longer introducing unused local variables.

(A var (_!) = e; or var (_ as int) = e; are not trivial, but are better rewritten to e!; and e as int;.)

lrhn avatar May 14 '24 09:05 lrhn

In #3820 it is argued that we should allow wildcarded late variable declarations, and leave it to lints and warnings to inform the developer that they are useless (an initializer is dead code, the memory is unused).

eernstg avatar May 19 '24 11:05 eernstg

Sounds like we'll probably let the analyzer warn on late wildcard variables. No changes needed for the spec.

Closing for now. If we need more discussion on this, feel free to reopen.

kallentu avatar May 20 '24 21:05 kallentu

@sgrekhov, do we need to revisit some co19 tests involving late wildcards?

eernstg avatar May 21 '24 07:05 eernstg

@sgrekhov, do we need to revisit some co19 tests involving late wildcards?

Yes, we need! I'll do it

sgrekhov avatar May 21 '24 09:05 sgrekhov