Big update to prettier-extension-monkeyc

Ok, so 2.0.19 is out.

• Fixes the issue of needing to add imports when inlining from one file to another, and also resolves a whole slew of issues with symbol lookup.
• Fixes renaming of local variables, which was broken a couple of releases ago, and adds some integration tests to make sure I don't break it again.
• Adds diagnostics for missing symbols, so typo'd variable names will produce warnings as you type. I've tried to match the runtime, rather than the type checker's idea of what a missing symbol is, because the type checker reports errors for things that work at runtime, and fails to produce errors for things that will crash at runtime.
• Reverses the sense of (:inline_foo). Now it inlines unless foo is in excludeAnnotations.
• Fixes a couple of compatibility issues with 4.1.4_Compiler2Beta

As always, if it misreports something (ie if it says a symbol doesn't exist, but it works at runtime) bug reports (especially with repro steps) would be appreciated.

I found a bug (using 4.1.4 (old compiler)):

After a huge refactoring I ran the app in the sim with optimized code. It compiled, and then failed with runtime error, that it doesn't find MyClass.width symbol.
When I ran it without the optimizer then it compiled, and didn't fail (though the output was strange). I looked at the optimized code where it failed, and I realized that I left a variable in the class that was not supposed to be there.

so my original code looks something like:

class BaseClass {
    private var width as Number = 0; // this is the leftover variable
}

class MyClass extends BaseClass {
    function foo() {
        bar(width); // this is where the optimized app failed during runtime
    }
}

The optimized code has this line where it failed:
var x = bar(BaseClass.width);

Can you give me more details? The optimizer will only change bar(width) to bar(BaseClass.width) while inlining, and even then, only if there were a conflict with a local variable. So there's some context missing here.

I tried the following, and found a slightly different bug. While inlining localConflict, it *failed* to change h to Base.h, and so bar ended up as "var h = 42; return h + h;". That only happened with functions that could be inlined in expression context though (ie with more complex functions, it correctly renamed h to Base.h). I've got a fix, but I'd like to figure out how your example happened before posting another update.

module Inheritance {
    class Base {
        hidden var h as Number = 1;
        private var p as Number = 1;
    }
    class Child extends Base {
        (:inline)
        function localConflict() as Number {
            return h;
        }
        (:inline,:typecheck(false))
        function localPrivate() as Number {
            return p;
        }
        function bar() as Number {
            var h = 42;
            /* @match "Base.h + h" */
            return localConflict() + h;
        }
        function baz() as Number {
            var p = 42;
            /* @match "Base.p + p" */
            return localPrivate() + p;
        }
    }
    (:test)
    function inherit(logger as Logger) as Boolean {
        var x = new Child();
        return x.bar() == 43;
    }
    (:test)
    function crash(logger as Logger) as Boolean {
        var x = new Child();
        return x.baz() == 43;
    }
}

When inlining localPrivate (after my fix), the resulting code does crash at runtime. But so does the original code. And so does *your* example as written. So there must be something else going on...

I'll note that my optimizer currently doesn't take any notice of hidden/private/protected. I will probably add more support for that (although the lookup rules, even without that are complex enough, and they seem to be changing with the new compiler).

My assumption is that if the original code works, so will the transformed code. Otoh, it's certainly possible that the original code fails, and the optimized code passes. eg if you had put "private const width = 42", the optimizer would have substituted 42 instead of Base.width, and code that should have failed at runtime will suddenly start "working".

That is problematic, but not as bad as breaking working code...

import Toybox.Lang;
import Toybox.Graphics;
import Toybox.WatchUi;

class MissingSymbol extends WatchUi.DataField {

    // this was left here by accident
    private var width as Number = 0;

    // corrected code:
    // no width :)

    public function initialize() {
    }

    public function onUpdate(dc as Graphics.Dc) {
        View.onUpdate(dc);

        var width = dc.getWidth();

        // forgot to pass width:
        drawHrZoneGauge(dc);

        // this is the coddected code:
        // drawHrZoneGauge(dc, width);
    }

    (:black_and_white, :no_gauge, :inline) hidden function drawHrZoneGauge() as Void {}
    (:color, :gauge, :inline)
    hidden function drawHrZoneGauge(dc as Graphics.Dc) as Void {
        // the unoptimized code didn't fail here because of the accidentally kept self.width
        dc.fillRectangle(0, 0, width, 10);

        // strangely when I tried to cut down the example and I had here foo(width) then the problem doesn't happen in the optimized code.
        // so it must be to do somehow with using the passed dc when inlined...
        // foo(width);
    }

    // this is the coddected code:

    // (:black_and_white, :no_gauge, :inline) hidden function drawHrZoneGauge(width as Number) as Void {}
    // (:color, :gauge, :inline)
    // hidden function drawHrZoneGauge(dc as Graphics.Dc, width as Number) as Void {
    //     dc.fillRectangle(0, 0y, width, 10);
    // }

    hidden function foo(w as Number) as Void {
    }
}

To test it add:
var missingSymbol as MissingSymbol = new MissingSymbol();
missingSymbol.onUpdate(dc);

You might be able to reproduce it by replacing dc with something else, but I had this as a datafield....

flocsy🤠 said:

You might be able to reproduce it by replacing dc with something else, but I had this as a datafield

Thanks, that was enough to show me what was going on, and make a small repro:

    class Base {
        private var p as Number = 1;
        (:inline)
        function badQualifier() {
            var x = p;
            return x;
        }
        function foo() {
            var p;
            p = badQualifier();
            return p + 24;
        }
    }

before my fix, that got translated to:

    class Base {
        private var p as Number = 1;
        function foo() {
            var p;
            {
                var x = Base.p;
                p = x;
            }
            return p + 24;
        }
    }

It turns out that while that "Base.p" works for public and protected variables, it doesn't work for privates, so my fix is to first try "self.p" to see if that resolves. That should always work for privates (unless the program was broken to start with), and if it fails, we just fall back to the original algorithm.

So now we get:

    class Base {
        private var p as Number = 1;
        function foo() {
            var p;
            {
                var x = self.p;
                p = x;
            }
            return p + 24;
        }
    }

This broke some of my tests, because (with a similar setup) I was expecting a global variable to be translated from g to $.g, after inlining, but now it ends up as self.g. So I started to look into fixing *that*, but then noticed that self.g actually generates less code than $.g (by 4 bytes). In fact, prefixing an identifier by "self." (when the identifier isn't a local) doesn't seem to change the code size at all.

So good news all round :-)

I'm just waiting for tests, and should push a fix later this afternoon.

can you post an example mc/project and what it looks like before and after?  I'm not really sure how this would be used and what the result would be,  As there only seems to be one user, it seems most folks are as confused as I am!

But jim_m_58, since when are you using the prettier extension? And for inlining???? I thought all your apps are manually optimized already

markw65 I removed all the $... from my code because it adds lot of extra code. As for self.g in case g is not a member: It sounds like a bug in the compiler, don't you think? I mean if you have a global variable g, then you can either write $.g or just g (if there's no local g), but self.g should not work, should it? If it works IMHO it's a bug, though I can probably explain to myself how that happens: looks like self.g tells the compiler to not use the local variable called g, and instead of starting to look for a variable called g in a waterfall starting from the local scope, start one scope higher. Though to me it sounds strange that it finds a local g if there's no g in the class. I think then the compiler should fail in compile time.
There's one thing I don't understand: why and when do you replace "g" with "self.g" (or until now Base.g)? In most cases you can keep it as "g", don't you? And in some cases it won't help, and you'll need to rename the variable.

