Big update to prettier-extension-monkeyc

SirLancelot said:

If I'm only excluding :highmem from 4 devices, I really don't want to define :lowmem and exclude it from the other 50

I think you need a pair of exclusions... but its generally not so bad. you can do

base.excludeAnnotations = lowmem
device1.excludeAnnotations = highmem
device2.excludeAnnotations = highmem
device3.excludeAnnotations = highmem
device4.excludeAnnotations = highmem

Its only a problem if base (or some other category, such as round-240x240, has other excludeAnnotations and you want those other annotations to be inherited by the 4 devices. But in that case you can do

commonExclusions = foo;bar;baz
base.excludeAnnotations = $(commonExclusions);lowmem
device1.excludeAnnotations = $(commonExclusions);highmem
device2.excludeAnnotations = $(commonExclusions);highmem
device3.excludeAnnotations = $(commonExclusions);highmem
device4.excludeAnnotations = $(commonExclusions);highmem

Ah, I didn't realize you could have base.excludeAnnotations.  Thanks so much, this should really help!

v2.0.26 is out

• It fixes some issues with the sizeBasedPRE pass. I had disabled parts of it while debugging an edge case, and forgot to re-enable it before release. For my projects it saves almost twice as much now.
• It adds an unused variable cleanup pass, which kills variables that are assigned to, but not otherwise used.
• Improves side effect analysis, enabling better inlining, and better size-based-pre

Is it possible to have the extension ignore certain parts of my code? For example, I have a menu that doubles as a menu2 for newer devices, and is just a regular menu for all other devices. The code goes like this.

function onSelect(item) {
	menu(item);
}

function onMenuItem(item) {
	menu(item);
}

function menu(item) {
	var id;
	if(hasMenu2){ 
		id = item.getId();
	} else {
		id = item;
	}

	if (id == :item_1) {  // EXIT
        exit();
	} else if (id == :item_2) {
	
	}
}

However, the extension removes either onSelect or onMenuItem, and thus my whole code crashes on one of the sets of devices. It would be great if we could disable the extension with something like:

// prettier: disable

more code

// prettier: enable

Thanks!

markw65 it looks like this is what I wrote you some time ago that you shouldn't remove public functions?

Why don't you add some excludeAnnotations, so only the needed function is kept for the device?

(:newer)
function onSelect(item) {
  menu(item);
}

(:older)
function onMenuItem(item) {
  menu(item);
}

SirLancelot said:

However, the extension removes either onSelect or onMenuItem

Can you explain a bit more about how your code is organized? Presumably onSelect and onMenuItem are defined inside a class, and you seem to be saying that the class sometimes extends MenuInputDelegate, and sometimes extends Menu2InputDelegate. How exactly do you achieve that?

Also, you say that the optimizer either removes onMenuItem, or it removes onSelect. That sounds like exactly what it *should* be doing; unless its removing the *wrong* one.

SirLancelot said:

It would be great if we could disable the extension with something like

Thats not really doable. I could add a feature where adding say (:keep) to a function would prevent the optimizer from removing it; but that would only be needed if the optimizer has a bug (and apparently it does).

One thing the optimizer looks for though is symbols that might possibly be passed to Lang.Method (which currently pretty much means it sees a mention of the symbol somewhere), so one thing you could try is adding

var keepSelect = :onSelect;

var keepOnMenuItem = :onMenuItem;

Unfortunately, the last update to the optimizer will kill unused local variables... so those would have to be globals, which will therefore add a small amount of code to your project.

But if you can share exactly what you're doing (or produce a cut down version of what you're doing that exhibits the problem) I'm pretty sure I can fix it.

flocsy🤠 said:

it looks like this is what I wrote you some time ago that you shouldn't remove public functions

Well, no... I shouldn't remove possibly-called-functions; so clearly this is a bug. But I'm really curious about how he can have a single class definition that extends different base classes in different bulids...

flocsy🤠 said:

Why don't you add some excludeAnnotations, so only the needed function is kept for the device?

This is what's confusing me... only one of the functions is used on any given device, and he's saying that the optimizer removes one or the other (and from the way he wrote that, it removes one on some devices, and the other on other devices). Which sounds like exactly what you would want...

SirLancelot said:

However, the extension removes either onSelect or onMenuItem, and thus my whole code crashes on one of the sets of devices

Ok, I think I've figured out what's going on.

The documentation says that you must extend a Menu2InputDelegate to handle a Menu2, and a MenuInputDelegate to handle a Menu.

If you do that, then my optimizer knows which methods can be "magically" called by the system, and won't remove them in any circumstances.

But I've just tried extending a MenuInputDelegate, and using it for a Menu2, and as long as I define all the methods that might be called, it works. In fact, I don't even need to extend MenuInputDelegate; I can just define a class that doesn't extend anything, and it works fine. But if I do either of those, I can then repro the issue where my optimizer removes functions that are going to be called by the system.

Note that if you don't extend Menu2InputDelegate, then you have to implement onBack and onWrap, or your app will crash when those methods are invoked (and if you use CustomMenuItems, you also need onDone, onFooter and onTitle).

So on the one hand, the best solution seems to be to use the correct delegate for the menu kind you're using, as indicated in the documentation. But I also noticed that by not extending Menu2InputDelegate, and adding an onWrap() that just returns true, I can save 28 bytes of code, and 18 bytes of data. So although its not explicitly documented as safe, it is tempting to do it...

So I see two solutions. One is to add all the methods used by any of the delegate classes to a list of methods that should never be removed. That works for this case, but I'm sure there are other Toybox classes that the docs say "you must extend this class" when it really means "you must provide a class that matches this interface". So I probably do need to add support for (:keep).

[edit: I was inadvertently testing this with the type checker turned off. Once I turned it on again, I had to do a lot of explicit casting to make it compile if I didn't extend any of the Delegate classes. But as long as I extended *a* delegate class, the type checker was completely happy. So there's at least a hint that you're supposed to extend a delegate class, rather than just matching its interface]