Why could this code give invalid value in ERA?

There is an optimizer in the garmin compiler.  You can set the level in VSC or include in your monkey.jungle file, where X is the level you want to use 

:

markw65 FYI

[edit: I didn't see page one of the discussion here - and didn't realize you were using my optimizer, so this is probably wrong]

I noticed in this bug report that Garmin's compiler sometimes optimizes "has" expressions based on the specified "api level". In that bug, it was incorrectly optimizing Toybox.SensorHistory has :getBodyBatteryHistory to false, which resulted in missing info on a watch that did support it. But maybe there are cases where it incorrectly gets optimized to true?

If you can tell us the device model(s) where this happens, I might be able to figure it out.

In any case maybe try Jim's suggestion, and turn off the optimizer? Alternatively, to avoid losing optimizations altogether, you could use the workaround in that bug report: put the symbols into variables.

var sensorHistory = :SensorHistory;
var getBodyBatteryHistory = :getBodyBatteryHistory;
if (Toybox has sensorHistory && Toybox.SensorHistory has getBodyBatteryHistory) {
    var lastvalue_0 = Toybox.SensorHistory.getBodyBatteryHistory({:period => new Time.Duration(900)}).next();
    if (lastvalue_0 != null && lastvalue_0 has :data && lastvalue_0.data != null) {
        _percentage = lastvalue_0.data.toFloat();
    }
}

Also, if you add "-g" to the compilation command (either by running it from the command line, or adding it to compilerOptions in the MonkeyC settings in VSCode), it will dump the bytecode. There will be a lot of it, but if you search through for the definition of your function (the name will occur a few times, but you're looking for something like:

.../myClass/myFunction:
    argc <number of arguments>
    ... // lots more bytecodes
.../myClass/myFunction_end:

If you post everything from myFunction: to myFunction_end:, I can probably tell you whether this is the problem or not...

RobinMorgan said:

Actually convers into the following when using prettier monkey C

If thats the case, it's a bug. I noticed a long time ago that I couldn't trust the documentation to report nullable results, and so the optimizer doesn't normally get rid of null checks. It *does* emit a diagnostic, saying that the value should never be null, but it should leave it alone. Let me see if I can repro.

RobinMorgan said:

But maybe the creator of Prettier Monkey C should also be notified then?

The easy way to do that is from the marketplace page, on the right hand side there's an "issues" link. Click that and add a new issue. That should work for most extensions.

I just tried your example, and for me, given:

  var xyz = Toybox.SensorHistory.getBodyBatteryHistory({
    :period => new Time.Duration(900),
  });
  if (xyz != null) {
    var lastvalue_0 = xyz.next();
    return lastvalue_0;
  }
  return null;

the optimized code comes out as:

  var xyz = SensorHistory /*>Toybox.SensorHistory<*/.getBodyBatteryHistory({
    :period => new Time.Duration(900),
  });
  if (xyz != null) {
    return xyz.next();
  }
  return null;

ie the null test is not removed. I do get a warning:

WARNING> fenix5xplus: SimpleTestApp.mc:18:7: This comparison seems redundant because xyz should never be null

but it doesn't get rid of it, because of the known issues with the api specifications.

So back to the original problem... did you post the optimized code where the crash happens, or the original? If the latter, can you post the actual, optimized code?

I've found the problem.

My optimizer won't remove "if (xyz != null) {" when xyz is declared as a non-nullable object, because I explicitly prevent that optimization due to the problems discussed elsewhere with the declarations of various Toybox functions.

But it does optimize away "if (xyz) {", even though it shouldn't, for exactly the same reason.

I hadn't tried/tested that, because Garmin's type checker will complain about it:

Unexpected type: expected '$.Toybox.Lang.Boolean' but received '$.Toybox.SensorHistory.SensorHistoryIterator'.

So at -l2 or -l3 your code won't even compile (and at -l1 it warns); but it is accepted by the runtime, so I need to fix the optimizer.

The problem is fixed in v2.0.64 of Prettier MonkeyC.

To summarize the discussion, the problem is that although Toybox.SensorHistory.getBodyBatteryHistory is declared as never returning null, sometimes it does. I had already ensured that my optimizer didn't take advantage of that in cases like:

var xyz = Toybox.SensorHistory.getBodyBatteryHistory({
    :period => new Time.Duration(900),
  });
  if (xyz != null) {
     ...

however, I'd missed this case, where you just test for "truthyness", without the explicit null check:

var xyz = Toybox.SensorHistory.getBodyBatteryHistory({
    :period => new Time.Duration(900),
  });
  if (xyz) {

and in that case, my optimizer would remove the if (which would be fine, except for the Garmin bug).

v2.0.64 fixes that issue.

Hi Mark

Sorry for my late reply, was a bit busy

So this is my original code

and basically if I null check the getBodyBatteryHistory I get a warning

  var bodybattery = Toybox.SensorHistory.getBodyBatteryHistory({
          :period => new Time.Duration(900),
        });
        if (bodybattery != null) {

Thank you btw for looking into this! Love the optimizer, it saves me another couple of desperate needed bytes of memory every single time

RobinMorgan said:

and basically if I null check the getBodyBatteryHistory I get a warning

Yes, I'm not exactly happy with the current situation.

Right now, I don't ever remove null checks (and after yesterday's update that includes when the check is written as 'if (x)' rather than 'if (x != null)'), because of the bugs in Garmin's Toybox type declarations. But in a lot of cases, the checks would be safe to remove (eg 'var x = new MyClass(); ...; if (x != null) {}'), so I ended up spitting out a warning, so that the user can make their own decision. But I guess the warning needs to make it clearer that the check really might be needed in some cases...

I have an idea for a better solution - so I may have another update before too long.