I'm not using it because I have no clue what it would do for me.  That's why I asked for an example.  Does anyone other than you use it?

The whole inline thing makes me ask, why?    At this level it should pretty up the code, and do things like correct tabs, split extra long lines, etc.  And it needs to do something if monkey types are not involved, but it seems that is required!

Can you explain?

As for self.g in case g is not a member: It sounds like a bug in the compiler, don't you think

I'm pretty sure its not a bug. Its just that the scoping and lookup rules for monkeyc are somewhat unexpected. I did a long write up of how I believe its supposed to work recently (as part of a bug report). But the behavior is surprising.

For one thing, *everything* has a self. You can see it in the arguments. If you're in a module, self is a reference to the module.

Other interesting things are that module.foo means lookup foo in "module" and if its not found there, look it up in the next module out. To be more concrete, since Properties is defined in Application, which is defined in Toybox, I can say things like Toybox.Application.Properties.WatchUi.Lang.Number.

So what happens, is that lookups go as expected as far as Properties, then WatchUi isn't found. The "inheritance" chain from Properties goes to Application, then Toybox, so it looks for it in Application, where it also isn't found, and then looks for it in Toybox, where it *is* found. Then it looks up Lang in WatchUi, but again, its not there, so it looks in Toybox, and finds it, and then it looks up Number in Lang, and finds that...

The same thing happens with self. If Im in module Foo inside Bar inside Baz, and I say self.X, it will look for X in Foo, then Bar, then Baz.

If I'm in a *class* and say self, it looks up the symbol in the class, then the super class, then ... until it hits Object. If its *still* not found, it looks it up in all the modules that contain the class, and if that fails it looks it up in all the modules that contain the Base class etc, until it gets to Object, at which point it looks it up in Lang, then Toybox (and its in that precise order that it looks for things). Which is why you can say "x instance of Number" in a non-static class method (without importing anything), but you have to say "x instanceof Lang.Number" outside of a class *even if you've imported Toybox.Lang*.