Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid creating TAnons from object declarations if we have an expected type #11474

Open
wants to merge 5 commits into
base: development
Choose a base branch
from

Conversation

Simn
Copy link
Member

@Simn Simn commented Jan 8, 2024

I saw in the hxb writer that we were dealing with hundreds of TAnons that should really be haxe.PosInfos. Anons are difficult to handle for a binary interface because we have to identify them properly, and doing so is not particularly pleasant

A large portion of them come from object declaration typing against an expected type, which results in something like [ObjectDecl:{ methodName : String, lineNumber : Int, fileName : String, className : String }] in the typed AST. With this PR, it emits [ObjectDecl:haxe.PosInfos] instead.

This cuts the amount of generated anons in our unit tests roughly in half, from 1700 to 800. It's still a rather high number that I want to investigate, but I think some of it comes from macro reification not creating objects against an explicit type.

Right now this doesn't quite work yet on HL:

 ERROR  src/unit/TestBasetypes.hx:214: characters 38-44

 214 |   var k = Lambda.array( { iterator : h.keys } );
     |                                      ^^^^^^
     | Don't know how to cast ():virtual(hasNext:method:():bool,next:method:():String) to method:():virtual(hasNext:method:():bool,next:method:():String)

The dump diff looks like what I'd expect:

Merge_dhWGkBMg1z

I don't know why genhl is offended by that because its to_type seems to be recursing upon typedefs, so I must be missing something else here.

@skial skial mentioned this pull request Jan 9, 2024
1 task
@Simn
Copy link
Member Author

Simn commented Jul 26, 2024

@yuxiaomao Could you look into the HL failure some time?

@Simn
Copy link
Member Author

Simn commented Jul 26, 2024

I'm avoiding the issue for now, but this isn't a good solution. Yuxiao identified the core problem which we can reproduce from this simple code:

function array(it:Iterable<String>) {}

function main() {
	array({iterator: null});
}

The issue is that the TObjectDecl is now typed as Iterable, and its function iterator is a Method while with pure anons it would be a Var. The HL generator tries to set the object field, but that doesn't work if the field comes out as HMethod.

I don't know what the correct solution here is though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants