-
-
Notifications
You must be signed in to change notification settings - Fork 646
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
[jvm] Functional Interfaces not extending proper interface when interface has type parameters #11390
Comments
I think you forgot to update the Main.hx, I'm still getting the old output from that repo. |
Didn't see your reply here, ehe. I did not update Main.hx for this issue, only the |
Did you get around to testing out this example more? It should reliably produce the error. |
I can't check right now, but when I initially tried to reproduce this I got exactly the same output as before, which is why I thought you forgot to update something. I'll check it again on Monday. |
I checked this out today (I never said which Monday I meant!) and fixed something not strictly related, but I still cannot reproduce the problem if
|
I will have to attempt a working reproduction for this. I don't reproduce the issue in the minimal repro either but I am experiencing an issue where the proper interface is not extended (but a bunch of other interfaces are) even when the parameter is properly typed. |
@Simn I have found a minimal reproduction for the issue. https://github.com/EliteMasterEric/Issue11390
The issue presents itself when the functional interface has a type parameter, see |
You and your robots...
|
There was a cascade of problems here:
The unification problems involved here make me question my approach to this as a whole. However, we can't rely on the assignments detected in the typer because the actual closure in question might not even be seen there. It then seems necessary to compare all closure classes against all interface candidates. Perhaps these checks could be done at Haxe-level to utilize our unification mechanisms, but this could lead to differences with how the JVM expects assignments to work too... |
Template project works now, but the use case I was trying to solve still doesn't. Back to the robot factory I guess. EDIT: For the record, this is the function I'm trying to call but failing at: https://github.com/FabricMC/fabric/blob/65c120eded0fb3d65c10cd2a616e38e22746963d/fabric-data-generation-api-v1/src/main/java/net/fabricmc/fabric/api/datagen/v1/FabricDataGenerator.java#L129 |
I don't think this should be reopened since an issue was fixed. I'll make a new one once I have a reproduction. Not easy to make a minimal repro, since I tried to make an example with just the Fabric JAR that has the problem class and Haxe complained that it needs Minecraft as a dependency. |
I figured it out. The functional interface took an argument and expected a return result of I was doing that, but if I don't explicitly define the return type of the functional interface as a Here's a reproduction. Reopening this rather than making a new issue since it DOES work as long as everything is explicitly defined. |
Yeah, I'd like to make this more robust. The problem right now is that the Haxe typer checks for assignment (unification) while the functional interface detector in genjvm checks for (mostly) equality. That's why cases like this are admitted by the typer, but then missed by genjvm. Not an ideal situation... If type relations have to be taken into account, we're pretty much forced to do these checks at Haxe-level because we don't know anything about relationships at JVM-signature-level. |
I can't even javac your example though:
|
Try if the linked commit does anything for you. I'm not very confident in our test suite regarding this, so there's a good chance that it blows up. |
I'm on
|
* use identity for field params * support partial generic expansion for fields * take off @:generic * try something for enum constructor type params * gencommon says no * communicate field type parameters by index again * surely anons are just a list of anon field refs * fix generic type param names * let's get dangerous * handle CBOs in HxbRestore too * well * chdir * handle warnings as CBOs * don't try to hxb import.hx because there's nothing in there * don't roundtrip on Cross and Eval way too many false positives in tests from printing * add CFLR, don't resolve fields like a crazy person * fix but it's still broken * stop the roundtrip nonsense for now, enough other problems * decode binary modules in check_display_file * remove flush_fields, should not be needed with CFLR * maybe fix generic type parameter naming * add ENFR too * pull stricter tanon changes and remove some debug * unused open * fix overloads a bit more but it's still not 100% working * fix generic type param naming a bit more * check for cf_overloads instead of CfOverloads The flag isn't set for ghetto overloads, so this should catch all relevant cases. * fix overloads even more * add enum when adding enum field ref * fix overloads for real * small optimization for the VERY common depth = 0 case * properly initialize typedef monos * map anons before identifying them * write tpp paths instead of names * remove some debuggery from the writer * embrace ttp * don't follow away Null<Void> in function returns * encode basic types directly I'm not sure if I want to keep this optimization, but the changes to com.basic seem like a good idea regardless. At the moment it might be possible to get a hold of the ominous `m` monomorph in Common.create, and binding that one to anything would lead to interesting results. * add ANFR for anon fields * Happy new year! * [typer] fix functional interface type parameter leak see #11390 * ANFR earlier * only write cf_expr_unoptimized if it differs from cf_expr * write texpr positions are bit smarter * add field contexts, hashcons expression type instances * [display] populate server/modules from hxb since there's only that atm * remove cf_expr_unoptimized optimization again because the hashconsing has already broken it We need a different solution for this silly field because it takes up a lot of space and time for very little gain. Maybe something like a binary diff to cf_expr would make sense here. * forward declare locals in texpr way less Hashtbling * be less awkward around ttps They're your friend! * purge some debug from the reader We never want to write to stderr because that hangs the display tests. * don't generate TLazy and remove random bool Also add comment about empty anons * make api an argument of the read function This is a prerequisite for making the reader reentrant, which should now be possible. * demo how continuations could work * reorder tables to something that makes sense to me * separate chunk classes more * add cl_type to avoid hundreds of ClassStatics anons * don't set anon fields too early * properly deal with ClassStatics in unification and field typing also convert unify_anons to something human-readable * bring back missing checks * add small ring cache per type kind * add more elaborate stats behind -D hxb.stats * move expr stuff out of the way because I keep scrolling past it * change mono byte to 1, avoid extra 1 byte for immediate type instance values * encode simple type instances immediately * potentially avoid some GC write barriers by splitting up pos * use less ignorant fast_eq * warnings * basic reader stats * minor cleanup * fix class field scoping * infer nested status from stack Did you know there can be anon fields inside enum fields? Crazy... * field type parameters 2.0 * put all texpr type instances in an array * change -bcp to --hxb-lib * create directory before creating file... * format stats output nicer because that's very important * rename hxml so I find it easier * encode Void directly because it actually appears a lot * reorganize type instance bytes, inline common list lengths * warnings * write blocks less awkwardly also don't read arrays only to turn them into lists * don't write i32 if it can be helped * pass current chunk to pos writer * add texpr stats * optimize TCall * optimize static calls * refactor pos writer slightly * optimize this.field * fix generator printing * enable disabled tests * remove overly fancy position optimization * write TInt as leb too * Revert "write TInt as leb too" This reverts commit ce6c8de. * let's see where we're at * check context before checking cache for display files see #11480 * stop leaving debug prints in your commits ffs * adjust test Calling exclude before saving means that the class is extern on the next iteration, and thus not generated. * dodge arcane sourcemap test problem * dodge more * change chunk to module in the writer * use SimnBuffer * remove roundtrip * avoid some object overhead in the reader * use correct v_id when reading from .hxb * [hxb] use cache hxb for --hxb too * don't generate aux output (including hxb) during diagnostics * warnings * only cache context when using compilation server * write chunk name before length, and remove crc The 4 bytes of the name don't contribute to the length, so this makes more sense. * minor refactor to make interface a bit saner * skip hxb generation on display too * warnings * move module_cache to HxbData That might not be the ideal place for it either but it's better than tType.ml * rework reader interface a bit as well * read metadata in forward data see go2hx/go2hx#174 * read anon field meta too * persist ordered list of chunks instead of one big bytes thing * remove feature nonsense and revert meta change * warnings * fix server/module, get all data from hxb cache directly * align chunk order * [hxb] don't overwrite taint reason with check_display_file * [server] we're sending cacheState, not 'dirty' * [tests] update test * [hxb] generate proper warnings for unbound type parameters * Simplify unbound ttp positions; not like they're often useful anyway... * ocaml syntax is hard * anon fields can have @:overload too... * use singular empty anon * slightly improve field vs. overloads handling * remove some debug because hxb is perfect * [TYPF] skip module name, only write pack when different from current module (private types) * no need to complicate things * [tests] add test for #11480 * add CFEX, but don't generate anything into it yet * move local type params to expressions * don't write type parameter length twice * write ttp host * write nested status so the reader doesn't have to track this * more CFEX work * rename chunks * move all field references in front of definitions There's currently no data dependency which requires this, but it's more future-proof in case there's ever something along the lines of dependent types. * rename chunk reader functions too * sanity catch Error when resolving types * more sanity * forward declare module type parameters in MTF We need to know their identity early so the instance builder can work. Constraints and defaults are set later. * fix overload handling again * remove manual cl_build call, add enable_field_access to api see #11493 * add marker chunks, read hxb modules delayed see #11493 * work around java.Init issue so we can run JVM roundtrip on CI see #11493 * fail nicer if we can't write the archive * write cl_flags in MTF We want to know early if a class is an interface * remove enable_field_access, rely on typing passes instead see #11493 * only load a native lib once we actually need it * generate extern modules too * activate EXD, handle field type parameters awfully for now * don't write empty chunks * reduce diff against development * bucket class fields by their name to avoid long linear lookups * remove rings Not worth it anymore because the type instance simple/not_simple distinction is fast enough. * comment out stats writes because this might be an observable overhead * time writer per-module so we can find pathological modules * lose some more byte-level OOP overhead * remove some debug noise * merge IOChunk and Chunk, remove string_pool * no comment * simplify type instance writing again Reuse a single chunk that we reset when writing a texpr type instance. * add custom StringPool implementation * optimize locals a bit more * maybe deal with duplicate var declaration * revert local changes * try local optimization again * invert texpr order: kind, type, pos This will allow us to omit some type instances in case they can be inferred from the kind. * introduce implicit texpr type instances * walk back a little * don't double match type params * avoid some EXD reading * small cleanup * avoid some list reading for things that aren't lists * reverse field order and create both list and PMap at the same time * Generate dump even with --no-output * avoid write_list for anon fields too Folding a PMap to a list only to then get its length (which is a linear operation) and iterate over it isn't the power move that I thought it was. * write pos pairs where applicable * delay IO.input creation, add per-chunk timer * avoid IO in reader too * don't look if you want to sleep at night * less nightmares * remove abstract reader * do less in display requests CI exploding in 3... 2... 1... * start working on delayed expression reading see #11498 * Add hxb.stats to ignored defines for signature * put the dodge in place, activate see #11498 * adjust to cl_init changes * used hashed identity pool for anon fields too * less classes * less classes * no classes * avoid some pointless DynArray to List operations * more DynArray * add MDR to deal with import to @:keep * write CLR and friends after CFR CFR might add more CLR. Also add sanity checks to make sure we don't modify pools after exporting them. * actually install cf_type * change field type parameter storing to something more awkward and efficient * avoid some unnecessary byte blitting * fix HxbId insertion on fields without expression * Warnings * [server-hxb] only read expressions eagerly when full typing * [hxb] write type for TMeta * [hxb] whitespace nazi * tanon identification / tunification: stricterer EqStricter * Why was this debug even pushed.. * hacktoberfest * [display] server/type: only restore hxb headers * check if 3725 is still a problem * remove TODO comment * Remove TODOs * clean up unit hxmls * don't null-terminate bytes * [tests] add display test for static field completion Also run again a couple tests after caching type * die when writing out empty module or type name The reader would die anyway --------- Co-authored-by: Rudy Ges <k@klabz.org>
* use identity for field params * support partial generic expansion for fields * take off @:generic * try something for enum constructor type params * gencommon says no * communicate field type parameters by index again * surely anons are just a list of anon field refs * fix generic type param names * let's get dangerous * handle CBOs in HxbRestore too * well * chdir * handle warnings as CBOs * don't try to hxb import.hx because there's nothing in there * don't roundtrip on Cross and Eval way too many false positives in tests from printing * add CFLR, don't resolve fields like a crazy person * fix but it's still broken * stop the roundtrip nonsense for now, enough other problems * decode binary modules in check_display_file * remove flush_fields, should not be needed with CFLR * maybe fix generic type parameter naming * add ENFR too * pull stricter tanon changes and remove some debug * unused open * fix overloads a bit more but it's still not 100% working * fix generic type param naming a bit more * check for cf_overloads instead of CfOverloads The flag isn't set for ghetto overloads, so this should catch all relevant cases. * fix overloads even more * add enum when adding enum field ref * fix overloads for real * small optimization for the VERY common depth = 0 case * properly initialize typedef monos * map anons before identifying them * write tpp paths instead of names * remove some debuggery from the writer * embrace ttp * don't follow away Null<Void> in function returns * encode basic types directly I'm not sure if I want to keep this optimization, but the changes to com.basic seem like a good idea regardless. At the moment it might be possible to get a hold of the ominous `m` monomorph in Common.create, and binding that one to anything would lead to interesting results. * add ANFR for anon fields * Happy new year! * [typer] fix functional interface type parameter leak see HaxeFoundation#11390 * ANFR earlier * only write cf_expr_unoptimized if it differs from cf_expr * write texpr positions are bit smarter * add field contexts, hashcons expression type instances * [display] populate server/modules from hxb since there's only that atm * remove cf_expr_unoptimized optimization again because the hashconsing has already broken it We need a different solution for this silly field because it takes up a lot of space and time for very little gain. Maybe something like a binary diff to cf_expr would make sense here. * forward declare locals in texpr way less Hashtbling * be less awkward around ttps They're your friend! * purge some debug from the reader We never want to write to stderr because that hangs the display tests. * don't generate TLazy and remove random bool Also add comment about empty anons * make api an argument of the read function This is a prerequisite for making the reader reentrant, which should now be possible. * demo how continuations could work * reorder tables to something that makes sense to me * separate chunk classes more * add cl_type to avoid hundreds of ClassStatics anons * don't set anon fields too early * properly deal with ClassStatics in unification and field typing also convert unify_anons to something human-readable * bring back missing checks * add small ring cache per type kind * add more elaborate stats behind -D hxb.stats * move expr stuff out of the way because I keep scrolling past it * change mono byte to 1, avoid extra 1 byte for immediate type instance values * encode simple type instances immediately * potentially avoid some GC write barriers by splitting up pos * use less ignorant fast_eq * warnings * basic reader stats * minor cleanup * fix class field scoping * infer nested status from stack Did you know there can be anon fields inside enum fields? Crazy... * field type parameters 2.0 * put all texpr type instances in an array * change -bcp to --hxb-lib * create directory before creating file... * format stats output nicer because that's very important * rename hxml so I find it easier * encode Void directly because it actually appears a lot * reorganize type instance bytes, inline common list lengths * warnings * write blocks less awkwardly also don't read arrays only to turn them into lists * don't write i32 if it can be helped * pass current chunk to pos writer * add texpr stats * optimize TCall * optimize static calls * refactor pos writer slightly * optimize this.field * fix generator printing * enable disabled tests * remove overly fancy position optimization * write TInt as leb too * Revert "write TInt as leb too" This reverts commit ce6c8de. * let's see where we're at * check context before checking cache for display files see HaxeFoundation#11480 * stop leaving debug prints in your commits ffs * adjust test Calling exclude before saving means that the class is extern on the next iteration, and thus not generated. * dodge arcane sourcemap test problem * dodge more * change chunk to module in the writer * use SimnBuffer * remove roundtrip * avoid some object overhead in the reader * use correct v_id when reading from .hxb * [hxb] use cache hxb for --hxb too * don't generate aux output (including hxb) during diagnostics * warnings * only cache context when using compilation server * write chunk name before length, and remove crc The 4 bytes of the name don't contribute to the length, so this makes more sense. * minor refactor to make interface a bit saner * skip hxb generation on display too * warnings * move module_cache to HxbData That might not be the ideal place for it either but it's better than tType.ml * rework reader interface a bit as well * read metadata in forward data see go2hx/go2hx#174 * read anon field meta too * persist ordered list of chunks instead of one big bytes thing * remove feature nonsense and revert meta change * warnings * fix server/module, get all data from hxb cache directly * align chunk order * [hxb] don't overwrite taint reason with check_display_file * [server] we're sending cacheState, not 'dirty' * [tests] update test * [hxb] generate proper warnings for unbound type parameters * Simplify unbound ttp positions; not like they're often useful anyway... * ocaml syntax is hard * anon fields can have @:overload too... * use singular empty anon * slightly improve field vs. overloads handling * remove some debug because hxb is perfect * [TYPF] skip module name, only write pack when different from current module (private types) * no need to complicate things * [tests] add test for HaxeFoundation#11480 * add CFEX, but don't generate anything into it yet * move local type params to expressions * don't write type parameter length twice * write ttp host * write nested status so the reader doesn't have to track this * more CFEX work * rename chunks * move all field references in front of definitions There's currently no data dependency which requires this, but it's more future-proof in case there's ever something along the lines of dependent types. * rename chunk reader functions too * sanity catch Error when resolving types * more sanity * forward declare module type parameters in MTF We need to know their identity early so the instance builder can work. Constraints and defaults are set later. * fix overload handling again * remove manual cl_build call, add enable_field_access to api see HaxeFoundation#11493 * add marker chunks, read hxb modules delayed see HaxeFoundation#11493 * work around java.Init issue so we can run JVM roundtrip on CI see HaxeFoundation#11493 * fail nicer if we can't write the archive * write cl_flags in MTF We want to know early if a class is an interface * remove enable_field_access, rely on typing passes instead see HaxeFoundation#11493 * only load a native lib once we actually need it * generate extern modules too * activate EXD, handle field type parameters awfully for now * don't write empty chunks * reduce diff against development * bucket class fields by their name to avoid long linear lookups * remove rings Not worth it anymore because the type instance simple/not_simple distinction is fast enough. * comment out stats writes because this might be an observable overhead * time writer per-module so we can find pathological modules * lose some more byte-level OOP overhead * remove some debug noise * merge IOChunk and Chunk, remove string_pool * no comment * simplify type instance writing again Reuse a single chunk that we reset when writing a texpr type instance. * add custom StringPool implementation * optimize locals a bit more * maybe deal with duplicate var declaration * revert local changes * try local optimization again * invert texpr order: kind, type, pos This will allow us to omit some type instances in case they can be inferred from the kind. * introduce implicit texpr type instances * walk back a little * don't double match type params * avoid some EXD reading * small cleanup * avoid some list reading for things that aren't lists * reverse field order and create both list and PMap at the same time * Generate dump even with --no-output * avoid write_list for anon fields too Folding a PMap to a list only to then get its length (which is a linear operation) and iterate over it isn't the power move that I thought it was. * write pos pairs where applicable * delay IO.input creation, add per-chunk timer * avoid IO in reader too * don't look if you want to sleep at night * less nightmares * remove abstract reader * do less in display requests CI exploding in 3... 2... 1... * start working on delayed expression reading see HaxeFoundation#11498 * Add hxb.stats to ignored defines for signature * put the dodge in place, activate see HaxeFoundation#11498 * adjust to cl_init changes * used hashed identity pool for anon fields too * less classes * less classes * no classes * avoid some pointless DynArray to List operations * more DynArray * add MDR to deal with import to @:keep * write CLR and friends after CFR CFR might add more CLR. Also add sanity checks to make sure we don't modify pools after exporting them. * actually install cf_type * change field type parameter storing to something more awkward and efficient * avoid some unnecessary byte blitting * fix HxbId insertion on fields without expression * Warnings * [server-hxb] only read expressions eagerly when full typing * [hxb] write type for TMeta * [hxb] whitespace nazi * tanon identification / tunification: stricterer EqStricter * Why was this debug even pushed.. * hacktoberfest * [display] server/type: only restore hxb headers * check if 3725 is still a problem * remove TODO comment * Remove TODOs * clean up unit hxmls * don't null-terminate bytes * [tests] add display test for static field completion Also run again a couple tests after caching type * die when writing out empty module or type name The reader would die anyway --------- Co-authored-by: Rudy Ges <k@klabz.org>
Minimal reproduction
https://github.com/EliteMasterEric/Issue11054/tree/488fbb13733643d5406fbcae42802b07d72652e0
Error Message
No compile error is received, instead Java experiences an error at runtime:
Notes
This is a similar case to #11054, which occurs in Haxe
5c05e6d
. Since the main test case of #11054 is resolved, this is not a regression, so I am making a new issue for ease of tracking.The text was updated successfully, but these errors were encountered: