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

Promote different types to dynamic? #98

Open
jishnub opened this issue Jan 8, 2023 · 7 comments
Open

Promote different types to dynamic? #98

jishnub opened this issue Jan 8, 2023 · 7 comments

Comments

@jishnub
Copy link
Contributor

jishnub commented Jan 8, 2023

julia> promote(static(0), static(1))
ERROR: promotion of types StaticInt{0} and StaticInt{1} failed to change any arguments

Perhaps these should return Ints?

@Tokazama
Copy link
Collaborator

Tokazama commented Jan 9, 2023

It seems like the appropriate thing to do would be return (static(0), static(1)), just the same as promote(0, 1) returning (0, 1).

@jishnub
Copy link
Contributor Author

jishnub commented Jan 9, 2023

Won't that defeat the idea of promoting to a common type?

@Tokazama
Copy link
Collaborator

Tokazama commented Jan 9, 2023

The difference in type here is more of an implementation detail. If we could have StaticInt without the actual number stored as the type we would.

@Tokazama
Copy link
Collaborator

Tokazama commented Jan 9, 2023

Perhaps we need a specialized promotion approach similar to Unitful.jl

@jishnub
Copy link
Contributor Author

jishnub commented Jan 10, 2023

promote is expected to "Convert all arguments to a common type", and I find patterns such as the following useful:

julia> struct A{T}
       x :: T
       y :: T
       end

julia> A(x...) = A(promote(x...)...)
A

julia> A(1,1)
A{Int64}(1, 1)

julia> A(1.0,2)
A{Float64}(1.0, 2.0)

Would be nice to have this accept static arguments as well, even if the result isn't static.

@chriselrod
Copy link
Collaborator

I think we should generally respect the interfaces of methods like convert or promote.
Ideally, the language would be able to enforce interfaces more broadly -- and allow users to add rules for their own functions.
But Base.convert has at least started doing this:

julia> convert(StaticInt{4}, 2)
ERROR: TypeError: in typeassert, expected StaticInt{4}, got a value of type StaticInt{2}
Stacktrace:
 [1] convert(#unused#::Type{StaticInt{4}}, x::Int64)
   @ Base ./number.jl:7
 [2] top-level scope
   @ REPL[4]:1

And I'd be on board with Base.promote following suite.

@Tokazama
Copy link
Collaborator

I understand the logic behind using promote this way but I still don't think this is what the original intention behind promote was. It was to make it easier to pass mismatched types to things like +. I'm not sure what the benefit of this is besides occasionally avoiding errors when there is not a proper overload of a method for static types. In such cases it may end up causing unintended behavior for users that's harder to pinpoint the root cause.

I just want to make sure we aren't changing this behavior just for a quick fix

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

No branches or pull requests

3 participants