Big update to prettier-extension-monkeyc

flocsy🤠 said:

Both give the same warning in the editor

This was just an oversight. When a const doesn't have a declared type (as this one doesn't), I would deduce the type from the initializer, but only for certain simple expressions - and arrays were not included.

note that

const FOO as Number = 42;

has a declared type, while

const FOO = BAR as Number;

does not.

In any case, I've fixed it to always deduce the type from the initializer, and the problem is gone.

Btw, as with the other issues, this just makes the optimizer more conservative; it knows less about the type than it should, so may miss some optimization opportunities, but won't make incorrect decisions (at least not without some other bug).

Strangely, Garmin's type checker doesn't like your second declaration for some reason (but was happy with the first):

ERROR: fenix5xplus: source/C.mc:21,6: Cannot assign value '$.Toybox.Lang.Array<$.Toybox.Lang.Dictionary<$.Toybox.Lang.Boolean or $.Toybox.Lang.Char or $.Toybox.Lang.Double or $.Toybox.Lang.Float or $.Toybox.Lang.Long or $.Toybox.Lang.Number or $.Toybox.Lang.String,Null or $.Toybox.Application.PropertyKeyType or $.Toybox.Lang.Array<$.Toybox.Application.PropertyValueType> or $.Toybox.Lang.Dictionary<$.Toybox.Application.PropertyKeyType,$.Toybox.Application.PropertyValueType> or $.Toybox.WatchUi.BitmapResource>>' to member ':DEFAULT_USERS_PROPERTIES2'.

If I give it a declared type (matching the cast) then Garmin's type checker is happy.

flocsy🤠 said:

Unfortunately I don't see any change at all

I wasn't really expecting code size differences... out of 150 or so projects I compile, only 4 changed size.

I fixed one issue that could cause the type checker to lose track of the type of a member expression (a.b.c etc). And when it loses track, it assumes "Any" for the type, so again, it just makes it more conservative than it needs to be. But can also result in strange warnings. Obviously, you've found another way for this to happen...

The reason you see the strange warnings about seemingly unrelated classes, is that when its looking up remove in b, it tries to find every possible definition of remove that's compatible with the known type of b. If the "known" type of b is "Any" its going to find every method/constant/enum/class/module in the program (including the Toybox) thats named remove. It *has* to do that to ensure it's conservative about any optimizations it performs.

So the warnings are understandable, and not an indication of an error per se (or rather, the error is that it failed to determine the type of b when it really should have known it).

I'm still inclined to think this is a temporary glitch, where right after inlining the function, the inlined parts have no type information (because type propagation hasn't run on them yet).

What I try to do is skip over the just-inlined body, process the rest of the caller, and then re-run type propagation on the caller, and re-optimize it (possibly doing more inlining along the way). But in your case, I think something causes it to re-opimize the inlined code before type propagation has run again. So it generates the warnings, but then goes back and optimizes again, this time with the correct type information. But so far, I can't figure out how its happening.

ok, so even after your fix it's better to have the ugly:
const DEFAULT_USERS_PROPERTIES as PropertyDictionaryArray = [] as PropertyDictionaryArray;

> Strangely, Garmin's type checker doesn't like your second declaration for some reason (but was happy with the first):

I guess it's a bug that they only know it's an Array in certain context, but not sure.

would it help if I send you the logs from 2.0.42 in email?

Can you add a warning when I accidentally inline a "big" function more than once? At least for the huge ones that are obviously a mistake :) 

It happened again. I hade the (:inline) there for a while, and I think it was only used once when I added it, but then later I used it somewhere else, and didn't notice for a few months, and now that I start to get Out Of Memory Errors in ERA I noticed this, but I believe the optimizer could warn about this.

Or I can again drop in the old idea to have at least something like :inline_once.
Or maybe it would make more sense (even though it would not be backward compatible) to make :inline only work if it's only used once and add a new annotation: :inline_any or something like that that will inline multiple times (this would be used for things like 2 simple functions that either call App.setProperty or Properties.setValue for example)

flocsy🤠 said:

ok, so even after your fix it's better to have the ugly

For my optimizer? No. After the fix it will make no difference. The issue was that I simply punted on "complex" looking initializers because the code to get the type of a constant could be called many times; but I've fixed that by caching the result.

On the other hand, it looks like you *have* to add the declaration in the second case to get Garmin's type checker to accept it.

So the only reasons I see for declaring the type of a constant would be if Garmin requires it, or if you want the type to be wider than what would be deduced (eg Array<Number or String> when Array<Number> would be deduced). But I can't really think of a good reason for wanting to do that right now.

Btw, my type checker should deduce the type of a literal array. So eg

  var x as Array<Number> = [1,2,3];

works. But Garmin's type checker will reject that...

flocsy🤠 said:

Or I can again drop in the old idea to have at least something like :inline_once

I guess I need a better definition of what that means.

If foo is inline_once, and its only used once from bar, but bar is inline, do I inline foo into bar and then prevent bar from being inlined at all; or do I not inline foo into bar and then inline bar wherever it likes?

But also, currently all the inlining decisions are made on the fly. I find a call, see that inlining was requested, and try to inline it - which may fail for any number of reasons, and then continue on to the next call... 

In the "inline_once" world, I'd have to find every call to an inline_once function, determine which ones were only called once, determine which of those I could actually inline, then for each one that I did inline determine whether it changed the number of calls to any of the other inline_once functions (which it might, because an if-block might be eliminated after inlining), and continue.

Its doable, and I'm heading towards a more global optimization framework, so doing it may become a lot easier, but for now its not trivial.

Warning about inlining "big" functions more than once is probably more doable - I just need to add some kind of size calculation (which is going to be somewhat guesswork).