Big update to prettier-extension-monkeyc

Well :) This is not the first time that either the optimizer points to some bug of mine or like in this case it made me think, and I realized I could change the code to pass 1 array instead of the last 4 or last 5 params, as it is anyway passed as an array. It has a slight theoretical disadvantage: it looses typechecks, but I can live with it, especially that it also allowed me to inline the 2 Lang.format callas and alltogether decreased code+data size by 48 bytes :)

flocsy said:

Why doesn't VSC display a peek window with all the references

Well, you can always hit Shift-F12 for "Goto References", or Option-Shift-F12 for "Visit All References". Its possible there's some way to customize the behavior - either for you as a user, or for me as an extension writer, but that kind of thing tends to be hard to find in the documentation unless you know what you're looking for...

flocsy said:

Expected $.C.foo to return Number or String but got Null or Number or String

With a bit of tweaking to your original example, I have been able to repro that error. I'll try to figure out whats going on. It may be that its correct...

I think it is correct, that's what I wrote you yesterday. For my purposes it's good enough that I add "as Number or String". If it didn't crash for almost a year it's going to be ok

flocsy said:

I think it is correct

I simplified it further, and it's definitely a bug in my code. This version still gives the warning, even though it clearly can't be null.

var mS as String?;

function foo(c as Boolean) as String {
    var s = mS;
    var r = null;
    if (c) {
        r = s;
        if (r == null) {
            r = "";
        }
    } else {
        r = "";
    }
    return r;
}

The problem is that I added some code to determine when variables definitely hold the same value, and perform some optimizations based on that. In this case it knows that at the line that does "if (r==null)" r and s hold the same value, and it decides to use s instead of r (in this case, thats a bad decision, but sometimes it can eliminate a variable).

So now it says 'if (s == null) { r = ""; }' and it superficially looks like it's not checking the r==null case. But part of the reason for doing the equivalency analysis is so that when you do a comparison like this it doesn't just know that s is null inside the if, it also knows that r is null there. But I missed a case.

Anyway, there's a simple fix. I probably won't push a new release just yet because it doesn't seem urgent, and I have a number of other things I want to work on.

flocsy said:

Why doesn't VSC display a peek window with all the references

It turns out that VSCode has a lot of configuration options around this, but none do exactly what you want.

The default behavior for clicking on a symbol is "Goto Definition". But there's an override that applies if Goto Definition would take you to the current location - and you can pick pretty much any editor command as the override. 

I was looking at fixing a slightly different issue, and realized that there is a fairly simple solution. When you click on a definition, I can restrict the results to just that one.

This mostly does what you want, but results in somewhat asymmetric behavior - goto definition on any of the definitions always takes you to that definition (and so VSCode's default override takes you to the references). But goto definition from any of the references takes you to all the definitions.

Worse, if I always apply that, then Rename Symbol renames all refs and defs if you click on a ref, but only that single definition, and all references if you click on a definition... so it has to be applied selectively.

But I think I can give you an option for the behavior you're after.

2 new things:

1. you seem to remove public functions that are not called by my code. Now I don't know what is the right thing to do:
- remove unused functions (no matter if they are public or private or overriding a function in the base class)
- don't remove "unused" public or protected functions if they override a function in the base class [that is probably called]
- don't remove unused public functions because they might be called by someone (I'm not using barrels, so I don't know if this is meaningful. If barrels can be distributed as bytecode then publics shouldn't be removed, because the public functions could be called by the user of the barrel)

2. with this code:

Stack Overflow again. I'm again testing in the simulator on fr255 that has 252kB DF memory and I only use ~32 of it, but unfortunately run out of stack, but only when compiled using the prettier optimizer. So I am thinking loudly what options do I have:

1. in my code use class variables directly instead of copying them to local -> increases code size slightly and in some places prettier will "optimize it out"

2. in my code use arrays to hold local variables -> probably increases the code a bit more, and loose the typechecking if I want to keep more than one type of variables in the same array.

3. prettier could have setting(s) to do some of the above tricks

4. prettier could re-use the local variables and this might be enough (and I think it's already under development)

5. in my specific case (I added new features to my code that doesn't go to devices with low memory, so they're OK, and the devices that get the new feature have enough memory to run without the prettier optimizer) it would help and probably easier to implement this than #3 (and there might already be a solution to this, but I don't know what "Prettier Monkey C: Ignored Annotations, Ignored Exclude Annotations exactly mean) there could be a way to exclude a whole device from being optimized when I do Export Optimized Project. It would also have the advantage of not having to use #1 or #2 in the code common between low and high memory devices (95% of the code) so the low memory devices wouldn't suffer any penalty.

update: I checked, and in one of my 2 biggest functions (after inlining) (compute, onUpdate) there are 40 local variables. So probably #4 would be OK for a short while, though I'm not sure if it'll still be enough after another half a year.

Anyway, maybe you should use instead of pre_foo just pre1,...preN and in every place where it's used add a comment with it's "current" meaning (just to make it easier to read it):
var pre1;
pre1 = 1;
if (mX > pre1 /*1*/) {..}
..
pre1 = mY;
if (pre1 /*mY*/ != null && pre1/*mY*/.bar == 3) {...} // though maybe it's enough to add /*mY*/ once if it's used multiple times in 1 line or even in 1 expression. 

also I think some of the currently generated variables are unnecessary altogether:

log("foo: " + foo + ", bar: " + bar);

(:meee) LOG = true;
(:inline)
function log(msg as String) as Void {
    if (LOG) {
        logRelease(msg);
    }
}

function logRelease(msg as String) as Void {
    System.println(Time.now().value() + " " + msg);
}

gets inlined into:

{
  var msg = "foo: " + foo + ", bar: " + bar;
  logRelease(msg);
}

I don't see why this couldn't be:

{
  logRelease("foo: " + foo + ", bar: " + bar);
}