-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Inference regression in julia 1.7 #43296
Comments
Could you try bisecting this? |
@JeffBezanson bisected to e8caa27 |
MWE: r(b) = r(typeof(b))
r(::Type) = nothing
r(::Nothing) = nonexistent
struct C{t,I} end
r(::Type{C{c,d}}) where {c,d} = f(r(c), e)
f(::Nothing, :) = nothing
f(g, :) = h
k(b, j, :) = l
k(b, j, ::Nothing) = b
i(b, j) = k(b, j, r(j))
struct A{d} end
Base.return_types(i, (Int64, C{C{C{A, Tuple}, Tuple}})) |
This is now explicitly disallowed in inference:
Since it is inferring / calling:
And we decided (e8caa27) that this pattern is not something we care about, relative to other patterns. |
Does this mean this bug won't be fixed? |
This pattern can arise in the wild, when you combine trait based dispatch with recursion. This is the cause in Accessors.jl: OpticStyle(optic) = OpticStyle(typeof(optic))
OpticStyle(::Type{MyOptic}) = ...
doit(optic) = _doit(optic, OpticStyle(optic))
function _doit(optic::ComposedOptic, ::SomeStyle)
x1 = doit(optic.inner)
...
x2 = doit(optic.outer)
...
end A workaround is to replace OpticStyle(::Type{MyOptic}) = ... by OpticStyle(::MyOptic) = ... |
Ah, interesting. I wonder if we can do something about that too. |
Maybe try @generated function OpticStyle(optic::T) where {T}
OpticStyle(T)
end This worked for me in a similar situation. |
If I do this, it seems dispatch selects the wrong method in some situations. I thought @generated function OpticStyle(optic::T) where {T}
OpticStyle(T)
end and function OpticStyle(optic::T) where {T}
OpticStyle(T)
end have exactly the same semantics, but seems not to be the case. Base.@pure OpticStyle(obj) = OpticStyle(typeof(obj)) solves the problem, but will probably lead to bugs. |
We had a special case for Type that disallowed type trait recursion in favor of a pattern that almost never appears in code (only once in the compiler by accident where it doesn't matter). This was unnecessarily confusing and unexpected to predict what can infer, and made traits harder than necessary (such as Broadcast.ndims since 70fc3cd). Fix #43296 Fix #43368
We had a special case for Type that disallowed type trait recursion in favor of a pattern that almost never appears in code (only once in the compiler by accident where it doesn't matter). This was unnecessarily confusing and unexpected to predict what can infer, and made traits harder than necessary (such as Broadcast.ndims since 70fc3cd). Fix #43296 Fix #43368
We had a special case for Type that disallowed type trait recursion in favor of a pattern that almost never appears in code (only once in the compiler by accident where it doesn't matter). This was unnecessarily confusing and unexpected to predict what can infer, and made traits harder than necessary (such as Broadcast.ndims since 70fc3cd). Fix #43296 Fix #43368
We had a special case for Type that disallowed type trait recursion in favor of a pattern that almost never appears in code (only once in the compiler by accident where it doesn't matter). This was unnecessarily confusing and unexpected to predict what can infer, and made traits harder than necessary (such as Broadcast.ndims since 70fc3cd). Fix #43296 Fix #43368 (cherry picked from commit 33e3d9f)
We had a special case for Type that disallowed type trait recursion in favor of a pattern that almost never appears in code (only once in the compiler by accident where it doesn't matter). This was unnecessarily confusing and unexpected to predict what can infer, and made traits harder than necessary (such as Broadcast.ndims since 70fc3cd). Fix #43296 Fix #43368 (cherry picked from commit 33e3d9f)
The following snipped infers in 1.6 and even 1.7-rc1 but does not anymore. Came up here:
The text was updated successfully, but these errors were encountered: