Big update to prettier-extension-monkeyc

Yes. Fixing this drops out of the "wishful thinking" optimization you described, which is already underway, and I should be ready to release that in the next few days. But this is pretty much an edge case. One more use of "-", and pre would have kicked in anyway...

flocsy said:

Again, not a bug, but probably you shouldn't rename variables for no apparent reason

So this is because copy propagation replaced zero by its value, and the pre replaced the zeros by a generated variable. I've thought about trying to preserve names when this happens, but its pretty low priority...

flocsy said:

it should've realize that HR_ZONES_MOCK

This one is a surprise. Across all the code I run through the optimizer I found several cases where I was previously failing to replace constants, that now work, but none where it previously worked and now fails. But you always manage to find the edge cases :-)

I'll see if I can repro.

flocsy said:

const HR_ZONES_MOCK = null as Array<Number>

So this one is because you're lying... and the optimizer should be warning you about it too:

WARNING: The type Null cannot be converted to Array<Number> because they have nothing in common

So basically you're saying that HR_ZONES_MOCK is an array, and arrays are never null.

But there is a bug of sorts. If you correct your declaration

const HR_ZONES_MOCK = null as Array<Number>?;

then the warning goes away, but it still doesn't get optimized. And that's because Array<Number>? might be null, and it might not; ie once you throw in a cast, I warn if the cast is incompatible with the value, but then ignore the value and trust the type. Turning trustDeclaredTypes off also doesn't help, because that just effectively turns all declared types into Any, so now its just "null as Any".

So... this is clearly suboptimal, but I'm not quite clear on the best solution. I think we want something like:

• With trustDeclaredTypes disabled, all casts should be ignored, rather than treated as a cast to "Any". Then regardless of the cast, turning off trustDeclaredTypes fixes the issue.
• With trustDeclaredTypes enabled, the code as written should fail to optimize, and when written as Array<Number>? it should optimize correctly.

But I'm assuming the reason for the cast is so that Garmin's type checker doesn't complain about setHeartRateZones(HR_ZONES_MOCK); 

And Array<Number>? won't keep it happy here (even though you explicitly check for null). But if you change it to 

if (MOCK && mHeartRateZones == null && HR_ZONES_MOCK != null)

then Garmin's type checker is happy.

I'm still thinking about this, because even something innocuous like

const MY_NUMBER = 4 as Number;

is currently causing similar issues (ie if (MY_NUMBER==4) doesn't get optimized); but that one is much easier to fix.

I'll check later why it's like that but if i remember correctly it's because in earlier version of the compiler i could have:

(:memory32K) const X = [];

And when I compiled for a device with 16K memory it accidentally didn't fail, and treated it as null.

But you are totally right that it should be with the question mark. Probably you can open a bug report about this one. I think Garmin take those relatively seriously.

I think it's better to fix this with the trustFeclaredTypes enabled. If it needs to be disabled because of this then it'll loose even more on lots of other places where types could be used to gain more

so if const X = 4 as Number; is easy to fix, then what about const Y = 4 as Number?; Isn't that the same as Array<Number>?; Why does the type bother you? There are 2 different things you do: 1 type check for warnings and colorizing in VSC, and optimizing. Now I can see that there's a difference between 4 and 4.0, but when it gets to nulls? Is null == null true? Yes. Is null as Number == null as Float? I think it should be yes, isn't it?

Ok, so there were similar cases, but even those I'm not sure if they're needed now. But here it's a special case where I really needed it. This is a mock, and it might be needed to be set to null, because in some devices I don't have enough memory to be able to acomodate the code (there MOCK=false) but in some devices I do have enough memory to mock some parts but not this, so there I set it to null. And all the other places (variables, function parameters) that get this value were already properly Nullable. This is really a bug in the compiler that let me do this, it should've failed with -l 3.

forums.garmin.com/.../bug-const-x-null-as-array-number-doesn-t-give-an-error-with-strict-typecheck

flocsy said:

so if const X = 4 as Number; is easy to fix, then what about const Y = 4 as Number?;

Well, the first one is easy to fix because the type completely agrees with the value. There's no reason not to just drop the cast altogether, and just turn it into "const X = 4".

The second one is a bit worse, but not much. I *could* just ignore the cast (since I know that Y isn't null). And in this case, thats probably fine - at least, I can't, off the top of my head, think of a case where (4 as Number?) would type check, but 4 would not.

But that's not true of your example. Your example shouldn't need a cast, because you check the value for null, so the call is unreachable, and shouldn't issue a warning. But Garmin's type checker errors out anyway.

So basically the problem I'm having is that I want to ignore casts that I know are false; but that might mean that Garmin's type checker falls over later. After all, you must have inserted the cast for a reason...