• Replies 1094 replies
  • Subscribers 30 subscribers
  • Views 122060 views
  • Users 0 members are here
Options
Related

Big update to prettier-extension-monkeyc

markw65
over 2 years ago

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]

  • over 1 year ago in reply to flocsy🤠
    flocsy🤠 said:
    Again: it created the variable that it doesn't use later. log() is not called

    I found this one. In fact, I'd already fixed it higher up the stack (in amongst some not yet released features) but didn't realize I needed it here too. Will put out a new release soon.

  • over 1 year ago in reply to flocsy🤠

    Yes. Fixing this drops out of the "wishful thinking" optimization you described, which is already underway, and I should be ready to release that in the next few days. But this is pretty much an edge case. One more use of "-", and pre would have kicked in anyway...

  • over 1 year ago in reply to flocsy🤠
    flocsy🤠 said:
    Again, not a bug, but probably you shouldn't rename variables for no apparent reason

    So this is because copy propagation replaced zero by its value, and the pre replaced the zeros by a generated variable. I've thought about trying to preserve names when this happens, but its pretty low priority...

  • over 1 year ago in reply to flocsy🤠
    flocsy🤠 said:
    it should've realize that HR_ZONES_MOCK

    This one is a surprise. Across all the code I run through the optimizer I found several cases where I was previously failing to replace constants, that now work, but none where it previously worked and now fails. But you always manage to find the edge cases :-)

    I'll see if I can repro.

  • over 1 year ago in reply to flocsy🤠
    flocsy🤠 said:
    const HR_ZONES_MOCK = null as Array<Number>

    So this one is because you're lying... and the optimizer should be warning you about it too:

    WARNING: The type Null cannot be converted to Array<Number> because they have nothing in common

    So basically you're saying that HR_ZONES_MOCK is an array, and arrays are never null.

    But there is a bug of sorts. If you correct your declaration

    const HR_ZONES_MOCK = null as Array<Number>?;

    then the warning goes away, but it still doesn't get optimized. And that's because Array<Number>? might be null, and it might not; ie once you throw in a cast, I warn if the cast is incompatible with the value, but then ignore the value and trust the type. Turning trustDeclaredTypes off also doesn't help, because that just effectively turns all declared types into Any, so now its just "null as Any".

    So... this is clearly suboptimal, but I'm not quite clear on the best solution. I think we want something like:

    • With trustDeclaredTypes disabled, all casts should be ignored, rather than treated as a cast to "Any". Then regardless of the cast, turning off trustDeclaredTypes fixes the issue.
    • With trustDeclaredTypes enabled, the code as written should fail to optimize, and when written as Array<Number>? it should optimize correctly.

    But I'm assuming the reason for the cast is so that Garmin's type checker doesn't complain about setHeartRateZones(HR_ZONES_MOCK); 

    And Array<Number>? won't keep it happy here (even though you explicitly check for null). But if you change it to 

    if (MOCK && mHeartRateZones == null && HR_ZONES_MOCK != null)

    then Garmin's type checker is happy.

    I'm still thinking about this, because even something innocuous like

    const MY_NUMBER = 4 as Number;

    is currently causing similar issues (ie if (MY_NUMBER==4) doesn't get optimized); but that one is much easier to fix.

  • over 1 year ago in reply to markw65

    I'll check later why it's like that but if i remember correctly it's because in earlier version of the compiler i could have:

    (:memory32K) const X = [];

    And when I compiled for a device with 16K memory it accidentally didn't fail, and treated it as null.

    But you are totally right that it should be with the question mark. Probably you can open a bug report about this one. I think Garmin take those relatively seriously.

    I think it's better to fix this with the trustFeclaredTypes enabled. If it needs to be disabled because of this then it'll loose even more on lots of other places where types could be used to gain more

  • over 1 year ago in reply to markw65

    so if const X = 4 as Number; is easy to fix, then what about const Y = 4 as Number?; Isn't that the same as Array<Number>?; Why does the type bother you? There are 2 different things you do: 1 type check for warnings and colorizing in VSC, and optimizing. Now I can see that there's a difference between 4 and 4.0, but when it gets to nulls? Is null == null true? Yes. Is null as Number == null as Float? I think it should be yes, isn't it?

  • over 1 year ago in reply to flocsy🤠

    Ok, so there were similar cases, but even those I'm not sure if they're needed now. But here it's a special case where I really needed it. This is a mock, and it might be needed to be set to null, because in some devices I don't have enough memory to be able to acomodate the code (there MOCK=false) but in some devices I do have enough memory to mock some parts but not this, so there I set it to null. And all the other places (variables, function parameters) that get this value were already properly Nullable. This is really a bug in the compiler that let me do this, it should've failed with -l 3.

  • over 1 year ago in reply to flocsy🤠
    flocsy🤠 said:
    so if const X = 4 as Number; is easy to fix, then what about const Y = 4 as Number?;

    Well, the first one is easy to fix because the type completely agrees with the value. There's no reason not to just drop the cast altogether, and just turn it into "const X = 4".

    The second one is a bit worse, but not much. I *could* just ignore the cast (since I know that Y isn't null). And in this case, thats probably fine - at least, I can't, off the top of my head, think of a case where (4 as Number?) would type check, but 4 would not.

    But that's not true of your example. Your example shouldn't need a cast, because you check the value for null, so the call is unreachable, and shouldn't issue a warning. But Garmin's type checker errors out anyway.

    So basically the problem I'm having is that I want to ignore casts that I know are false; but that might mean that Garmin's type checker falls over later. After all, you must have inserted the cast for a reason...