Nightly Safe Macros

[Aidan63]

This attempts to update the reader and writer macros to play nicely with haxe nightlies completion.

The type of the reader / writer is hashed and used as part of the generated types name and it will avoid defining multiple classes for the same type, this is following the guidelines from this PR (https://github.com/HaxeFoundation/haxe/pull/11159).

This seems to work nicely in my experience so far, previously completion use to fall apart around parsers but it seems to be holding up well so far.

The defineType call in copyType still seems a bit dodgy, but I'm not sure why this function exists / when its exactly used yet.

cc @kLabz in case they want to make sure I've not misunderstood that haxe PRs purpose.

[Aidan63]

Tests for this would be nice (the existing functionality ones pass but the CI is knackered right now) but I'm not sure how I'd go about testing this language server stuff? Add some basic tests to check if two json parsers for the same type give instances of the same class instead of the old incrementing counter?

[Aidan63]

Upon further usage, some more work might be needed...

[Aidan63]

Having done some debugging into the above redefinition stuff it appears the persistent map is getting emptied for hover requests

function main() {
    final p1 = new JsonParser<Array<String>>();
    final p2 = new JsonParser<Array<String>>();
}

Hovering over p1 or p2 will result in the redefinition error. When the language server first starts up you see the initial definitions being made and the parser being re-used when it encounters it again.

Listening on port 127.0.0.1:6001
Haxe print:
D:/programming/haxe/json2object/src/json2object/reader/DataBuilder.macro.hx:959: parser for "Array<String>" does not exist

Haxe print:
D:/programming/haxe/json2object/src/json2object/reader/DataBuilder.macro.hx:940: parser for "Array<String>" already defined

Haxe print:
D:/programming/haxe/json2object/src/json2object/reader/DataBuilder.macro.hx:944: parsed still alive, reusing

Haxe print:
D:/programming/haxe/json2object/src/json2object/reader/DataBuilder.macro.hx:959: parser for "String" does not exist

Done.

However any hover requests will report that the parser was not found in the map and attempt to define another one, resulting in the redefinition.

Haxe print:
D:/programming/haxe/json2object/src/json2object/reader/DataBuilder.macro.hx:959: parser for "Array<String>" does not exist

[Error - 10:45:12 AM] Request textDocument/hover failed.
  Message: Type name JsonParser_fae276e53583ba7855144d0eb60db288 is redefined from module JsonParser_fae276e53583ba7855144d0eb60db288 (9e7fa2c0a6509dcc92664874ea06a99e)
  Code: -32603 

If I remove the map check and just keep the isAlive call then things seem to work fine. Any thoughts?

(all with lastest nightly and vscode)

[kLabz]

Checking if there's something wrong on compiler side here.. :thinking:

[kLabz]

Having done some debugging into the above redefinition stuff it appears the persistent map is getting emptied for hover requests

function main() {
    final p1 = new JsonParser<Array<String>>();
    final p2 = new JsonParser<Array<String>>();
}

Hovering over p1 or p2 will result in the redefinition error.

How exactly are you doing that? I cannot reproduce :/

[Aidan63]

I just tried again and its not happening for me now either... I'll check the proper project I first saw it in later on instead of just a small sample.

[kLabz]

I did spot some weird behavior, that went away by replacing isAlive's resolveType based approach by getType, which is a bit concerning.. both should work just fine.

[Aidan63]

I have noticed that if I have a static JsonParser and hover over it the type will show as Dynamic instead of the generated type, also occasionally on those static hovers I get nothing and a "cannot construct dynamic" message appears in the server output.

Have not mentioned it before as I'm not sure if thats related to this or something else, but I'll try and narrow that one down as well.

[kLabz]

Yeah, it's the Dynamic thing that's happening.

[kLabz]

Uhhhh.. makes sense actually:

https://github.com/HaxeFoundation/haxe/blob/development/src/typing/typeload.ml#L644-L648

https://github.com/HaxeFoundation/haxe/pull/11760

[Aidan63]

I gave that merge ago and seems to work well, also tried it out on another project I was seeing occasional Dynamics on hover results involving macros.

[kLabz]

Right now this PR doesn't work properly because it relies on resolveType which has issues during display requests. isAlive should be changed

Edit: PR has been merged on Haxe side so this ~~will~~ should work on future nightlies

[Aidan63]

Oops, forgot about this. When you said isAlive should be changed, was that in relation to needing a fix on the haxe side and so it should be ok now? Or is there still something wrong with it that needs fixing?