Big update to prettier-extension-monkeyc

Optimizer bug: When I compile this: https://github.com/flocsy/connectiq-samples/blob/master/libraries/text-picker/TextPicker.mc then I get this error:

ERROR: edge1030: bin/optimized/group019-release/source/source/lib/TextPicker.mc:376,8: Cannot find symbol ':charSelected' on type 'Null'.

I think that a stupid bug in the compiler (that we reported at least a year ago I think multiple times, that the compiler complains in code blocks like: if (foo != null) {foo.bar();} - well probably not in this simple case, but in almost any other case) that forced me to "decorate" the  TextPickerDelegate.onSelect with "(picker as TextPickerView)" in 3 of 6 places but not in the other 3 places probably caused the optimizer to do something like:

picker.charSelected();

flocsy said:

What if the "bytecode" thinks that there is no return value

That's not an issue. A function that "doesn't return anything" is supposed to, and usually does return Null (in fact, you can construct functions that return arbitrary values without a return-with-expression statement - I've filed issues about this in the past). So the bytecode always has to consume the result of a function call.

In general, casting a function that returns one type, to one that returns another could have arbitrarily bad results. But I think you're right in principle. If someone calls a function that's declared as returning void, there's nothing "bad" they can do with the result without at least getting a type-checker error. So maybe it's ok to allow that one specific cast.

Meanwhile the workaround for any "because they have nothing in common" type error is to cast to Object? first:

method(:onNextPage) as Object? as Method() as Void

    hidden function formatTimeGoal(timeInSeconds as Number?, goalTime as Number?, goal as Goal, includeSeconds as Boolean) as String or Number or Null {
        var time = getGoalDisplayValue(timeInSeconds, goalTime, goal) as Number?;
        if (goal.displayId == PROPERTY_DISPLAY_PERCENT) {
            return time; // percent as Number
        }
        return formatTime(time, includeSeconds);
    }

    // (:inline)
    hidden function formatTime(time as Number?, includeSeconds as Boolean) as String? {
        if (time != null) {
            var SIXTY = 60;
            var DIGIT1 = "%u";
            var DIGIT2 = "%.2u";
            var SEP = ":";
            var EMPTY = "";
            var sign = time < 0 ? '-' : EMPTY;
            if (time < 0) {
                time = -time;
            }
            var seconds = time % SIXTY;
            time = time / SIXTY;
            var minutes = time % SIXTY;
            var hours = time / SIXTY;
            time = sign + (hours > 0 ? hours.format(DIGIT1) + SEP : EMPTY) + minutes.format(hours > 0 ? DIGIT2 : DIGIT1)
                + (includeSeconds ? SEP + seconds.format(DIGIT2) : EMPTY);
        }
        return time;
    }

    hidden function getGoalDisplayValue(valueInKCal as Number or Float or Null, goalInKCal as Number or Float or Null, goal as Goal) as Number or Float or Null {
       return something
    }

When I add :inline annotation to formatTime (which is only used in this 1 place) then I get a compilation error:

ERROR: fr255: bin/optimized/group063-release/source/source/Foo.mc:331,8: Attempting to perform operation 'lt' with invalid types 'Null' and '$.Toybox.Lang.Number'.

it doesn't like: time < pre_0

This is the optimized code:

  hidden function formatTimeGoal(
    timeInSeconds as Number?,
    goalTime as Number?,
    goal as Goal,
    includeSeconds as Boolean
  ) as String or Number or Null {
    var pre_0;
    var time = getGoalDisplayValue(timeInSeconds, goalTime, goal) as Number?;
    if (goal.displayId == 2) {
      return time; // percent as Number
    }
    {
      timeInSeconds /*>pmcr_time_0<*/ = time;
      if (timeInSeconds /*>pmcr_time_0<*/ != null) {
        pre_0 = 0;
        if (timeInSeconds /*>pmcr_time_0<*/ < pre_0) {
          timeInSeconds /*>time<*/ = pre_0 - timeInSeconds /*>pmcr_time_0<*/;
        }
        goalTime /*>pre_60<*/ = 60;
        goal /*>pre___<*/ = "";
        var seconds = timeInSeconds /*>time<*/ % goalTime /*>pre_60<*/;
        timeInSeconds /*>time<*/ =
          timeInSeconds /*>time<*/ / goalTime /*>pre_60<*/;
        var hours = timeInSeconds /*>time<*/ / goalTime /*>pre_60<*/;
        timeInSeconds /*>time<*/ = // the error is on this line
          (time < pre_0 ? '-' : goal /*>pre___<*/) +
          (hours > pre_0 ? hours.format("%u") + ":" : goal /*>pre___<*/) +
          (timeInSeconds /*>time<*/ % goalTime /*>pre_60<*/).format(
            hours > pre_0 ? "%.2u" : "%u"
          ) +
          (includeSeconds ? ":" + seconds.format("%.2u") : goal /*>pre___<*/);
      }
      return timeInSeconds /*>time<*/;
    }
  }

I used to make it default to not running Garmin's type checker on the optimized code. That's mostly reasonable because you're going to test the unoptimized code anyway (right?), and we know that Garmin's type checker has a lot of these issues.

Here the problem is this:

function foo(a as Number?) {
  var b = a;
  if (a != Null) {
    return b + 4; // Garmin thinks that b can be Null
  }
}

