Big update to prettier-extension-monkeyc

I've posted about prettier-extension-monkeyc before, but I've added a bunch of new features that developers will probably like (well, I've been missing them, so maybe you have too).

The new features it implements for VSCode include:

  • Goto Definition. Point at a symbol, Ctrl/Cmd click, and it will take you to the definition. Or F12
  • Goto References. Right click on a symbol and select "Goto References". It will show you all the references. Or Shift-F12
  • Peek Definition/Peek References. Same as above, but in a popup window so you don't lose your place in the original document.
  • Rename Symbol. Right click on a local, function, class or module name, and select "Rename Symbol". It will rename all the references. It doesn't yet work for class members/methods.
  • Goto Symbol. Type Ctrl/Cmd-Shift-O and pick a symbol from the drop down (which has a hierarchical view of all symbols in the current file). This also appears as an outline across the top of the file.
  • Open Symbol By Name. Type Ctrl/Cmd-T, then start typing letters from a symbol name. A drop down will be populated with all matching symbols from anywhere in your project.

Older features include a prettier based formatter for monkeyc, and a monkeyc optimizer that will build/run/export an optimized version of your project.

[edit: My last couple of replies seem to have just disappeared, and the whole conversation seems to be in a jumbled order, so tldr: there's a new test-release at https://github.com/markw65/prettier-extension-monkeyc/releases/tag/v2.0.9 which seems to work for me on linux. I'll do more verification tomorrow, and push a proper update to the vscode store once I'm sure everything is working]

  • OK, I was able to "fix it": I realized that I had the following code:

    if (maxHr != null && zero < (maxHr as Number)) {
        var z0 = (MAX_HR_2_Z1_MIN_PERCENT * maxHr).toNumber();
        var z1 = (MAX_HR_2_Z1_MAX_PERCENT * maxHr).toNumber();
        var z2 = (MAX_HR_2_Z2_MAX_PERCENT * maxHr).toNumber();
        var z3 = (MAX_HR_2_Z3_MAX_PERCENT * maxHr).toNumber();
        var z4 = (MAX_HR_2_Z4_MAX_PERCENT * maxHr).toNumber();
        var z5 = maxHr;
        heartRateZones = [z0 - one, z1, z2, z3, z4, z5] as Array<Number>;
    }
    

    and I just replaced it with:

    if (maxHr != null && zero < (maxHr as Number)) {
        heartRateZones = [
            (MAX_HR_2_Z1_MIN_PERCENT * maxHr).toNumber() - one,
            (MAX_HR_2_Z1_MAX_PERCENT * maxHr).toNumber(),
            (MAX_HR_2_Z2_MAX_PERCENT * maxHr).toNumber(),
            (MAX_HR_2_Z3_MAX_PERCENT * maxHr).toNumber(),
            (MAX_HR_2_Z4_MAX_PERCENT * maxHr).toNumber(),
            maxHr] as Array<Number>;
    }
    

    Which makes me think that in this and similar cases the optimizer could give a warning that these variables are unnecessary and could be inlined by the developer (in this case it saves code, and stack). Maybe the optimizer could just do this replace automatically as well in some cases.

  • you seem to remove public functions that are not called by my code.

    Yes, as long as there's no way to call them.

    There is a small hole in this, which is that I do rely on inheritance to tell me what the watch might call. eg if you write a class that extends DataField, I know onUpdate might be called, because DataField defines onUpdate. But it turns out that (at least some of) the inheritance described in the monkeyc documentation is not required. eg as long as it implements all the necessary methods, you can use any class as a Menu2InputDelegate; it doesn't actually have to extend Menu2InputDelegate. And if you do that, I'm going to remove your methods, and it's going to crash (this came up in a discussion several months ago if you remember). Note that Garmin's type checker will yell at you if you do this anyway, so you have to turn that off (or explicitly cast things) to do this in the first place.

    So I added a (:keep) attribute. If you really must define Ui classes that don't extend the appropriate base class, then you'll have to manually mark the methods you want to keep. 

    If barrels can be distributed as bytecode

    They aren't they're distributed as zipped source code. If you try to run my optimizer on a barrel project it will tell you to run it on the project that uses the barrel instead - for pretty much this reason.

    However the optimizer enters some loop and probably dies with a stackoverflow

    Oh. I'll take a look. But you're saying this only happens with a program that was broken to start with?

  • I don't see why this couldn't be

    Yes, obviously it could. We could have a rule that said that any variable that's used only once, and used immediately after its definition (ie no calls, no assignments, no update-expressions in between), then we can just replace the use of the variable by its initializer. That works perfectly for this example, but fails for the z0-z5 example you posted later.

    I do plan to make improvements to this, but I want something thats at least good enough to the the z0 transformation. And I now have the data-flow framework I need to do it.

  • Anyway, maybe you should use instead of pre_foo just pre1,...preN and in every place where it's used add a comment with it's "current" meaning (just to make it easier to read it)

    The plan is to make this generic. Rather than fixing pre to not create extra variables, have a final step that merges non-conflicting variables. That way it can reduce the number of variables even when it wasn't the optimizer that created them...

  • Yes, only with the broken code 

  • v2.0.45 is out.

    • Fixes an incorrect type checker warning
    • Adds a CompletionItemProvider. This provides completion suggestions as you type. Garmin's extension already does that, but it doesn't know about local variables, and it mostly doesn't know about context, so it suggests every name it knows about, rather than just the ones that apply at this point in the code. Unfortunately, there's no way to turn Garmin's off, so its suggestions get mixed in with mine. For the most part though, my suggestions should generally appear at or near the top.
    • Adds a SignatureHelpProvider. This is triggered when you type the opening '(' for a call, or a ",", and it shows the arguments to the current function, and which one you're currently entering.
  • However the optimizer enters some loop and probably dies with a stackoverflow

    Doesn't repro for me with either 2.0.44 or the just released 2.0.45. Also, I assume you meant no_foo isn't set as an exclude annotation, and foo is set as an exclude annotation; since the other way around seems like it should compile. In any case, I tried it both ways, and didn't get an error.

  • In the last couple of upgrades of Prettier Monkey C I did not see a notification that there is a newer version (I think it was an Update button). This might be because there wasn't enough time between the release and when I checked. However what's even stranger is that after I click cogwheel > Install Another Version > 2.0.45 (or whatever the latest at this time), then I see Installing displayed for a second, and that's it. It doesn't display anything else, and I'm pretty sure in the past there was some "Reload required" that I had to click on. In fact if I restart VSC then it does display: Installing (for more than 5 seconds) and then Reload Required. So I'm not sure if this is related to your code or to VSC (1.74.3 (Universal) on Mac) but this worked better in the past than now.

    Strange, I even moved the test to another small project, where there's almost no code next to it:

    //monkey.jungle:
    
    project.manifest = manifest.xml
    project.typecheck = 3
    project.optimization = 2z
    
    base.sourcePath = source
    base.excludeAnnotations = memory32K
    
    
    // Test.mc:
    
    import Toybox.Activity;
    import Toybox.Lang;
    import Toybox.WatchUi;
    
    class Test extends WatchUi.DataField {
        (:memory16K) const FOO = false;
        (:memory32K) const FOO = true;
    
        (:memory32K)
        var mFoo as Number = 1;
    
        public function compute(info as Activity.Info) as Void {
            x();
        }
    
        (:inline)
        function x() as Void {
            var foo;
            if (FOO) {foo = mFoo;}
            else {foo = 0;}
        }
    }
    

    Settings (2.0.45): ERROR, ERROR, ERROR, DEFAULT, YES, "", "", "", bin/optimized, check, check, check, Strict, check, uncheck, check, uncheck

  • Thanks - I found the infinite recursion bug with that test case.

    Simple fix. I'll ship it with the next release.

  • Stack Overflow again

    Ok, so between dead store elimination, and type propagation, I had all the information I needed to compute local variable conflicts. So adding a pass to merge local variables that don't conflict was fairly easy.

    The one snag is that it makes such a huge difference to the generated code that a lot of my tests are broken by it (eg an inliner test that says "expect to see this call replaced by something that looks like...).

    Anyway, I just need to decide whether to disable it for those tests, or to update the pattern matchers to recognize the transformed code.

    But meanwhile, here's the results on a random piece of code I found on the internet.

    Before: one (unused) parameter, and 3 locals

        function compute(info as Activity.Info) as Numeric or Duration or String or Null {
            var result;
            if (hasBodyBattery) {
                var iterator = Toybox.SensorHistory.getBodyBatteryHistory({:period => 1, :order => SensorHistory.ORDER_NEWEST_FIRST});
                var sample = iterator.next();
                if (sample != null && (sample as SensorSample).data != null) {
                    result = ((sample as SensorSample).data as Number).toNumber();
                } else {
                    result = "--";
                }
            } else {
                result = strNotSupported;
            }
            return result;
        }
    

    After: one heavily reused parameter and no locals

        function compute(
            info as Activity.Info
        ) as Numeric or Duration or String or Null {
            if (hasBodyBattery) {
                info /*>iterator<*/ = Toybox.SensorHistory.getBodyBatteryHistory({
                    :period => 1,
                    :order => 0 as Toybox.SensorHistory.Order,
                });
                info /*>sample<*/ = info /*>iterator<*/.next();
                if (
                    info /*>sample<*/ != null &&
                    (info /*>sample<*/ as SensorSample).data != null
                ) {
                    info /*>result<*/ = (
                        (info /*>sample<*/ as SensorSample).data as Number
                    ).toNumber();
                } else {
                    info /*>result<*/ = "--";
                }
            } else {
                info /*>result<*/ = strNotSupported;
            }
            return info /*>result<*/;
        }
    

    By getting rid of the locals, this saves 3 stack slots, but also shaves 2 bytes off the code size because the "incsp" instruction at the head of the function is eliminated.

    This doesn't address the single-use variable issue, but I'll probably go after that next.