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

Only allow Void in return types #11558

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

Only allow Void in return types #11558

wants to merge 3 commits into from

Conversation

Simn
Copy link
Member

@Simn Simn commented Feb 7, 2024

I think we generally agree that we want to disallow Void for type parameters, but this by itself would be pointless if we still allowed typedef NotVoid = Void. This PR disallows loading Void for anything that's not a function return type.

Right now, it doesn't do this for externs yet because there are several places in js/html and hxnodejs that have Promise<Void>. I'm not sure what to do with that right now, but we'll have to figure this out before merging.

I'm deleting the test for #6810 because that depends on typedeffing Void. However, the issue can be closed because it will no longer be relevant.

Edit: I forgot to mention something: We're also adding haxe.NoValue here which can be used instead of Void.


Closes #3463
Closes #6810
Closes #11555

@EliteMasterEric
Copy link
Contributor

🎉 for haxe.NoValue!

@Simn
Copy link
Member Author

Simn commented Feb 7, 2024

I'm kind of lying about the "only in return types" part here because there's also still the Void -> ? situation, which doesn't even go through the typeloader (which is why this doesn't break here).

I'm not sure what to do with the old function syntax in general. The new one is objectively superior and I'm generally not a fan of having two ways to express the same thing. On the other hand, is it really worth breaking code over something so idealistic? I think it would take some sort of actual problem to justify this, and I don't remember if there are any.

@skial skial mentioned this pull request Feb 7, 2024
1 task
@EliteMasterEric
Copy link
Contributor

I'd say a major release is definitely as good a time as any to make a breaking change. Previous versions of Haxe will still exist, and if we never make this change, the quality and readability of Haxe code overall will suffer as a result.

@kLabz
Copy link
Contributor

kLabz commented Feb 7, 2024

I'd say Void -> Void is fine, and (_:Void) -> Void isn't
Lots of people have been using the former for 10+ years now, and it's not even technically wrong

@Simn
Copy link
Member Author

Simn commented Feb 7, 2024

That's debatable because Void -> ? is not the same as (Void) -> ?, and I think that's the only case where this is true.

@nadako
Copy link
Member

nadako commented Feb 7, 2024

Why not remove Void->T in Haxe 5 in favor of ()->T

@Simn
Copy link
Member Author

Simn commented Feb 7, 2024

Yes that's the question I'm asking, but in that case we surely should remove the old function type syntax entirely and not just this special case.

@Simn
Copy link
Member Author

Simn commented Feb 8, 2024

I'm being too hasty with that haxe.NoValue change because it's not actually a NoValue type, but rather a Unit type. Consider this:

class C<A> {
	public function new() {}

	public function invoke(a:A) {}
}

function main() {
	var c = new C<haxe.NoValue>();
	c.invoke(null /* here */);
}

In order to call the invoke function, we need to pass a value at run-time. In that light, I think we need to add a haxe.Unit type (instead?) and use it here.

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