It's a bug (or at least, a deficiency)  in Garmin's type checker. So I'm not sure what I can do about it.

I think the option to turn off garmin's type checker when building the optimized code may no longer work. But I'm afraid I think that's the answer here (and your previous post). I'll make sure the option works in the next release.

I found a workaround that even makes smaller code:

    (:inline)
    hidden function formatTime(timeInSeconds as Number?, includeSeconds as Boolean) as String? {
        var time = null;
        if (timeInSeconds != null) {
            var SIXTY = 60;
            var DIGIT1 = "%u";
            var DIGIT2 = "%.2u";
            var SEP = ":";
            var EMPTY = "";
            var sign;
            if (timeInSeconds < 0) {
                time = -timeInSeconds;
                sign = '-';
            } else {
                time = timeInSeconds;
                sign = EMPTY;
            }
            var seconds = time % SIXTY;
            time = time / SIXTY;
            var minutes = time % SIXTY;
            var hours = time / SIXTY;
            time = sign + (hours > 0 ? hours.format(DIGIT1) + SEP : EMPTY) + minutes.format(hours > 0 ? DIGIT2 : DIGIT1)
                + (includeSeconds ? SEP + seconds.format(DIGIT2) : EMPTY);
        }
        return time;
    }

Possibly bug: classes that are not used are kept. Why? Am I missing some use-case?

I develop my app, and inside it I prepared a few classes as a "library" for future re-use in other projects. There are some classes that are not used in any way. An example:

import Toybox.Lang;

class FloatPickerView extends TextPickerView {
    const DIGITS = "1234567890.";  // valid characters for input
    function initialize(title as String, header as String, footer as String, minChars as Number, maxChars as Number, value as String or Number or Null) {
        TextPickerView.initialize(DIGITS, title, header, footer, minChars, maxChars, value);
    }
}

IMHO there's no reason to have it here.

Now it's possible you're removing it in the post build phase, I don't see that, but it's there in the optimized/ directory.

I have a problem: I run my DF (optiized) in simulator in fenix6 with peak memory usage: 26.1/28.5. 2kB seems to be a good margin. But when I try it on a real device then I can't even add the DF to the layout, it doesn't even get to the crash"IQ!" icon, it just falls back to TIMER. When I look in CIQ_LOG.YML I see that it crashed very early with Out of memory error. So early it didn't even start to run my code as far as I can tell:

---
Error: 'Signature check failed on file: ._optimized-GoalDF'
Time: 2024-03-14T11:16:08Z
Part-Number: 006-B3289-00
Firmware-Version: '26.00'
Language-Code: eng
---
Error: Out Of Memory Error
Details: Failed loading application
Time: 2024-03-14T11:16:33Z
Part-Number: 006-B3289-00
Firmware-Version: '26.00'
Language-Code: eng
ConnectIQ-Version: 4.2.4
Filename: 'optimized-GoalDF'
Appname: Goal β
Stack: 
  - pc: 0x80000000
    Native: true

First I thought there must be some bug in the simulator that it doesn't simulate the memory usage close enough to the real device.

I even put together a POC to demonstrate it and open a bug for the simulator, but POC works: I have a huge string constant, and I am able to set it to the maximum length, such that if I add 1 more character to it then it gets the Out of memory error in the simulator (without optimizer). The same code exported for device (without optimizer to make it easy to reproduce). I expected it to fail with out of memory error, but no, it works fine.

So I wonder what can be different in my real DF and in the POC? How come the simulator runs the optimized code without any problem and the on real device crashes because there's not enough memory?

Also: anyone ever seen the 1st error in above: Error: 'Signature check failed on file: ._optimized-GoalDF' Can it be related?

flocsy said:

IMHO there's no reason to have it here

You're right. It's something I've been meaning to do, but for a long time I didn't have all the necessary information. eg the only reference to a particular class might be from a resource, and for a long time I didn't really analyze resource files.

I think I could find all the possible references to classes now, and then remove them the same way I do unreferenced functions - but I just haven't got around to it.

flocsy said:

I have a huge string constant, and I am able to set it to the maximum length, such that if I add 1 more character to it then it gets the Out of memory error in the simulator

So my guess is that the optimizer is doing some kind of constant folding, and producing an even longer string. I don't currently have any checks on string length, so that could easily happen.

flocsy said:

Also: anyone ever seen the 1st error in above: Error: 'Signature check failed on file

I've not noticed it, but it's probably true. Every .prg file has a signature section (garmin's compiler uses your developer certificate to sign the .prg).

The post build optimizer doesn't bother to fix the signature, because the simulator doesn't care (and neither does the watch if you side load the .prg). So my guess is that that's what it means. And unless something changed recently, it should be harmless.

Note that when running the post build optimizer on a .iq file, it *does* rewrite the signature (once for each embedded .prg file, and again in the manifest file), because the iq store would reject it otherwise.

I'll change it to always fix the signature, just to avoid any confusion regarding that message.

I don't think that the constant folding can explain this. I have some code. I compile it with the optimizer and run it on the simulator. Works fine. Then I build it for the same device with the optimizer (actually I don't remember if I did a build or just copied the already existing prg to the device) and on the real device it crashes. So I assume that the code in the simulator and on the real device was the same. Whatever was the constant folding the optimizer did, it should work the same way on simulator and on real device.