haxe icon indicating copy to clipboard operation
haxe copied to clipboard

Correct EnumValueMap implementation

Open RealyUniqueName opened this issue 4 years ago • 4 comments

Fixes #2479

This is an implementation with correct EnumValueMap behavior for non-scalar enum parameters.

Target-specific implementations could be added as a next step to improve runtime performance.

RealyUniqueName avatar Jun 30 '20 16:06 RealyUniqueName

Something fails on C#:

Command: haxe [compile-2.hxml,-D,fast_cast]
haxelib run hxcs hxcs_build.txt --haxe-version 4300 --feature-level 1
haxelib run hxcs hxcs_build.txt --haxe-version 4300 --feature-level 1
haxelib run hxcs hxcs_build.txt --haxe-version 4300 --feature-level 1 --out bin/main/bin/Main
src\haxe\ds\EnumValueMap.cs(48,38): error CS0029: Cannot implicitly convert type 'haxe.root.ValueType [d:\a\haxe\haxe\tests\misc\cs\csTwoLibs\bin\main\bin\haxeboot.dll]' to 'haxe.root.ValueType [d:\a\haxe\haxe\tests\misc\cs\csTwoLibs\bin\main\src\Type.cs(5)]'
src\Type.cs(5,15): (Location of symbol related to previous error)
src\haxe\ds\EnumValueMap.cs(49,39): error CS0029: Cannot implicitly convert type 'haxe.root.ValueType [d:\a\haxe\haxe\tests\misc\cs\csTwoLibs\bin\main\bin\haxeboot.dll]' to 'haxe.root.ValueType [d:\a\haxe\haxe\tests\misc\cs\csTwoLibs\bin\main\src\Type.cs(5)]'
src\Type.cs(5,15): (Location of symbol related to previous error)
Command exited with 1 in 5s: haxe [compile-2.hxml,-D,fast_cast]
test cs failed

Simn avatar Apr 11 '22 12:04 Simn

I have updated the branch. It doesn't have https://github.com/HaxeFoundation/haxe/commit/7dc9272865afe5f4e1b5347da75c6d2700c67c99, which I would like to solve in a cross-platform way because I have the same issue on the JVM target. I think we can add Type.numEnumParameters to deal with this.

Edit: Actually that won't do because then we still have to create real arrays in order to check the individual elements. Not sure what a good API would look like here...

Simn avatar Feb 09 '24 09:02 Simn

I've added the HL optimization back so that we can merge this. An improved API is blocked on #11568 which may or may not take a while longer to address.

I'm also not sure if this actually closes #2479 because the tests added here are pretty barebones, but I'll take Alex's word for it!

Simn avatar Feb 09 '24 10:02 Simn

... I never really looked at the actual implementation here though. Building a map on top of arrays can be done, but linear cost for both read and write isn't acceptable for maps.

Simn avatar Feb 09 '24 11:02 Simn