Skip to content

[Breaking change] Account for field promotion to Null when computing reachability based on equality testsΒ #56893

@stereotype441

Description

@stereotype441

Background

Normally, reachability analysis understands that comparing two values with static type Null is guaranteed to succeed, e.g.:

int test() {
  if (null == null) {
    return 0;
  }
  // No `return` statement but that's ok because this code path is unreachable
}

This works even if one of the values being compared is Null solely because it got type promoted via an is test:

int test(int? value) {
  if (value is! Null) return 0; // promotes `value` to type `Null`
  if (value == null) { // guaranteed to succeed
    return 0;
  }
  // No `return` statement but that's ok because this code path is unreachable
}

(Note that an is or as test is the only way to promote to the type Null. Type promotion doesn't promote values to the type Null based on an == check because it isn't really that useful to do so. And we have an analyzer warning that discourages people from using is Null or is! Null, so probably this doesn't arise very often in practice).

Unfortunately, due to a bug in how reachability analysis tracks field type promotion, it doesn't work if the value being compared is a promoted field:

class C {
  final int? _f;
  C(this._f);
}
int test(C c) {
  if (c._f is! Null) return 0; // promotes `c._f` to type `Null`
  if (c._f == null) { // reachability analysis fails to understand that this is guaranteed to succeed
    return 0;
  }
  // Error: missing `return` statement
}

Proposed change

The above-described bug will be fixed, so that if one of the values on the left or right side of == or != is promoted to the type Null, this promoted type will be used to determine reachability of the code that follows.

Rationale

Fixing this issue makes the reachability analysis of == and != tests dependent solely on the static types of the expressions being compared. This will allow the implementation of reachability analysis to be simplified.

Expected impact

This change is expected to be extremely low impact, for the following reasons:

  • Field promotion does not occur very often
  • Promoting to the type Null is rare (and discouraged)
  • Comparing a value to null when it is known to have type Null is not particularly useful
  • Usually, it is low impact for a code path that was previously considered reachable to be reclassified to unreachable, since for the most part, code being unreachable only results in a warning.

However, it's conceivable that a code path being unreachable could result in a change to type promotion behavior, which could in turn lead to type errors later in the program. Here is an example of how that could occur:

class C {
  final int? _f;
  C(this._f);
}
void test(C c, int? x) {
  if (c._f is! Null) return; // promotes `c._f` to type `Null`
  if (x == null) return; // promotes `x` to type `int`
  if (c._f != null) {
    // considerd reachable today; with the fix, will be considered unreachable
    x = null; // demotes `x` back to `int?` today; with the fix, will have no effect
  }
  var y = x; // inferred type is `int?` today; with the fix, will be `int`
  y = null; // ok today; with the fix, will be an error.
}

(Note that this example is quite contrived; I haven't come up with a real-world example where there is a problem.)

Mitigation

In the unlikely event that this change affects the behavior of your code, it will affect it by way of changing an inferred type (as in the example above). If this happens, you can restore the old behavior by adding explicit types to the program. (For example, in the code above, the old behavior could be restored by changing var y = x; to int? y = x;.)

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status

    Complete

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions