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]

  • You didn't forget to import anything. This code:

    import "Toybox.WatchUi";
    
    function getMenu() as Menu2 {
        return new Menu2({:title=>"My Menu2"});
    }

    compiles and type checks with compiler1 and compiler2, but if you compile it with compiler1, and try to run it, it will crash at the "new Menu2". If you want it to work with compiler1, or with -O0, you need to say "new WatchUi.Menu2(...)". There is nothing you can import that will fix it.

    I'm finding that a number of diagnostics seem to need more explanation, so I'm hoping I can make them link to a wiki page - but I've not really looked into that yet. For this specific warning, I can probably include the module that you need to add in order to fix the issue (ie WatchUi in this case) to make it a bit clearer.

  • DataFieldAlert will only be found when compiled with compiler2 at -O1 or above

    Just another note on this. I've been playing with 4.1.2 beta 2 today, and it seems that that version uses compiler2 regardless of the optimization level. So this will be less of an issue in future.

  • Version 2.0.41 is out.

    This extends the type analysis, and adds various optimizations that were dependent on it.

    In addition to using the inferred types for opimization, this version also adds a lot of type checker warnings, and runs the analysis in the background as you type. It doesn't yet cover everything that Garmin's type checker does (in particular, it doesn't warn about the types of arguments to binary or unary operators), but it does cover a number of things that Garmin's does not (eg inferring types for Array and Dictionary literals).

  • wow. big chaos... I set almost everything to the strictest in the settings:
    ERROR, ERROR, ERROR, DEFAULT, YES, "", "", "", bin/optimized, checked, checked, checked, Strict, checked, checked

    And I get errors that don't make sense in any level. i.e: wrong number of arguments to functions.

    And it looks like it's caused by a wrong "guess" by the optimizer:

    //// B.mc
    import Toybox.Lang;
    import Toybox.Math;
    
    class B {
      public function add(d as Number?) as Void {
      	...
      } 
    
      public function remove(n as Number) as Void {
        var s = mArr.size();
        var remove = n <= s ? n : s;
        for (var i = 0; i < remove; i++) {
          add(null);
        }
      }
    }
    
    //// C.mc
    import Toybox.Ant;
    import Toybox.Lang;
    
    class C {
      private var mB as B = new B();
    
      public function c() {
        d();
      }
      
      (:inline) hidden function d() {
        var b = self.mB;
        var beats = ...;// (Number);
        if (beats > 1) {
          b.remove(beats);
        }
      }
    }

    ERROR> fr255: source/C.mc:144:9: Argument 1 to $.B.add expected to be Null or Number but got Number or Long or Float or Double
    inlined from $.C.d in source/C.mc:238:45
    ERROR> fr255: source/C.mc:144:9: Argument 1 to $.B.remove expected to be Number but got Number or Long or Float or Double
    inlined from $.C.d in source/C.mc:254:44
    ERROR> fr255: source/C.mc:144:9: $.Toybox.PersistedContent.Track.remove expects 0 arguments, but got 1
    inlined from $.C.d in source/C.mc:254:21
    ERROR> fr255: source/C.mc:238:45: Argument 1 to $.B.add expected to be Null or Number but got Number or Long or Float or Double
    ERROR> fr255: source/C.mc:254:44: Argument 1 to $.B.remove expected to be Number but got Number or Long or Float or Double
    ERROR> fr255: source/C.mc:254:21: $.Toybox.PersistedContent.Track.remove expects 0 arguments, but got 1

    For some reason the optimizer identifies b.remove(beats) as $.Toybox.PersistedContent.Track.remove instead of $.B.remove.

    Turning off (only)Trust Declared Types removes the above errors (but has another one)

    Turning off (only) Propagate Types removes all errors.

  • wow. big chaos

    Well, I've certainly seen cases like that while working on the type analysis, but I thought I'd found and fixed everything.

    If I take your code, make minimal changes to remove the "..." etc, I get no errors or warnings:

    // B.mc
    import Toybox.Lang;
    import Toybox.Math;
    
    class B {
        public function add(d as Number?) as Void {}
        var mArr as Array<Number> = [1];
    
        public function remove(n as Number) as Void {
            var s = mArr.size();
            var remove = n <= s ? n : s;
            for (var i = 0; i < remove; i++) {
                add(null);
            }
        }
    }
    
    // C.mc
    
    import Toybox.Ant;
    import Toybox.Lang;
    
    class C {
        private var mB as B = new B();
    
        public function c() {
            d(42);
        }
    
        (:inline)
        hidden function d(beats as Number) {
            var b = mB;
            if (beats > 1) {
                b.remove(beats);
            }
        }
    }
    

    Looking at the generated code, I saw that both c() and d() got optimized away. So I added (new C()).c() to my App's initialize function. Now I get what I'd expect, but still no errors/warnings of any kind.

    If I change it so that beats is Number or Float, then I get the error

    Argument 1 to $.B.remove expected to be Number but got Number or Float

    similar to what you saw, but none of the other errors.

    Can you post something that actually compiles and produces the errors?

  • The next issue is probably a feature, not a bug :) It tells me that something that I declared and assumed can only be String or Number can also be Null. I tried to see why, and it is kind of true. It will never be null but the optimizer can't know it because it's tricky:

    var mS as String?;
    var mT as Number = -1;

    (:inline)
    function foo() as Number or String {
      // some code, then 4 inlined function calls:
      f1();
      f2();
      f3();
      f4();

      // here starts the problematic part, and lasts until the end of the function:
      var s = mS;
      var t = mT;
      var r;
      if (cond1) {
        if (cond2) {
          r = s;
          if (r == null) {
            r = loadResource(Rez.Strings.foo) as String;
          }
        } else {
          s = null;
          r = getString();
        }
      } else {
        if (t == 0) {
          s = loadResource(Rex.Strings.bar) as String;
        }
        if (t < 5) {
          t++;
          r = s as String;
        } else {
          if (t == 5) {
            s = null;
            t++;
          }
          if (cond3) {
            r = h; // Number
          } else {
            r = "--";
          }
        }
      }
      mT = t;
      mS = s;
      return r;
    }

    private var mArr as Array<String>;
    function setArr(arr as Array<String>) {
      mArr = arr;
    }

    function getString() as String {
      if (mArr == null) {
        setArr(["a","b","c"]);
      }
      return mArr[mI];
    }

    So I can understand why the optimizer thinks that foo() can also returns Null. However it does never do that because of the inner state of the class... So I guess I can do one of 2 things:

    1. you tell me what annotation to add on foo() (I guess either :typecheck(false) or the one you once told me already exists for your tests) - and I understand that using these annotations will probably have a negative effect on the optimizer and thus on the code size (especially that foo is even bigger than shown above) but I can test it and compare the results to the other option(s)

    2. I can probably change the code in foo() in a way that it'll be a bit bigger code size but explicitly ensures that r can't be null at the last line - which will add to the code size when not using the optimizer but probably will be worth compared to the 1st option because the optimizer will be able to gain more in foo()

    BTW foo() is inlined in a function and foo also calls at least 4 other inlined functions - but all those are before the problematic part of the code. So if the annotation on foo will cause the optimizer to not (or less) optimize foo, then maybe I'll try to refactor foo such a way that all the problematic part goes to another function, so the bad effect of the annotation doesn't propagate to the other parts of the code, and it'll be something like:

    (:inline)
    hidden function foo() as String or Number {
      // some code
      f1();
      f2();
      f3();
      f4();
      problematic();
    }

    (:inline)?
    (:no_inline)?
    (:typecheck(false))?
    (:other_annotation_you_tell_me)?
    fidden function problematic() as String or Number {
      ...
    }

  • I'll try. The above code was not tested it was just manually shrinked from my code as an example... Last time I tried to give you the minimal example I deleted all my files accidentally... I'll try to be safer now.

    UPDATE:

    I found 1 small change that caused it to compile with both Propagate Types, Size Based PRE and Trust Declared Types enabled. (BTW It has a very bad result, I'll send them below)

    Here's a more detailed version (still not tested though) that includes the problem and the fix that makes it compile:

    //// B.mc
    import Toybox.Lang;
    import Toybox.Math;
    
    class B {
      public function add(d as Number?) as Void {
      	...
      } 
    
      public function remove(n as Number) as Void {
        var s = mArr.size();
        var remove = n <= s ? n : s;
        for (var i = 0; i < remove; i++) {
          add(null);
        }
      }
    }
    
    //// C.mc
    import Toybox.Ant;
    import Toybox.Lang;
    
    class C {
      private var mB as B = new B();
    
      public function c() {
        d();
      }
      
      (:inline) hidden function d() {
        var b = self.mB;
        var beats = ...;// (Number);
        var t = 987;
    
        // I assumed this is Number as a result of integer arithmetic:
        var problematic = (t * 1000) / 1024;
    
        // my fix adds a few bytes of code but makes it compile:
        var problematic = ((t * 1000) / 1024).toNumber();
    
        if (cond1) {
          b.add(problematic);
        }
        if (beats > 1) {
          b.remove(beats);
        }
      }
    }

    So it looks as I wrongly assumed: 100/3 is Number and 100/3.0 is Float. Either I am wrong or the assumptions of the optimizer are wrong about this.

  • I think for the next release I’ll just use typecheck(false) to disable diagnostics locally. There’s no reason for that to affect optimizations- it will still use whatever it can deduce/prove. It just won’t tell you when that looks problematic. 

  • Sorry I was missleading: So after this "fix" it still gives most of the errors/warnings but then it does compile and run.

  • Away from my computer at the moment, but I think this is a bogus error reported during inlining. 

    just after inlining, there is no type info for the inlined code; but the optimizer thinks there should be, and reports errors as if there was. To avoid that, I try to avoid running the optimizer on the just-inlined code until the next time type propagation runs. But I think you’ve found a case where somehow the optimizer does see the untyped code. 

    note that it should never make bad (as in incorrect) optimization decisions in that case, but it does spit out some strange warnings. 

    the simple fix is to do a separate pass at the end to report all diagnostics; but that might miss cases where code has been optimized away entirely.