"as" and "instanceof" are inconsistent when are used with impossible type

[m4tiz5zktonj]

Noticed the following during the refactoring:

import Toybox.Lang;

function f(x as Number) as Number {
    if ((x as String) == "") { // "The type Number cannot be converted to String because they have nothing in common" - OK
        return 0;
    } else if (x instanceof String) { // No warnings - NOK
        return x.length();
    } else {
        return x;
    }
}

PMC gives warning about bad cast x as String, but doesn't about bad condition x instanceof String. Is it possible to improve instanceof check to work the same way as the as?

[markw65]

What the optimizer actually does is optimize away the if entirely.

While that is superficially inconsistent with the cast warning, it's entirely consistent with the rest of the optimizer. And I think it's the correct behavior.

In your case, it's obvious from the start that x instanceof String is false. But suppose you had:

(:featureX)
function getX(f as Boolean) as Number or String { return f ? 42 : "Hello" }
(:noFeatureX)
function getX(f as Boolean) as Number { return 42; }

function foo(f as Boolean) {
  var x = getX(f);
  if (x instanceof String) {
    // do something
  }
}

Now for some devices the instanceof is "redundant" and for others it's not. Issuing a warning would be annoying, because in some builds the instanceof check is required.

But you can't make the same argument for the cast - it's always wrong (in the sense that it is an incorrect assertion about the type of x - in your actual example the comparison will just return false).

[m4tiz5zktonj]

OK, thanks for the explanation, let's leave it as is.

My code has a heavy typedef instead of Number, and as I mentioned, I was refactoring - I was removing and adding some types from/to that typedef, and was expecting PMC to find all broken parts of code during the analysis phase so I cound update them accordingly. I've found that monkeyc reports such always false instanceof checks (but completely ignores wrong as expressions), so I'll just use both tools to cover all cases.

The following doesn't contradict anything, just an observation, that occurred to me:

import Toybox.Lang;

(:featureX)
function getX(f as Boolean) as Number or String {
    return f ? 42 : "Hello";
}
(:noFeatureX)
function getX(f as Boolean) as Number {
    return f ? 42 : 24;
}

(:featureX)
function foo(f as Boolean) as String {
    var x = getX(f);
    if (x instanceof String) {
        return x;
    } else {
        return x.format("%05d");
    }
}
(:noFeatureX)
function foo(f as Boolean) as String {
    var x = getX(f);
    return (x as Number).format("%03d"); // "x" is "Number or String" in PMC, so calling "format" without cast causes PMC error, since "String" doesn't have "format"
}

PMC can already give irrelevant warnings (as is redundant from the monkeyc's POV), since it analyzes all source code at once, regardless of exclude annotations. But most likely this is a completely different case.

Anyway, thanks again for this project!