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

[cs] Pointer detection broken #11527

Closed
Simn opened this issue Jan 31, 2024 · 7 comments · Fixed by #11551
Closed

[cs] Pointer detection broken #11527

Simn opened this issue Jan 31, 2024 · 7 comments · Fixed by #11551
Labels
platform-cs Everything related to c#

Comments

@Simn
Copy link
Member

Simn commented Jan 31, 2024

9e7a820 broke the C# pointer detection with some changes like this in the dump:

< 	[Cast:cs.PointerAccess<unit._TestCSharp.SomeStruct>] [Local addr(45777):cs.Pointer<unit._TestCSharp.SomeStruct>:cs.Pointer<unit._TestCSharp.SomeStruct>]
---
> 	[Cast:unit._TestCSharp.SomeStruct] [Cast:cs.PointerAccess<unit._TestCSharp.SomeStruct>] [Local addr(45777):cs.Pointer<unit._TestCSharp.SomeStruct>:cs.Pointer<unit._TestCSharp.SomeStruct>]

PointerAccess is a forwarding abstract:

@:forward abstract PointerAccess<T>(T) {}

The C# generator follows it away to look for the cs.Pointer type, but with the new dump the Cast:unit._TestCSharp.SomeStruct now gets in the way of that detection.

The problem here is that the new typing is more correct: We cast the abstract to the underlying type, then access its field. I'm not even sure why it wasn't like that before because the { e with etype = t } should also turn the expression into the underlying type.

@Simn Simn added the platform-cs Everything related to c# label Jan 31, 2024
@Simn
Copy link
Member Author

Simn commented Jan 31, 2024

Addressed in e2e359b. There's now @:forward.accessOnAbstract which generates the field access on the abstract instead of the underlying type. I think the way C# handles this is quite clever, so I'd like to still support it. It would be nice to have a more general way to "tag" field access, but I can't really think of anything.

@Simn Simn closed this as completed Jan 31, 2024
@Simn
Copy link
Member Author

Simn commented Jan 31, 2024

C# is still broken, but of course only on Linux and Mac because that seems to be this month's theme.

@Simn Simn reopened this Jan 31, 2024
@Simn
Copy link
Member Author

Simn commented Jan 31, 2024

< 			return ( (global::haxe.lang.StringExt.charCodeAt(global::haxe.lang.Runtime.toString(global::haxe.lang.Runtime.getField(a, "name", 1224700491, true)), 0)).@value < (global::haxe.lang.StringExt.charCodeAt(global::haxe.lang.Runtime.toString(global::haxe.lang.Runtime.getField(b, "name", 1224700491, true)), 0)).@value );
---
> 			return ( (global::haxe.lang.StringExt.charCodeAt(global::haxe.lang.Runtime.toString(global::haxe.lang.Runtime.getField(((object) (a) ), "name", 1224700491, true)), 0)).@value < (global::haxe.lang.StringExt.charCodeAt(global::haxe.lang.Runtime.toString(global::haxe.lang.Runtime.getField(((object) (b) ), "name", 1224700491, true)), 0)).@value );

So it adds ((object) (a) ) casts. While that's certainly weird, I can't immediately tell why it causes a syntax error:

/home/runner/work/haxe/haxe/tests/unit/src/unit/issues/Issue9123.hx(8,220): error CS1031: Type expected
/home/runner/work/haxe/haxe/tests/unit/src/unit/issues/Issue9123.hx(8,254): error CS1525: Unexpected symbol `)', expecting `;'

@kLabz
Copy link
Contributor

kLabz commented Jan 31, 2024

Kinda looks like #7200 ?

@Simn
Copy link
Member Author

Simn commented Jan 31, 2024

I tried dodging the cast altogether if the types are equal, but there's some insanity going on with >>> generation that causes loads of Cannot implicitly convert type 'long' to 'int'. An explicit conversion exists (are you missing a cast?) errors.

Simn added a commit that referenced this issue Jan 31, 2024
Simn added a commit that referenced this issue Jan 31, 2024
@Simn
Copy link
Member Author

Simn commented Jan 31, 2024

Next problem: The compiler comes across this crap:

class Task_1<T0> {
    static public var Factory(get,never) : cs.system.threading.tasks.TaskFactory_1<T0>;
}

This fails since c5f913c. I've hacked it so that the type parameter lookup is allowed for @:libType classes, but that's a terrible workaround. A better workaround will be the removal of the C# target.

@Simn
Copy link
Member Author

Simn commented Jan 31, 2024

Well, I guess I'll close here again because everything is "fixed".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform-cs Everything related to c#
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants