Skip to content

Conversation

odow
Copy link
Member

@odow odow commented Nov 30, 2021

The existing method was pretty terrible for inference.

@blegat
Copy link
Member

blegat commented Nov 30, 2021

So if I understand correctly:

Base.promote_rule(::Type{F}, ::Type{T}) where {T,F<:TypedScalarLike{T}} = F

is bad,

Base.promote_rule(::Type{F}, ::Type{T}) where {T,F<:MOI.ScalarAffineFunction{T}} = F

is bad but

Base.promote_rule(::Type{MOI.ScalarAffineFunction{T}}, ::Type{T}) where {T} = MOI.ScalarAffineFunction{T}

is good ?

@odow
Copy link
Member Author

odow commented Nov 30, 2021

The underlying issue is

julia> Union{} <: MOI.Utilities.TypedScalarLike{Float64}
true

which conflicts with

julia> @which promote_type(Union{}, Float64)
promote_type(::Type{Union{}}, ::Type{T}) where T in Base at promotion.jl:224

@odow
Copy link
Member Author

odow commented Nov 30, 2021

Why do we even need these methods? Coverage didn't hit them.

@blegat
Copy link
Member

blegat commented Nov 30, 2021

I see, I didn't think about Union{} ^^

Why do we even need these methods? Coverage didn't hit them.

Maybe because they are inlined ? I don't remember why they were added though.

@odow
Copy link
Member Author

odow commented Nov 30, 2021

Let's see if anything breaks.

@odow
Copy link
Member Author

odow commented Nov 30, 2021

I guess that's a false negative on the codecov then!

@odow odow merged commit e1b14f8 into master Dec 1, 2021
@odow odow deleted the od/promote_rule branch December 1, 2021 02:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants