Big update to prettier-extension-monkeyc

I have a question about code that might crash. I suppose this mostly affects untyped code...

Suppose I have something like:

class MyClass {
  const UniqueConstant = 42;
  var UniqueVariable = 5;
}

function foo(x) {
  if (x.UniqueVariable != 0) {
    return x.UniqueConstant;
  }
}

function bar(x) {
  return x.UniqueConstant;
}

function baz(x) {
  var y = x.UniqueConstant;
  return y;
}

The "Unique" names indicate that this is the only definition of a same-named entity anywhere in the program (ie there are no methods, enums, modules, classes or variables with those names anywhere else).

Now in foo, I see a use of x.UniqueVariable, and now I know for sure that x is an instance of MyClass. So now I can safely replace x.UniqueConstant by 42 (because if x is *not* an instance of MyClass, the program will crash and we won't get to the return). Nothing controversial so far - but this example is the motivation for doing this kind of type deduction.

In bar, I similarly know that *if* the function returns without crashing, it will return the value 42. But I don't want to remove the access to x.UniqueConstant, because now an incorrect program could continue to run, when it should have crashed. So I mark x.UniqueConstant as having a potential side effect, and the optimizer leaves it as it is. I think this is the right thing to do.

But now in baz, I try to do the same thing. I mark x.UniqueConstant as having side effects, so it's left as is. But the optimizer knows that y has the value 42. So the code ends up as:

function baz(x) {
  var y = x.UniqueConstant;
  return 42;
}

But now y is unused, so the code is converted to:

function baz(x) {
  x.UniqueConstant;
  return 42;
}

(note that it doesn't drop the x.UniqueConstant, because it was earlier marked as having potential side-effects). But now Garmin's compiler refuses to compile it, because x.UniqueConstant is an unused expression...

So do I just say wtf, the code is either correct, and returns the value 42, or it was going to crash, and I don't care what the optimized code does, so it might as well return 42...

Or do I give up on this kind of thing? I could also say that when I don't know the type of x for sure, I keep the type of x.UniqueConstant, but drop the value. In that case baz ends up mostly unchanged; but I'd still have the same problem with:

function buz(x) {
  var y = x.UniqueConstant;
  return y instanceof Lang.Number;
}

because the return statement gets optimized to "return true", so again y is dead, and we're left with a dangling initializer.

Also, note that these are pretty contrived; the main reason for doing this is that in a complex function with many uses of x, this approach lets us deduce the type of x from the first access (or first few accesses if the names aren't unique), and then we can optimize much better down stream. So the point is to allow the useful optimizations without falling into the traps listed above.

Am I just over thinking this? Should I just make a reasonable effort to preserve the "crashiness" of the original code, but not care about outliers like baz?

Nice blog. I figured out most of these tricks alone and with hard try. It could've save me some time if I find it earlier. There's one trick however that I don't understand. Why do you have this:

I think you are a little bit overthinking it. Add a sentence to the documentation that explains that the optimizer can work better if used with -l 3 and it is the recommended way to compile. Then make as much effort as you can to optimize in the best case (-l  3) scenario.

And if you still have time / need (though I bet that one good reason to make it work even better not with -l3 would be if your own projects don't compile with -l 3 and you want to be able to use it for them as well, BUT realizing the time and effort you put into the optimizer it's clearly not the point, 'cause then you could've fix all your projects to compile with strict mode in much less time :) then continue to try to improve it even without -l 3.

What I posted is the exact code and mResult is not used anywhere else after that in the function, and the function is not inlinable, however it is used "before" that in inlined code, so maybe this is the reason it didn't do the trick.

    (:memory16K)
    public function compute(info as Activity.Info) as Lang.Numeric or Time.Duration or Lang.String or Null {
        var extHr = computeExtHR();
        if (mResult2 != null) {
            extHr += " " + mResult2;
        }

        if (mTicker > FOUND_SECONDS) {
            extHr = strfHrZone(mConfFormat1, extHr, "", "", computeIntHR(info)); // inlined but doesn't have mResult2 in it
        }
        return extHr;
    }

    (:inline)
    hidden function computeExtHR() as AlphaNumeric {
        // ...
        var result2 = mResult2; // this was already manually optimized by me
        // ... result2 is used many times both as left and right value.
        mResult2 = result2;
    }

flocsy said:

# fix autobuild to a specific device

base.sourcePath=$(fr735xt.sourcePath)

Why not the "usual":

base.sourcePath = source

in eclipse the autobuild functioned without a specific device and the source folder alone does not build for me, I need a combo of source, source-ciq\some-version, source-device\some-device

in vscode it's probably no longer an issue as you always build for a specific device there.

markw65 said:

you probably don't want the optimizer to do it automatically

A good developer is lazy, if it can be automatic, I prefer automation.

Another challenge is probably that you would have to recurse through the tree as there can be multiple inheritance levels.

Anyway currently I'm not a user of your extension, but if you get to implementing class inheritance elimination, you can ping me and I'll test it out :) 

(:no_api2, :inline, :typecheck(false))
function getConfig(key as PropertyKeyType) as PropertyValueType {
    try {
        return ExtHRMApp.getProperty(key);
    } catch (e) {
        return null;
    }
}

After adding the try/catch I get the following error during optimization:

Starting optimization step...
ERROR: Internal: Error: Got exception `ReturnStatement got lost!' while processing node $.getConfigBool:VariableDeclaration from source/Config.mc:72:72
Error: Got exception `ReturnStatement got lost!' while processing node $.getConfigBool:VariableDeclaration from source/Config.mc:72:72
at inlineWithArgs (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:5691:19)
at inlineFunctionHelper (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:5782:16)
at inlineFunction (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:5792:17)
at optimizeCall (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:9160:25)
at state.post (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:8904:53)
at ~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:6365:40
at ast_traverseAst (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:437:20)
at ~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:398:34
at Array.reduce (<anonymous>)
at ast_traverseAst (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:396:38)
at ast_traverseAst (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:417:24)
at ~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:398:34
at Array.reduce (<anonymous>)
at ast_traverseAst (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:396:38)
at Object.traverse (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:6062:20)
at api_collectNamespaces (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/api.cjs:6437:11)
at ~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:8996:63
at Array.forEach (<anonymous>)
at optimizeMonkeyC (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:8995:26)
at async Promise.all (index 1)
at async generateOptimizedProject (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:9569:5)
at async buildOptimizedProject (~/.vscode/extensions/markw65.prettier-extension-monkeyc-2.0.38/node_modules/@markw65/monkeyc-optimizer/build/optimizer.cjs:9267:61)

* The terminal process terminated with exit code: 1.
* Terminal will be reused by tasks, press any key to close it.

* Executing task: omonkeyc: epix

I understand that it can't be inlined because there are 2 returns, but util recently in this case there was a warning.

flocsy said:

After adding the try/catch I get the following error during optimization

This one repros for me. I should be able to fix it.

flocsy said:

What I posted is the exact code and mResult is not used anywhere else after that in the function, and the function is not inlinable, however it is used "before" that in inlined code, so maybe this is the reason it didn't do the trick

I'm sure thats it. When I said "later" I should have just said elsewhere in the function. And obviously, it its there because of inlining thats all the same (pre runs after all the other optimizations). But I've tried to repro, by doing something along the lines you presented, and I've had no luck. Again, if there's any chance you can cut it down to something that exhibits the problem, and that you're willing to share, that would really help.

flocsy said:

After adding the try/catch I get the following error during optimization

So the bug is just that after issuing the warning that the function can't be inlined because it has too many return statements, there's a missing return (in my code), so it then tries to inline it, finds that it doesn't end with a return statement, and throws that internal error because the code (supposedly) checks all the conditions up front.

But the bug has been there for several releases (if not forever)... you seemed to imply this was new?