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

Base.isconcretetype incorrect for Type{T}? #54646

Closed
MilesCranmer opened this issue May 31, 2024 · 2 comments
Closed

Base.isconcretetype incorrect for Type{T}? #54646

MilesCranmer opened this issue May 31, 2024 · 2 comments

Comments

@MilesCranmer
Copy link
Member

MilesCranmer commented May 31, 2024

I saw the following weird behaviour on https://discourse.julialang.org/t/ann-dispatchdoctor-jl-offers-you-a-prescription-for-type-stability/114837/35

julia> Base.isconcretetype(Type{Float32})
false

I think this could be a bug? The definition states:

Determine whether type T is a concrete type, meaning it could have direct instances (values x such that typeof(x) === T).

However, typeof(T) returns a DataType rather than Type{Float32}. But the docstring for DataType says DataType <: Type{T}. So I'm confused.

If this indeed a bug I think it's because the @nospecialize here was implemented without expecting such an input:

isconcretetype(@nospecialize(t)) = (@_total_meta; isa(t, DataType) && (t.flags & 0x0002) == 0x0002)

The solution would be to add a

isconcretetype(::Type{Type{T}}) where {T} = isconcretetype(T)

whose recursion would automatically deal with Type{Type{Float32}} and so on.

But obviously this would be a massive breaking change so wouldn't be possible. So I guess I'm just looking for some clarification :)

@MilesCranmer MilesCranmer changed the title Base.isconcretetype is incorrect for Type{T} Base.isconcretetype incorrect for Type{T}? May 31, 2024
@martinholters
Copy link
Member

The subtyping relationship is Type{Float32} <: DataType <: Type, and as you've discovered yourself, typeof(Float32)==DataType, so neither Type{Float32} nor Type are concrete, but DataType is.

What's confusing indeed is the DataType <: Type{T} in the docstring. While DataType <: Type{T} where T is true (and just a more verbose form of DataType <: Type), for any actual T, DataType <: Type{T} does not hold.

@MilesCranmer
Copy link
Member Author

Got it, thanks.

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

2 participants