Big update to prettier-extension-monkeyc

I've posted about prettier-extension-monkeyc before, but I've added a bunch of new features that developers will probably like (well, I've been missing them, so maybe you have too).

The new features it implements for VSCode include:

  • Goto Definition. Point at a symbol, Ctrl/Cmd click, and it will take you to the definition. Or F12
  • Goto References. Right click on a symbol and select "Goto References". It will show you all the references. Or Shift-F12
  • Peek Definition/Peek References. Same as above, but in a popup window so you don't lose your place in the original document.
  • Rename Symbol. Right click on a local, function, class or module name, and select "Rename Symbol". It will rename all the references. It doesn't yet work for class members/methods.
  • Goto Symbol. Type Ctrl/Cmd-Shift-O and pick a symbol from the drop down (which has a hierarchical view of all symbols in the current file). This also appears as an outline across the top of the file.
  • Open Symbol By Name. Type Ctrl/Cmd-T, then start typing letters from a symbol name. A drop down will be populated with all matching symbols from anywhere in your project.

Older features include a prettier based formatter for monkeyc, and a monkeyc optimizer that will build/run/export an optimized version of your project.

[edit: My last couple of replies seem to have just disappeared, and the whole conversation seems to be in a jumbled order, so tldr: there's a new test-release at https://github.com/markw65/prettier-extension-monkeyc/releases/tag/v2.0.9 which seems to work for me on linux. I'll do more verification tomorrow, and push a proper update to the vscode store once I'm sure everything is working]

  • Strange. Do you have any idea where to look? Maybe it's because I moved both the const and the rgb6 function to a global scope?

    BTW when I add (:inline) to rgb6 then I get this INFO 7 times:

    While inlining rgb6: This function can only be inlined in statement, assignment, if or return contexts

    So it's funny, because when I add (:inline) then it thinks at a "too early" stage that it can't be inlined, but if I remove the annotation then it does "inline" it even when I don't ask as part of the constant folding. So maybe the condition when this warning/info is logged could also be improved so it won't print it when it's anyway only constants.

    Update: even when I moved both rgb6 and ZONE_COLORS to the same class next to each other it's the same

  • Found the bug:

    when the rgb6 function is AFTER ZONE_COLORS in the source code (next to each other) then it can't fold it. When I switch their order it folds.

    Previously when I had both of them in global scope I had them in 2 different files, one where I have all my constants and the other in Utils.mc, so probably they "randomly" got into the "wrong" order.

    And another thing, not a bug, but I noticed that the output directory is bin/optimized/groupXYZ-debug/source/source/ (It has source/ twice)

  • Oh what a night, here's another one:

    I noticed that when I have the order: rgb6(); ZONE_COLORS (thus it gets const folded) the generated code size increased by 3 bytes compared to when the order is: ZONE_COLORS;rgb6() (so it DOESN'T get folded). This pointed me to another bug, 'cause it makes no sense.

    So I diff-ed the 2 outputs and I see that in the version when it didn't fold ZONE_COLORS is copied to pre_ZONE_COLORS (because it's used twice in 2 different functions that are inlined into the same function)

    However when it does do the constant folding then the constant is eliminated (not declared any more) and it's value is used twice, which would be OK for a Number, but this is a Long...

    I hope the above explains it to you, let me know if it's not enough and you need me to shrink it down to a small example that reproduces it.

    Also: can you change the l for Longs to L? Small l looks too much like 1... 1234l vs 1234L

  • Yes, I found that too. It's because the optimization pass also marks functions as suitable for constant-folding. So if it comes first, then the constant-folding happens the first time the constant is optimized, otherwise the second (and final time).

    But that revealed a second bug - sometimes it takes two optimization passes to fully constant fold an expression; and thats been hidden by the fact that there *are* two optimization passes. But in this case, constant folding doesn't start until the second pass, so we see the bug...

    It has source/ twice

    Yes. Under the group directory, I put a "source" directory, which includes all of the projects .mc files, and (when needed) a "barrels" directory, which contains any sources extracted from barrels.

    Under my "source" directory, all of your .mc files will be organized exactly as they appeared in your project - so if they're in the top level directory, they'll be directly under "source". If they're in a "source" directory in your project, then yes, they'll end up in "source" (my directory) then "source" (your directory).

  • I saw som funny local variable comments

    The comments are added by the new pass, and reflect what the names would have been if that pass hadn't run. pmcr_* variables are renames due to inlining (eg when you inline a function with a variable named x into a function that already has a variable named x (or has a global variable named x that shouldn't be hidden by the local) you get a pmcr_x.

    And yes, I suppose I could *also* add the comments to those, and then try to preserve the comments through all the other transformations that happen. And maybe I will at some point...

    Also, if you already add comments with original names, can you add the function names next to their inlined block

    Maybe... sometimes. It was easy to add to the variables in the local minimization pass because it's literally the last thing that happens. The variables are inserted with the comments, and then nothing else happens. But blocks get merged and rewritten a lot, so keeping track is likely to be complicated. I could solve that by preventing inlined blocks from getting merged, but that might miss optimization opportunities...

  • Again about :inline vs :inline_once vs :inline_multi vs warning when inlining "big" function multiple times:

    I have a big function: z(). In release it's only used once, so inlining it is beneficial. However when running in the simulator then sometimes I turn on mocking. In case the mocking is enabled then z() is called also from the mocking code. So currently I have these options:

    1. :no_inline - adds some code size when running in sim so I can't test the "peek memory scenarios" even when I have mocking turned off (because that would also add code and I don't have enough memory)

    2. :inline - solves the above case, but then I can't test with another device in simulator where I have mocking on because then the whole huge function is duplicated twice.

    3. :inline_SOMETHING - (where SOMETHING could be some of my existing excludeAnnotations) - there's no annotation that I can use for this. Maybe by adding some new annotation for mocking I could use :inline_no_mock?

    But the real solution would be to have :inline_once

  • which would be OK for a Number, but this is a Long

    When I first implemented pre, I did a lot of testing of this, and I'm pretty sure that what I saw was that for Longs, the cost per use was still 5 bytes (just like for Number). I assumed it was stashing the value of the long somewhere, and then referencing that stashed copy from the code. That's also what it does for String (you can see that in the byte code; but other than by looking at code/data size, its hard to see exactly what its doing for Longs).

    So that made it easy - I could treat Number, Long, Float, Double the same.

    But I just retested with 4.1.7 at O1 (its hard to assess O2 because you don't know what optimizations its doing), and with two uses of a Long, I save 1 byte by putting it in a local. And with 4.1.5 (or 4.1.7 at O0) I save 3 bytes by putting it in a local.

    The 3 bytes makes sense if it's embedding the 8 byte Long in the code: each reference would take 9 bytes vs 2 for a local. So replacing two uses would save 14 bytes on the uses, but would add 11 bytes to initialize the local. So the saving is 3.

    I don't see where the 1 byte comes from with 4.1.7.

    So now Im not sure if my original testing was wrong, or if something changed in the compiler. And in fact, something definitely did change in 4.1.7. So now I need to figure out a way to write automated tests for this so I'll know when things change, and also so I can verify the exact behavior...

  • Simple vs complex objects.  For a Number you have 1 byte for the object type and a 4 byte value.  For a string or long, you have 1 byte for the type, and a 4 byte pointer to the value.  So they might all look like 5 bytes, but complex objects take more than 5.  Storing a long takes 5+8 bytes for example as a long is 64 bits.

    Same with floats and doubles.  Floats are simple objects (32 bit) while doubles are complex (64 bit).  The other simple object is a Boolean and null

  • Simple vs complex objects.  For a Number you have 1 byte for the object type and a 4 byte value

    Thanks Jim! as helpful and relevant as always! Let's take an example shall we?

    Suppose we have a string thats 10 bytes long: "0123456789". So we agree that the string takes more than 10 bytes (the data for the string, and some type info at the least). But when you use it in code, it only takes 5 bytes per reference. If you use it 3 times in code, it takes 15 (code) bytes. And 10 times it uses 50 (code) bytes.

    But if you load it into a local up front, then it takes 7 bytes to put it in the local, and each subsequent use only takes 2 bytes.

    So if you use it 3 times, thats 6 + 7 = 13 bytes, saving you 2 bytes overall. Note that the length of the string, and the amount of data it holds, is completely irrelevant, because in both cases it's the same. And if you use it 10 times, it's 20 + 7 = 27 bytes, saving you 23. And again, the actual size of the string is irrelevant.

    So yes, obviously complex objects take more than 5 bytes in reality. But they don't necessarily add more than 5 bytes *per use* to the code.

    My previous post was just explaining that 6 months or so ago I *thought* I'd established that the compiler did the same thing for Long and Double (ie one copy of each distinct Long or Double, stored somewhere, and then 5 byte references to them from code).

    But it looks like either I was wrong, or the compiler has changed since then; and at least in 4.1.5 (or 4.1.7 at O0) each reference to a Long takes 9 bytes (so apparently the value of the Long is embedded directly in the code, rather than using a 5 byte reference to it). And Im still not sure what 4.1.7 at O1 or O2 is doing...

  • IMHO this is a bad warning (shouldn't be warning) [ah and BTW it's even displayed as an error because of my settings, which is another "bug", even if I set it to ERROR this shouldn't be logged as an error]:

    typedef PropertyDictionary as Dictionary<PropertyKeyType, PropertyValueType>;
    typedef PropertyDictionaryArray as Array<PropertyDictionary>;
    (:array_settings)
    function getConfigPropertyDictionaryArray(key as PropertyKeyType, defaultValue as PropertyDictionaryArray?) as PropertyDictionaryArray? {
        var value = getConfig(key);
        return value != null ? value as PropertyDictionaryArray : defaultValue;
    }
    
    function x() {
        var users = getConfigPropertyDictionaryArray("u", DEFAULT_USERS_PROPERTIES);
        for (var i = 0; i < users.size(); i++) {
                var user = users[i];
                if (user != null) { // This comparison seems redundant because user should never be null[pmc-analysis]
                    var x = user["x"];
                    ...
                }
            }
        }
    }

    and CURRENTLY in all build configurations:

    const DEFAULT_USERS_PROPERTIES = [] as Array<PropertyDictionary>;
     
    So if I don't want to get the warning then I need to remove the check for user != null. But if I do that then if/when I change the DEFAULT_USERS_PROPERTIES to null for some devices then I'll get a null pointer exception IMHO.