Skip to content

Commit fa4972d

Browse files
authored
Make haskey(::@kwargs{item::Bool}, :item) constant-fold (#59320)
Resolves #59269. ~~Makes #59292 somewhat worse by doubling-down on the same assumptions.~~ ~~We should straighten out that issue properly, but I don't see any reason not to get better inference in the mean time for the very common `@Kwargs` case.~~ We are already relying on those assumptions in Base (as that issue demonstrates), and @JeffBezanson says that this functionality wasn't intended to be supported: > I think the use case for it is picking not a subset, but alternate key set, specifically linear or cartesian indices for arrays. [...] So go ahead and add that haskey method **edit:** now also resolves #59292.
1 parent bb3f280 commit fa4972d

File tree

7 files changed

+31
-18
lines changed

7 files changed

+31
-18
lines changed

Compiler/test/inference.jl

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6515,4 +6515,11 @@ end <: Bool
65156515
Core.get_binding_type(m, n, xs...)
65166516
end <: Type
65176517

6518+
# issue #59269
6519+
function haskey_inference_test()
6520+
kwargs = Core.compilerbarrier(:const, Base.pairs((; item = false)))
6521+
return haskey(kwargs, :item) ? nothing : Any[]
6522+
end
6523+
@inferred haskey_inference_test()
6524+
65186525
end # module inference

base/essentials.jl

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -503,9 +503,9 @@ end
503503
Pairs{K, V, I, A}(data, itr) where {K, V, I, A} = $(Expr(:new, :(Pairs{K, V, I, A}), :(data isa A ? data : convert(A, data)), :(itr isa I ? itr : convert(I, itr))))
504504
Pairs{K, V}(data::A, itr::I) where {K, V, I, A} = $(Expr(:new, :(Pairs{K, V, I, A}), :data, :itr))
505505
Pairs{K}(data::A, itr::I) where {K, I, A} = $(Expr(:new, :(Pairs{K, eltype(A), I, A}), :data, :itr))
506-
Pairs(data::A, itr::I) where {I, A} = $(Expr(:new, :(Pairs{eltype(I), eltype(A), I, A}), :data, :itr))
506+
Pairs(data::A, itr::I) where {I, A} = $(Expr(:new, :(Pairs{I !== Nothing ? eltype(I) : keytype(A), eltype(A), I, A}), :data, :itr))
507507
end
508-
pairs(::Type{NamedTuple}) = Pairs{Symbol, V, NTuple{N, Symbol}, NamedTuple{names, T}} where {V, N, names, T<:NTuple{N, Any}}
508+
pairs(::Type{NamedTuple}) = Pairs{Symbol, V, Nothing, NT} where {V, NT <: NamedTuple}
509509

510510
"""
511511
Base.Pairs(values, keys) <: AbstractDict{eltype(keys), eltype(values)}

base/iterators.jl

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -267,48 +267,48 @@ pairs(::IndexLinear, A::AbstractArray) = Pairs(A, LinearIndices(A))
267267
# preserve indexing capabilities for known indexable types
268268
# faster than zip(keys(a), values(a)) for arrays
269269
pairs(tuple::Tuple) = Pairs{Int}(tuple, keys(tuple))
270-
pairs(nt::NamedTuple) = Pairs{Symbol}(nt, keys(nt))
270+
pairs(nt::NamedTuple) = Pairs{Symbol}(nt, nothing)
271271
pairs(v::Core.SimpleVector) = Pairs(v, LinearIndices(v))
272272
pairs(A::AbstractVector) = pairs(IndexLinear(), A)
273273
# pairs(v::Pairs) = v # listed for reference, but already defined from being an AbstractDict
274274

275275
pairs(::IndexCartesian, A::AbstractArray) = Pairs(A, Base.CartesianIndices(axes(A)))
276276
pairs(A::AbstractArray) = pairs(IndexCartesian(), A)
277277

278-
length(v::Pairs) = length(getfield(v, :itr))
279-
axes(v::Pairs) = axes(getfield(v, :itr))
280-
size(v::Pairs) = size(getfield(v, :itr))
278+
length(v::Pairs) = length(keys(v))
279+
axes(v::Pairs) = axes(keys(v))
280+
size(v::Pairs) = size(keys(v))
281281

282282
Base.@eval @propagate_inbounds function _pairs_elt(p::Pairs{K, V}, idx) where {K, V}
283283
return $(Expr(:new, :(Pair{K, V}), :idx, :(getfield(p, :data)[idx])))
284284
end
285285

286286
@propagate_inbounds function iterate(p::Pairs{K, V}, state...) where {K, V}
287-
x = iterate(getfield(p, :itr), state...)
287+
x = iterate(keys(p), state...)
288288
x === nothing && return x
289289
idx, next = x
290290
return (_pairs_elt(p, idx), next)
291291
end
292292

293-
@propagate_inbounds function iterate(r::Reverse{<:Pairs}, state=(reverse(getfield(r.itr, :itr)),))
293+
@propagate_inbounds function iterate(r::Reverse{<:Pairs}, state=(reverse(keys(r.itr)),))
294294
x = iterate(state...)
295295
x === nothing && return x
296296
idx, next = x
297297
return (_pairs_elt(r.itr, idx), (state[1], next))
298298
end
299299

300-
@inline isdone(v::Pairs, state...) = isdone(getfield(v, :itr), state...)
300+
@inline isdone(v::Pairs, state...) = isdone(keys(v), state...)
301301

302302
IteratorSize(::Type{<:Pairs{<:Any, <:Any, I}}) where {I} = IteratorSize(I)
303303
IteratorSize(::Type{<:Pairs{<:Any, <:Any, <:AbstractUnitRange, <:Tuple}}) = HasLength()
304304

305305
function last(v::Pairs{K, V}) where {K, V}
306-
idx = last(getfield(v, :itr))
306+
idx = last(keys(v))
307307
return Pair{K, V}(idx, v[idx])
308308
end
309309

310-
haskey(v::Pairs, key) = (key in getfield(v, :itr))
311-
keys(v::Pairs) = getfield(v, :itr)
310+
haskey(v::Pairs, key) = key in keys(v)
311+
keys(v::Pairs) = getfield(v, :itr) === nothing ? keys(getfield(v, :data)) : getfield(v, :itr)
312312
values(v::Pairs) = getfield(v, :data) # TODO: this should be a view of data subset by itr
313313
getindex(v::Pairs, key) = getfield(v, :data)[key]
314314
setindex!(v::Pairs, value, key) = (getfield(v, :data)[key] = value; v)

base/namedtuple.jl

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ merge(a::NamedTuple, b::NamedTuple{()}) = a
343343
merge(a::NamedTuple{()}, b::NamedTuple{()}) = a
344344
merge(a::NamedTuple{()}, b::NamedTuple) = b
345345

346-
merge(a::NamedTuple, b::Iterators.Pairs{<:Any,<:Any,<:Any,<:NamedTuple}) = merge(a, getfield(b, :data))
346+
merge(a::NamedTuple, b::Iterators.Pairs{<:Any,<:Any,Nothing,<:NamedTuple}) = merge(a, getfield(b, :data))
347347

348348
merge(a::NamedTuple, b::Iterators.Zip{<:Tuple{Any,Any}}) = merge(a, NamedTuple{Tuple(b.is[1])}(b.is[2]))
349349

@@ -535,7 +535,7 @@ when it is printed in the stack trace view.
535535
536536
```julia
537537
julia> @Kwargs{init::Int} # the internal representation of keyword arguments
538-
Base.Pairs{Symbol, Int64, Tuple{Symbol}, @NamedTuple{init::Int64}}
538+
Base.Pairs{Symbol, Int64, Nothing, @NamedTuple{init::Int64}}
539539
540540
julia> sum("julia"; init=1)
541541
ERROR: MethodError: no method matching +(::Char, ::Char)
@@ -578,7 +578,7 @@ Stacktrace:
578578
macro Kwargs(ex)
579579
return :(let
580580
NT = @NamedTuple $ex
581-
Base.Pairs{keytype(NT),eltype(NT),typeof(NT.parameters[1]),NT}
581+
Base.Pairs{keytype(NT),eltype(NT),Nothing,NT}
582582
end)
583583
end
584584

base/show.jl

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1116,6 +1116,8 @@ function show_type_name(io::IO, tn::Core.TypeName)
11161116
end
11171117

11181118
function maybe_kws_nt(x::DataType)
1119+
# manually-written version of
1120+
# x <: (Pairs{Symbol, eltype(NT), Nothing, NT} where NT <: NamedTuple)
11191121
x.name === typename(Pairs) || return nothing
11201122
length(x.parameters) == 4 || return nothing
11211123
x.parameters[1] === Symbol || return nothing
@@ -1125,7 +1127,7 @@ function maybe_kws_nt(x::DataType)
11251127
types isa DataType || return nothing
11261128
x.parameters[2] === eltype(p4) || return nothing
11271129
isa(syms, Tuple) || return nothing
1128-
x.parameters[3] === typeof(syms) || return nothing
1130+
x.parameters[3] === Nothing || return nothing
11291131
return p4
11301132
end
11311133
return nothing
@@ -3219,7 +3221,7 @@ function Base.showarg(io::IO, r::Iterators.Pairs{<:Integer, <:Any, <:Any, T}, to
32193221
print(io, "pairs(IndexLinear(), ::", T, ")")
32203222
end
32213223

3222-
function Base.showarg(io::IO, r::Iterators.Pairs{Symbol, <:Any, <:Any, T}, toplevel) where {T <: NamedTuple}
3224+
function Base.showarg(io::IO, r::Iterators.Pairs{Symbol, <:Any, Nothing, T}, toplevel) where {T <: NamedTuple}
32233225
print(io, "pairs(::NamedTuple)")
32243226
end
32253227

test/namedtuple.jl

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,10 @@ end
163163
@test merge(NamedTuple(), [:a=>1, :b=>2, :c=>3, :a=>4, :c=>5]) == (a=4, b=2, c=5)
164164
@test merge((c=0, z=1), [:a=>1, :b=>2, :c=>3, :a=>4, :c=>5]) == (c=5, z=1, a=4, b=2)
165165

166+
# https://github.com/JuliaLang/julia/issues/59292
167+
@test merge((; a = 1), Base.Pairs((; b = 2, c = 3), (:b,))) == (a = 1, b = 2)
168+
@test merge((; a = 1), Base.pairs((; b = 2, c = 3))) == (a = 1, b = 2, c = 3)
169+
166170
@test keys((a=1, b=2, c=3)) == (:a, :b, :c)
167171
@test keys(NamedTuple()) == ()
168172
@test keys((a=1,)) == (:a,)

test/show.jl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1450,7 +1450,7 @@ test_repr("(:).a")
14501450
@test repr(@NamedTuple{var"#"::Int64}) == "@NamedTuple{var\"#\"::Int64}"
14511451

14521452
# Test general printing of `Base.Pairs` (it should not use the `@Kwargs` macro syntax)
1453-
@test repr(@Kwargs{init::Int}) == "Base.Pairs{Symbol, $Int, Tuple{Symbol}, @NamedTuple{init::$Int}}"
1453+
@test repr(@Kwargs{init::Int}) == "Base.Pairs{Symbol, $Int, Nothing, @NamedTuple{init::$Int}}"
14541454

14551455
@testset "issue #42931" begin
14561456
@test repr(NTuple{4, :A}) == "Tuple{:A, :A, :A, :A}"

0 commit comments

Comments
 (0)