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]

  • Only an issue if you use Type Checking.  I'm got going to go back and add type checking to my 50 apps!

  • In the documentation there's an example of an "inline function": function foo(x as Number) as Number { return x * 3 - 1; }
    I tried to use this functionality, but it doesn't work for the following simple cases:

    import Toybox.Application;
    import Toybox.Lang;
    
    typedef Value as String or Number or Boolean or Null;
    
    (:no_api2, :memory16K)
    function setConfig(key as PropertyKeyType, val2 as PropertyValueType) as Void {
        AppBase.setProperty(key, val2);
    }
    (:api2, :memory32K)
    function setConfig(key as PropertyKeyType, val3 as PropertyValueType) as Void {
        Properties.setValue(key, val3);
    }
    
    (:no_api2, :memory16K)
    function getConfig(key as PropertyKeyType) as Value {
        return Application.AppBase.getProperty(key);
    }
    (:api2, :memory32K)
    function getConfig(key as PropertyKeyType) as Value {
        return Application.Properties.getValue(key);
    }
    
    function getConfigBool(key as PropertyKeyType, defaultValue as Boolean) as Boolean {
        var value = getConfig(key);
        if (value != null && !(value instanceof Lang.Boolean)) {
            if (value instanceof Lang.Number) {
                value = value != 0;
            } else if (value instanceof Lang.String) {
                value = value.length() > 0;
            }
        }
        if (value == null) {
            value = defaultValue;
        }
        return value;
    }
    
    function getConfigStr(key as PropertyKeyType, defaultValue as String?) as String? {
        var value = getConfig(key);
        if (value != null && !(value instanceof Lang.String)) {
            // TODO it can also be Float or Number that don't have toString
            value = value.toString();
        }
        if (value == null) {
            value = defaultValue;
        }
        return value;
    }
    
    function getConfigNumber(key as PropertyKeyType, defaultValue as Number) as Number {
        var value = getConfig(key);
        if (value != null && !(value instanceof Lang.Number)) {
            if (value instanceof String) {
                value = value.toNumber();
            } else if (value instanceof Boolean) {
                value = value ? 1 : 0;
            }
        }
        if (value == null) {
            value = defaultValue;
        }
        return value;
    }
    

    I included the actual file, 'cause I guess that it matters that these are not in a module or a class, but I would expect getConfig(key) to be replaced exactly as foo was replaced in the documentation's example.

    I also tried to put the whole function to 1 line but it didn't fix it: 

    function getConfig(key as PropertyKeyType) as Value {return Application.AppBase.getProperty(key);}
  • In the documentation there's an example of an "inline function"

    Sorry, misunderstanding there. Its not really an inliner, its a constant folder. If it can resolve the value of the called function to a constant, it will replace the call with that constant.

    So in the example, foo(1), and foo(42) will be constant folded, but foo(x) won't.

    So at present the optimizer won't even try to optimize your function. The main issue is that I'm aiming to reduce code size, and its very hard to do that when inlining.

    eg, even in your case, 'Application.AppBase.getProperty(key)' generates much more code than getConfig(key) (since x.y.z generates code for each component), to the extent that three calls to getConfig, plus the definition of getConfig are probably smaller than eliminating getConfig and calling Application.AppBase.getProperty three times directly. Its not actually doing that calculation, and being smart about whether to inline or not - so its *possible* that manually inlining would reduce the code size.

    cause I guess that it matters that these are not in a module or a class

    Its not relevant in this case, but just to be clear about the problems with resolving calls to methods:

    Calling functions in modules isn't an issue (whether they're called explicitly with the module name Foo.bar.baz(), or implicitly baz() (when the caller is also inside Foo.bar).

    Calling methods from other methods also isn't an issue if they're called as self.bar(), or implicitly as bar(). The problem comes when you have something like

      var x = new Foo();

      ...

      x.bar(); // <-- the optimizer doesn't know what type x is here, so it can't resolve the call

  • ok, so here's a feature idea:

    Let's have a special annotation: (:inline), if it's used then you can replace the code when this function is called with the function's body.


    update: it might be a little tricky because of imports:
    A.mc:

    function a() {
      setConfig("foo", "bar");
    }
    

    Config.mc:

    import Toybox.Application;
    (:no_api2, :inline)
    function setConfig(key as PropertyKeyType, val3 as PropertyValueType) as Void {
      Properties.setValue(key, val3);
    }
    
    (:api2, :inline)
    function setConfig(key as PropertyKeyType, val2 as PropertyValueType) as Void {
      AppBase.setProperty(key, val2);
    }
    

    If you inline it then A.mc will become:

    function a() {
      Properties.setValue("foo", "bar");
    }

    but the import of Application is not there... Maybe an import of Toybox.Application.Properties can be added automatically? Not sure, and it's not trivial, because it looks like Toybox.Application.Properties is a module, but Toybox.Application.AppBase is not, it's a class.. so it's not "symetrical":

  • Thats a nice idea. I'll give it a go. I'm also thinking of adding build options such as "Inline simple wrappers" - ie functions such as yours that do nothing but call another function.

    I don't think imports are a problem as such; as we've established, once you "import Toybox.Application" in one file, it's as if you did it in all files.

    But context is definitely an issue. eg

    module Foo {
      var x;
      (:inline)
      function getX() { return x; }
    }
    function bar() {
      return getX();
    }

    Now if you naively inline getX() into bar() it becomes "return x;". But that just means checking each unqualified identifier to make sure it resolves to the same thing in both contexts, and if not, qualify it as necessary to make sure it does refer to the correct thing. In this case "return Foo.x;"

    Also, it can be hard to inline some functions in some places. eg

    (:inline)
    function foo(x) {
      var y = 1;
      while (x > 0) {
        y *= x;
        x--;
      }
      return y;
    }
    function bar(x, y, z) {
      return f(z) + foo(x) + g(y);
    }

    Now the only way to inline foo is to break up the expression it occurs in, evaluate f(z) into a temporary, then inline foo (renaming y in the process) then "return t + foo_y + g(y)". I probably won't be supporting that initially...

  • As a side note, does this actually work for you?

    import Toybox.Application;
    
    (:api2, :inline)
    function setConfig(key as PropertyKeyType, val2 as PropertyValueType) as Void {
      AppBase.setProperty(key, val2);
    }

    For me, it compiles, even with strict type checking, but at runtime AppBase.setProperty fails with an "unable to find AppBase" error (which is what I'd expect).

    Changing it to "Application.AppBase.setProperty()" does seem to work, which makes sense according to the lookup rules, but I was still surprised it worked at runtime, since getProperty is a non-static method on AppBase, and the docs tell you to do "Application.getApp().setProperty()". Also, as far as I can tell, the App's properties are stored on a protected, non-static member called "mProperties" - so how would it get at them without an object? Maybe it automatically calls getApp() for you if it's called statically... [edit: actually, although mProperties is declared in api.mir, and has the right type to be the application properties, it seems to always be null. And its not mentioned in the docs]

  • I don't think you need to reinvent a compiler.. Let's make it easy. There's no much sense in inlining complex things (besides maybe 1 thing: readability). I thik that the 1st example: Foo.getx() is a case that might be considered a bit later, but wouldn't start with that. Think about it as the preprocessor that takes care of the inline functions in C. In other words: let only consider global functions (as in my example). Even then you can have problems with existing variable names in different levels...:

    (:inline)
    function g(a) {

        var b = a +1;

        return a*b;

    }

    var a = 5;

    function a() {

        var c = g(4*a); // we'll need something like: var c = (4*a)*((4*a)+1);

    }

    function b() {

      var a = 8;

      var f = g(a);

    }

    function c() {

      var b = 6;

      var f = g(a); // => we mean something like: f = a*(a+1); but if you just "inline" it then you'll have two "var b"-s :(

    }

    Also see my example, you'll need to do the inlining after the excludeAnnotations are processed:

    (:no_api2, :memory16K, :inline)
    function setConfig(key as PropertyKeyType, val2 as PropertyValueType) as Void {
      Application.AppBase.setProperty(key, val2);
    }
    (:api2, :memory32K, :inline)
    function setConfig(key as PropertyKeyType, val3 as PropertyValueType) as Void {
      Application.Properties.setValue(key, val3);
    }

    Only one of these should be kept (because of the other excludeAnnotations and my monkey.jungle) and only then process :inline.

    BTW you're right, it doesn't work with AppBase.setProperty, I got confused about the other thing I was testing at the same time:

    import Toybox.Application;
    // import Toybox.Application.AppBase; // this would give an error, because AppBase is a class and not a module
    import Toybox.Application.Properties; // this works

    AppBase.setProperty(key, val2); // this will not work even though Toybox.Application is imported :(
    Properties.setValue(key, val3); // this works if Toybox.Application.Properties is imported but not if only Toybox.Application is. Very confusing (not your fault, probably Garmin's)
  • BTW i checked the gain/price of inlining a small function and I was surprised: not only you were wrong, you were very wrong :) 

    // no inline code:5725, data:3148
    function getApp() as MyApp {
     return Application.getApp() as MyApp;
    }

    function getConfig(key as PropertyKeyType) as Value {
     return getApp().getProperty(key);
    }

    // inline code:5700, data:3130
    function getConfig(key as PropertyKeyType) as Value {
      return Application.getApp().getProperty(key);
    }

    So not only your hunch that inlining will increase code was not right, but it actually decreased significantly by 25 bytes in code and 18 bytes in data. This only works because I made sure that getApp() was only used in 1 place. (When it was used in another place, then inlining it in 1 place only indeed increased code size by 4 bytes.) But I agree this is not trivial, but I don't think that your code is supposed to deal with this calculation. This should be done by the developer (as I did here). What you could do is give "compilation" (optimization) warning if your code thinks that it's actually making the code bigger :)
  • But that was exactly my point. My guess was specifically that calling the wrapper function 3 times, would result in less code than inlining it three times (and deleting the wrapper function). And that was based on calls to the wrapper function looking like "foo(x)", while calls to the wrapped function looked like "a.b.bar(x)". It's the "a.b." that increases the code size.

    In this case, calling the wrapper only saves one ".", so the savings are less per call site... and you're only calling it once! And yes, obviously, its better to inline a function thats only called once [edit: *] if you can also delete the callee. But that requires global analysis that I'm not currently doing. eg when constant folding functions, I just do it one call site at a time, because it's *always* better to do it if it's possible (both from a code size *and* a performance point of view). At the end, *if* I can prove I've eliminated all calls to the function, I also delete it.

    [*] well, it *ought* to be better. Ive seen cases in c/c++ where the code ends up worse because the compiler's heuristics break down when a function gets too big, or has too many locals, or whatever.

  • I don't think you need to reinvent a compiler.. Let's make it easy.

    Followed by a long explanation of why we really *have* to reinvent a compiler :-)

    But the point is I've already written it. I've gathered all the information about the program that I need to determine when its safe to inline; when I'd need to add qualifiers to identifiers to make sure they had the same meaning at the new site; even when to rename locals to avoid conflicts. Its just a case of putting it together to do the inlining. My point was more that in a lot of cases I'll most likely just punt, because the fixups required at the callsite are hard. And most of those cases are going to be when the body of the function can't be expressed as a single expression.

    Also see my example, you'll need to do the inlining after the excludeAnnotations are processed:

    I already have to do that in order to constant fold functions...