Big update to prettier-extension-monkeyc

Here are some stats:

sdk 4.1.7: code: 14865 data: 6336

prettier 2.0.40 no propagate, no trusted: code: 13733 data: 5266
prettier 2.0.40 no propagate, trusted: code: 13730 data: 5266
prettier 2.0.40 propagate, no trusted: code: 13637 data: 5290
prettier 2.0.40 propagate, trusted: code: 13627 data: 5290

prettier 2.0.41 no propagate, no trusted: code: 13733 data: 5266
prettier 2.0.41 no propagate, trusted: code: 13730 data: 5266
prettier 2.0.41 propagate, no trusted: code: 13598 data: 5290
prettier 2.0.41 propagate, trusted: code: 13575 data: 5291

There WAS some other strange issue: some strange bug in 2.0.40. I don't know how to send you a minimal version to reproduce this but there was a serious problem with 2.0.40: it created code that causes stack overflow error. This is 100% reproducible with my project, but not sure I'll be able to shrink it down and still have it. And also it happens only with the older version, let me know if it still is interesting to you.

happens: when the PRE is enabled (in all 4 variations of propagate and trust)

flocsy🤠 said:

Here are some stats:

That looks pretty good? I thought you said that the results were bad when you finally got it to compile?

Yes it's very good! When I tried it for the 1st time with the new version there were some optimization problems but after I fixed them it produced much better code.

flocsy🤠 said:

it created code that causes stack overflow error

Hmm. Assuming it's not a bug causing a recursive call of some kind, I suppose that means that I really should try to share variables to keep the stack frames small. I must admit Ive been assuming that stack space wasn't much of an issue.

I would definitely like to investigate, but there's probably no way to do so without the actual code. When you get the stack overflow does it print a backtrace?

If so, maybe you could see which functions were active, and what the optimizer had done to them (in terms of adding new variables) compared to what it does in the new version.

One improvement in the new version is that if you already have a value in a local, it should use that instead of creating a pre-variable. In the following

  var x = some_computation();
  mMemberVariable = x;
  ...
  mMemberVariable.foo();
  if (mMemberVariable.bar == 42) { whatever(); }
  

it should use x for the last two mMemberVariable references instead of creating a new pre_mMemberVariable local and using that. So maybe something like that pulled it back down to a safe level

flocsy🤠 said:

So it looks as I wrongly assumed: 100/3 is Number

Yes, 100/3 is Number, and my type checker definitely knows that.

But if its the issue I mentioned above where the typechecer is running on code that it thinks has had type propagation run, but it actually hasn't, then this could happen because <unknown-type> / <unknown-type> is going to produce Number or Float or Long or Double (or its going to crash; but there's no need for the optimizer to worry about that possibility, as long as it doesn't cause it to happen when it wouldn't have happened in the first place).

There's also one other edge case (besides inlining) that might cause something like this. If there's a complex conditional guarding a block of code (eg if (!(a && b) || c && !d) { whatever }), and type propagation manages to deduce that the code is unreachable, it won't propagate types into the unreachable block.

But if the optimizer fails to realize that the code is unreachable (which, obviously, is a bug, but there are subtle differences between the environment that type propagation runs in, and the environment that the optimizer runs in) then again, its likely to report errors because everything in the block has unknown types.

There are if-s around the 2 calls that appear in the errors but both are reachable and I know that at least the 1st one was reached because that's the code that calculates my new feature.

flocsy🤠 said:

So I can understand why the optimizer thinks that foo() can also returns Null

Can you explain it? I've tried reading that code a few times, and it looks to me like r is assigned a non-null value on every path through the function. Am I missing something?

I am pretty sure that in the reality it's never null, otherwise I would've see it in ERA. And even according to the snippet I sent it should not be null, however there is something that I considered as a possibility to "sneak in" a null:

getString() that I sent above is a simplification of the code in another class. And even though I know it will never return null because I initialized it with ["a","b","c"] but in THEORY:

private var mArr as Array<String>;
...
return mArr[mI];

Can't this return null?

I mean I can do:
initialize() {
  mArr = new[3];
}

and then there are 3 nulls in mArr. I haven't try this, so maybe it would fail somewhere else (especially because I run with -l 3) but IMHO it would work. And if so then maybe this introduces a possible String or Null.

Ah and here is the real deal: in theory the following could be IMHO:
mS = null; mT = -1; cond1 = false; so this is what we do:
var s = mS;
var t = mT;
// cond1's else:
  if (t == 0) {}
  if (t < 5) {
    r = s as String; // this is a lie, because not s is null
  }
}
return r;

IMHO this is what the optimizer saw (especially because of the type propagation, and I guess it's more clever than I am... it doesn't believe that s is String there.