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

Incorrect type widening with multiple arguments to generic T with new solver #1293

Open
Ketasaja opened this issue Jun 11, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@Ketasaja
Copy link
Contributor

Ketasaja commented Jun 11, 2024

Tested on v0.629, with no errors in strict or non-strict mode:

--!strict
local t: {string} = {}
table.insert(t, true)
@Ketasaja Ketasaja added the bug Something isn't working label Jun 11, 2024
@aatxe
Copy link
Collaborator

aatxe commented Jun 11, 2024

I'm not sure that I'd necessarily call this a bug. There does exist a type to provide for t such that this typechecks: number | boolean.

@Ketasaja
Copy link
Contributor Author

Ketasaja commented Jun 11, 2024

I'm not sure that I'd necessarily call this a bug. There does exist a type to provide for t such that this typechecks: number | boolean.

You're right, I don't have label changing permissions though.

My usecase is that the ECS library I use has world:set(id, Position, Vector3.new(0, 0, 0)), where Position is ecr.component() :: Vector3.

The advantage over function overloads is that there's no need to define components upfront, which makes it easy for other libraries to define them.

Maybe if other users have similar usecases it could be worth considering delaying this inference change in strict mode if generic constraints are a long way away?

@Ketasaja
Copy link
Contributor Author

More common example:

--!strict
local t: {string} = {}
table.insert(t, 1)

@aatxe
Copy link
Collaborator

aatxe commented Jun 11, 2024

I don't think there's an immediately obvious way for us to "delay" this change, but if the type of one of the fields is already required to be some particular type (e.g. in that table.insert example, or if your Position is cast to Vector3 or annotated Vector3), then I don't think we should be widening the type to include additional components. It's really only in the case of examples like your original one where I think the behavior is expected. So, there is probably still a bug here, but one more specific than your initial report.

@Ketasaja Ketasaja changed the title No concrete type equality checks when multiple fill same generic T with new solver Incorrect type widening with multiple arguments to generic T Jun 11, 2024
@Ketasaja Ketasaja changed the title Incorrect type widening with multiple arguments to generic T Incorrect type widening with multiple arguments to generic T with new solver Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

2 participants