Runtime and analyzer generate different source for the same macro ("library augment" missing in runtime)

[DanTup]

When debugging, we need the macro generated files produced by the analyzer and runtime to match up so that breakpoints added before you start running (done by navigating using the analyzer to the analyzer-produced virtual files) are in the correct locations when the code is run.

Currently (with https://dart-review.googlesource.com/c/sdk/+/349864 patched in), there seems to be a difference ("library augment" missing in the runtime version) which means all of the locations are different.

Left is from the analyzer, right is from the VM.

[bwilkerson]

@jakemac53

[jakemac53]

Interesting, @johnniwinther is the cfe removing this line by chance?

[davidmorgan]

FYI #54718 is on me to add a way of testing that CFE and analyzer output the same code.

[jakemac53]

Working to resolve this in https://dart-review.googlesource.com/c/sdk/+/353322 . This analyzer was adding this before and this works towards moving that into the shared code.

[DanTup]

@jakemac53 this was fixed, although I'm seeing differences with prefixes in recent code (c506bb8):

I believe everything I have is consistent from that version (analyzer, VM etc.), but it's possible I've missed something. Any ideas?

[scheglov]

It looks that CFE does not remove unnecessary import prefixes yet.

[jakemac53]

cc @davidmorgan maybe we should prioritize a test ensuring no differences here :D

@scheglov just so I understand, the analyzer is using the code optimizer now, but the CFE is not?

The priority should imo be that we generate the same code - so I think we should either revert the analyzer change to use the code optimizer, or land the support to use it in the CFE ASAP if that is easy to do.

[scheglov]

Well, it is trivial to remove from the analyzer.
IIRC it was told that CFE can support this too.
But actually implementing it in CFE is outside of my normal scope.
So, I would prefer someone else to do this integration.

[jakemac53]

Ok, I would recommend reverting it in the analyzer for now then, until @johnniwinther can implement (or delegate) support for it.

[scheglov]

https://dart-review.googlesource.com/c/sdk/+/357441 to disable for now in the analyzer.

[davidmorgan]

Yeeeeah there is often a case for testing being cheaper the sooner you add it ;) if it looks like I should tackle the golden tests as the very next thing I do please let me know.

[jakemac53]

Yeeeeah there is often a case for testing being cheaper the sooner you add it ;) if it looks like I should tackle the golden tests as the very next thing I do please let me know.

I think getting flutter working should be the priority, but this should probably follow up closely behind

[davidmorgan]

Closing with #54718 to track adding test support / coverage