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.

Sign up for GitHub

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

Add promote_rule methods instead of promote_type #56779

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Add promote_rule methods instead of promote_type #56779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
37b70c8
Add more `promote_rule` methods instead of `promote_type`
jishnub Dec 8, 2024
2e13d83
Add a comment and backticks in error
jishnub Dec 8, 2024
52e5024
Add test for conflict
jishnub Dec 8, 2024
30d1239
Test for identical type promotion
jishnub Dec 8, 2024
8c385a8
Update comment in `promote_type` method
jishnub Dec 8, 2024
800b9d4
Update comment above `promote_result`
jishnub Dec 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions base/promotion.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,18 +302,12 @@ promote_type(T) = T
promote_type(T, S, U) = (@inline; promote_type(promote_type(T, S), U))
promote_type(T, S, U, V...) = (@inline; afoldl(promote_type, promote_type(T, S, U), V...))

promote_type(::Type{Bottom}, ::Type{Bottom}) = Bottom
promote_type(::Type{T}, ::Type{T}) where {T} = T
promote_type(::Type{T}, ::Type{Bottom}) where {T} = T
promote_type(::Type{Bottom}, ::Type{T}) where {T} = T

function promote_type(::Type{T}, ::Type{S}) where {T,S}
@inline
# Try promote_rule in both orders. Typically only one is defined,
# and there is a fallback returning Bottom below, so the common case is
# promote_type(T, S) =>
# promote_result(T, S, result, Bottom) =>
# typejoin(result, Bottom) => result
# promote_result(T, S, result, Bottom) => result
promote_result(T, S, promote_rule(T,S), promote_rule(S,T))
end

Expand All @@ -326,15 +320,34 @@ it for new types as appropriate.
"""
function promote_rule end

promote_rule(::Type, ::Type) = Bottom
# add a level of indirection to avoid method ambiguities
promote_rule(T::Type, S::Type) = _promote_rule(T, S)
_promote_rule(::Type, ::Type) = Bottom
_promote_rule(::Type{T}, ::Type{T}) where {T} = T
# Define some methods to avoid needing to enumerate unrelated possibilities when presented
# with Type{<:T}, and return a value in general accordance with the result given by promote_type
promote_rule(::Type{Bottom}, slurp...) = Bottom
promote_rule(::Type{Bottom}, ::Type{Bottom}, slurp...) = Bottom # not strictly necessary, since the next method would match unambiguously anyways
promote_rule(::Type{Bottom}, ::Type{T}, slurp...) where {T} = T
promote_rule(::Type{T}, ::Type{Bottom}, slurp...) where {T} = T

# if both the arguments are identical, or if both the orderings in promote_rule
# are defined to return identical results, we may return the result directly
promote_result(::Type{T},::Type{T},::Type{T},::Type{T}) where {T} = T
promote_result(::Type,::Type,::Type{T},::Type{T}) where {T} = T
# If only one promote_rule is defined, use the definition directly
promote_result(::Type,::Type,::Type{T},::Type{Bottom}) where {T} = T
promote_result(::Type,::Type,::Type{Bottom},::Type{T}) where {T} = T
# if multiple promote_rules are defined, try to promote the results
promote_result(::Type,::Type,::Type{T},::Type{S}) where {T,S} = (@inline; promote_type(T,S))
# avoid recursion if the types don't change under promote_rule, and throw an informative error instead
function _throw_promote_type_fail(A::Type, B::Type)
throw(ArgumentError(LazyString("`promote_type(", A, ", ", B, ")` failed as conflicting `promote_rule`s were ",
"detected with either argument being a possible result.")))
end
promote_result(::Type{T},::Type{S},::Type{T},::Type{S}) where {T,S} = _throw_promote_type_fail(T, S)
promote_result(::Type{T},::Type{S},::Type{S},::Type{T}) where {T,S} = _throw_promote_type_fail(T, S)

Comment on lines +343 to +350
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, this these lines are unrelated to the rest of the change, and improve the error message for promote_rule mistakes (explicit error instead of a stack overflow crash), so these can be merged separately from the other controversial changes?

# If no promote_rule is defined, both directions give Bottom. In that
# case use typejoin on the original types instead.
promote_result(::Type{T},::Type{S},::Type{Bottom},::Type{Bottom}) where {T,S} = (@inline; typejoin(T, S))
Expand Down
16 changes: 16 additions & 0 deletions test/core.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8389,3 +8389,19 @@ f_call_me() = invoke(f_invoke_me, f_invoke_me_ci)
f_invalidate_me() = 2
@test_throws ErrorException invoke(f_invoke_me, f_invoke_me_ci)
@test_throws ErrorException f_call_me()

@testset "promote_rule and promote_result overloads" begin
struct PromoteA end
struct PromoteB end

@testset "error with conflicting promote_rules" begin
Base.promote_rule(::Type{PromoteA}, ::Type{PromoteB}) = PromoteA
Base.promote_rule(::Type{PromoteB}, ::Type{PromoteA}) = PromoteB
@test_throws ArgumentError promote_type(PromoteA, PromoteB)
end

@testset "promote_rule overload for identical types" begin
Base.promote_rule(::Type{PromoteA}, ::Type{PromoteA}) = PromoteB
@test promote_type(PromoteA, PromoteA) == PromoteB
end
end
Loading