Skip to content

Analyzer constant evaluation special-cases identical(int, double). #60647

Open
@lrhn

Description

@lrhn

The analyzer constant evaluation has a special case for identical(c1, c2) when one is a double and the other an int, in particular:

    // Workaround for Flutter `const kIsWeb = identical(0, 0.0)`.
    if (type.isDartCoreInt && rightOperand.type.isDartCoreDouble ||
        type.isDartCoreDouble && rightOperand.type.isDartCoreInt) {
      return DartObjectImpl(
        typeSystem,
        typeSystem.typeProvider.boolType,
        BoolState.UNKNOWN_VALUE,
      );
    }
    // ...

I think it should stop doing that.

The behavior is that if one value is an int and the other a double, the analyzer decides that it doesn't know if they are identical or not. (Even if they weren't even equal.)

That can have downstream consequences, where code that tries to have separate branches for JS-numbers and native numbers will have both branches analyzed, even if one of them will always fail.

As a degenerate example:

const int minNativeInt = (1 << 63); // Zero on web.
const int minJSInt = -(double.maxFinite / 2 ~/ 1 + 1) * 2; // Zero on native.

const bool jsNumbers = identical(1, 1.0);
class C {
  final int x;
  const C(int v)
    : assert(jsNumbers ? minJSInt != 0 : minNativeInt != 0), // Should succeed. Or throw here.
      x = jsNumbers ? (v ~/ minJSInt) : (v ~/ minNativeInt); // Should never fail with a division by zero.
}

void main() {
  print("Native: $minNativeInt");
  print("Web   : $minJSInt");
  const C(0); // Runs on all platforms, analyzer fails to analyze.
}

The real issue occurred in testing, where I needed a bigger delta than 1 for some tests when running with JS precision,
so I had const offset = jsNumbers ? 1024 : 1;. That ends up being an "unknown integer constant" in the analyzer, not either actual value I wanted.
Then I tested constant evaluation of const Duration(hours: max, microseconds: offset), and it did not trigger an assert that checked for overflow (probably because an "unkown bool" value of an assert does not report an error), but it did fail in a later branch guarded by the same condition as the assert, because it hit something that was intended to throw during non-constant evaluation, and be guarded by the assert in constant evaluation.

Which means I got spurious errors in an error test when run with the analyzer.

I think the analyzer should not special case here. It runs constant evaluation with either JS or native semantics, and it's not like it doesn't know which. (It's native.)

Trying to not admit that just pushes people to do other tests, like:

const jsNumbers = 0x20_0000_0000_0000 == (0x20_0000_0000_0000 + 1); // 2^54 (+ 1)

which compares only integers, still distinguishes when those integers are actually doubles, and doesn't pretend that it doesn't know in the analyzer.

(That's what I switched to in my test, and is putting into package:expect/variations.dart instead of identical(1, 1.0).)

Behavior seems to come from: #47404
The goal was to not consider identical(0, 0.0) a constant for fixes, since it can have different behaviors on different platforms. That's technically correct, but also inconsistent with the analyzer actually having only one number type, which is used for all other computations.
It makes kIsWeb useless as a guard for constant code which only works on one platform or the other.

I think the only actually sound way for the analyzer to consider both native and web behavior for constants is to let its constant values have two parallel paths, one with native integers and one with doubles instead of integers, and then implement constant computations in parallel. Otherwise the analyzer behavior won't be correct for either semantics.
And it'll be a whack-a-mole job to find all the comparisons users use to detect web number semantics (like the one above).

Metadata

Metadata

Assignees

No one assigned

    Labels

    analyzer-constantsarea-dart-modelFor issues related to conformance to the language spec in the parser, compilers or the CLI analyzer.model-const-evalImplementation of constant evaluation in analyzer/cfetype-bugIncorrect behavior (everything from a crash to more subtle misbehavior)

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions