Improve foldl's stability on nested Iterators.
#45789
Conversation
1. Make `Fix1(f, Int)` stable 2. split `_xfadjoint` into `_xfadjoint_unwrap` and `_xfadjoint_wrap`
base/operators.jl
Outdated
| @@ -1078,8 +1078,7 @@ struct Fix1{F,T} <: Function | |||
| f::F | |||
| x::T | |||
|
|
|||
| Fix1(f::F, x::T) where {F,T} = new{F,T}(f, x) | |||
| Fix1(f::Type{F}, x::T) where {F,T} = new{Type{F},T}(f, x) | |||
| Fix1(f, x) = new{Core.Typeof(f),Core.Typeof(x)}(f, x) | |||
There was a problem hiding this comment.
This usage of Core.Typeof is not correct, because Typeof doesn't always return legal types:
julia> struct Fix1{F,T} <: Function
f::F
x::T
Fix1(f, x) = new{Core.Typeof(f),Core.Typeof(x)}(f, x)
end
julia> Fix1(==, Vector{Core._typevar(:T, Union{}, Any)})
ERROR: TypeError: in new, expected DataType, got Type{Fix1{typeof(==), Type{Array{T, 1}}}}
Stacktrace:
[1] Fix1(f::Function, x::Type)
@ Main ./REPL[3]:5
[2] top-level scope
@ REPL[4]:1There was a problem hiding this comment.
Can we use it for f? (I see ComposedFunction(outer, inner) = new{Core.Typeof(outer),Core.Typeof(inner)}(outer, inner) above)
There was a problem hiding this comment.
I don't think it's possible to define methods on incomplete types with free typevars, but it will change the kind of error thrown to a TypeError during construction instead of a MethodError when actually called, so it's not great either
There was a problem hiding this comment.
I tried f(::Type{T}) where {T} = T, but f(Vector{Core._typevar(:T, Union{}, Any)}) throws a ERROR: UndefVarError: T not defined
So we need something like:
_typeof(x) = typeof(x)
_typeof(::Type{T}) where {T} = @isdefined(T) ? Type{T} : DataType?
If this is acceptable, should we also modifies
Line 931 in 68d62ab
?
There was a problem hiding this comment.
Is there anything we should talk at this point, or is this PR ready for merge?
There was a problem hiding this comment.
I'm not familiar with incomplete type. If core devs are happy with the change in a929310, I think it’s ready to merge.
And fuse it in `Base._xfadjoint`.
N5N3
force-pushed
the
recursion_inference
branch
from
29e506f to
8e9d35f
Compare
|
Closes #45715 |
|
@vtjnash - since this doesn't change behavior, could it be backported to |
Ah, I meant Julia v1.8, of course. |
* Make `Fix1(f, Int)` inference-stable * split `_xfadjoint` into `_xfadjoint_unwrap` and `_xfadjoint_wrap` * Improve `(c::ComposedFunction)(x...)`'s inferability * and fuse it in `Base._xfadjoint`. * define a `Typeof` operator that will partly work around internal type-system bugs Closes JuliaLang#45715
|
It would be really great if this could be backported to 1.8. |
* Make `Fix1(f, Int)` inference-stable * split `_xfadjoint` into `_xfadjoint_unwrap` and `_xfadjoint_wrap` * Improve `(c::ComposedFunction)(x...)`'s inferability * and fuse it in `Base._xfadjoint`. * define a `Typeof` operator that will partly work around internal type-system bugs Closes #45715 (cherry picked from commit d58289c)
Fix #45748.
This PR splits
_xfadjointinto_xfadjoint_unwrapand_xfadjoint_wrapthus helps compiler to realize that this recursion is convergent.Test added.