Big update to prettier-extension-monkeyc

9811827 said:

The crucial point is that whatever the folder's name is, this problem occurs only when barrel's source is inside "bin".

That's surprising, and I need to get to the bottom of it.

I created a test case as I wrote above, with (:foo) and (:bar), and it did repro the problem. I then rewrote my fix, with the checks in the correct place, and that fixed my test case.

But your test case still crashed - but with a new infinite recursion stack. And I realized there are at least a couple more places where we'll hit the same infinite recursion problem. So i started to look at what exactly is going wrong in your case, because as you say, you don't actually have a cyclic class graph.

The problem is this. When class A extends B, in addition to the inheritance, Garmin's compiler inserts a member called B whose value is the class B (you can see it in the ClassDef's that -g produces - the first member of every class has the name of its super class). It does this (I think) so that code like this works:

class MyApp extends Application.AppBase {
    function initialize() {
        AppBase.initialize();
    }
}

Without the member with the name AppBase you'd have to write Application.AppBase.initialize(); and I don't think that would work, because self wouldn't be passed in correctly.

In any case, my optimizer tries to do the same thing, and the bug seems to be that in your test case, "class TestClass extends TestBarrel.TestClass {}", ends up inserting a member called TestClass that refers to $.TestClass, rather than $.TestBarrel.TestClass.

When I try to simplify your test case, that doesn't happen; so maybe it does have something to do with where the barrel is...

While I understand that you try to make it work for every use case that works with the Garmin compiler, I think having source files under bin/ is... edge case? Very bad practice? What I'm trying to say is that maybe it's easier to change their generator code to output to gen/ instead of bin/gen/? I also use gen/ for this stuff.

flocsy said:

change their generator code to output to gen/ instead of bin/gen/

I agree, it would help, and I'll, probably, do this.

My idea was based on Garmin's compiler behavior, that puts generated source code for launching unit tests under "bin".

flocsy said:

having source files under bin/ is... edge case? Very bad practice?

flocsy said:

I also use gen/ for this stuff.

If you have fixed number of generated files, then you don't need to clean "gen" directory properly and can simply overwrite it during each build. I have a varying number of files, so I need to enhance existing clean procedure, and I wanted to avoid that by using "bin".

At first I didn't even think, that this problem is in sources location. Especially when sources inside "bin" work without problems for regular projects, but fail for barrels, and PMC itself puts unpacked barrels into "bin" and reads them without issues.

My goal is to inform Mark, that the problem exists, but it's up to him to decide what to do with it.

9811827 said:

I have a varying number of files, so I need to enhance existing clean procedure

So `rm -rf gen/` isn't the same?

flocsy said:

rm -rf gen/

Thank you, I guess, that's what I'll end up with.

Things are a bit more sophisticated than I describe here, and I need to check first, which other part of my build process such command will or won't affect, that's why I use "bin" now as it already satisfies all my current needs.

OK, I trust you on that, I just wonder what's the difference? Is there some "magic" that VSC or Monkey C plugin know about bin? I mean the stuff under bin/ aren't there until you "delete" them by Clean Project, in which case the whole bin/ directory is deleted, which is the same as deleting the whole gen/ dir?

flocsy said:

Is there some "magic" that VSC or Monkey C plugin know about bin?

Maybe, it is irrational, but I want all default tools to work as they intended to work, even for my non-default project structure, and "Monkey C: Clean project" cleans everything, that is not the committed source code, and I wanted it to do so here.

markw65 said:

But what about "base.sourcePath = b*n"

As I see it is not possible to have such "sourcePath" (at least for SDK 7 and at least for barrels). Barrelbuild command doesn't accept "b*n", "bi*", "bin*", "bin/*", "bin/**" as valid "base.sourcePath" values - only "bin", "bin/*.mc" and "bin/**.mc" and their variations (like "bin/my-unique-gensrc-dir-name/**.mc") will work.

markw65 said:

Garmin's compiler inserts a member

markw65 said:

so that code like this works

Thanks for the info, earlier I was a bit wondering how exactly this syntax works and why it is not the more common "super.methodName()" or something like that, but I didn't come that far. Maybe, it is related to the fact, that you can call not parent class, but any grandparent up to the Object, if it has such method. It is possible for custom class, that extends WatchUi.WatchFace (that in its turn extends WatchUi.View), to call either "WatchFace.onUpdate(dc)" or "View.onUpdate(dc)" (let's skip the problem whether the watch face will update or not properly according to business logic - I haven't checked it, I'm talking about technical possibility to write such code). The bytecode is different:

    getselfv WatchFace // the hidden member, you mentioned
    getsv onUpdate
    frpush
    lgetv 1
    invokem 2

for "WatchFace.onUpdate(dc)" vs

    getmv Toybox_WatchUi View // direct class method call without having an instance???
    getsv onUpdate
    frpush
    lgetv 1
    invokem 2

for "View.onUpdate(dc)".

By the way, generated app from watch face template calls "View.onUpdate(dc)", while class extends "WatchFace". I can't explain this.

flocsy said:

What I'm trying to say is that maybe it's easier to change their generator code to output to gen/ instead of bin/gen/?

I sort of agree. But the only reason I have to skip "bin" paths is because is because the default sourcePath includes all .mc files under the project, and I want the optimizer to work for projects that don't set it.

My current fix goes way beyond what's necessary for that. I think what I'll do is change it so that it only filters out files that were specified with the default path, ie "**/*.mc". Anything else was explicitly specified by the project author, and if it's not compatible with the optimizer, that's their problem to fix.

9811827 said:

and PMC itself puts unpacked barrels into "bin" and reads them without issues.

It does, but only because I fixed a bug last week! Before that, compiled barrels suffered from exactly this problem. I unpacked the files into bin, and then when the optimizer looked for them, it ruled them out (when I first implemented barrel support, I got it right - but at some point I noticed that I had several different functions for resolving jungle file locations, and refactored the code, introducing the bug).

I fixed that one by making the check relative to the jungle directory. The unpacked barrel's jungle was also inside bin, and so the relative path from the jungle to the files didn't start with bin.

One more issue is that right now it's filtering out anything in "bin" - but you can put the generated files anywhere, so it should really be looking at prettierMonkeyC.outputPath to determine what to filter out.

So I think the overall fix is going to be:

• Look at prettierMonkeyC.outputPath to determine what to filter out (I think this fixes your issue because it will only filter out things under bin/optimized, and your files are under other directories under bin)
• Only apply the filter if the search path was **/*.mc (this also fixes your issue)

9811827 said:

for "WatchFace.onUpdate(dc)" vs

9811827 said:

for "View.onUpdate(dc)".

This is the magic of frpush. And I was wrong when I said I didn't think that Application.AppBase.initialize would handle self correctly.

Every time getv (or one of the newer instructions that does something+getv) executes, it notes the module or class where the looked up object was found. Then in most cases, frpush will push that object onto the stack as the "self" for the called function. But if it was a class, and "self" is an Object (ie an instance of a class, and not a Class or a Module), then it just pushes "self".

This does exactly what you want when you call "SomeBaseClass.foo()". It looks up foo in SomeBaseClass, and then calls it with the current "self".

But if you do something like:

class A {
  function foo() { return self instanceof A; }
}
class B {
  function bar() {
    System.println(A.foo()); // prints false!
  }
}

Then it goes horribly wrong. My example is pretty mundane, but eg any calls in A will be resolved in the scope of B. So if we added a method buz to A, and tried to call it from foo, it wouldn't be found. Worse, if we also added buz to B, the call from A would find the one in B. Similarly references to member variables etc would fail.