Big update to prettier-extension-monkeyc

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: 

flocsy🤠 said:

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);
}

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]

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;
}

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.

flocsy🤠 said:

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: