Skip to content

Commit

Permalink
Make sure Core.Compiler can throw kwarg mismatch errors (#50174)
Browse files Browse the repository at this point in the history
The _new_NamedTuple helper was in a Base-only branch, causing

```

julia> Core.eval(Core.Compiler, quote f(;a=1) = a end)
f (generic function with 1 method)

julia> Core.Compiler.f(;b=2)
ERROR: UndefVarError: `_new_NamedTuple` not defined
Stacktrace:
 [1] macro expansion
   @ Core.Compiler ./namedtuple.jl:0 [inlined]
 [2] structdiff(a::@NamedTuple{b::Int64}, b::Type{NamedTuple{(:a,)}})
   @ Core.Compiler ./namedtuple.jl:421
 [3] top-level scope
   @ REPL[2]:1
```

After this change, we have the expected
```
julia> Core.eval(Core.Compiler, quote f(;a=1) = a end)
f (generic function with 1 method)

julia> Core.Compiler.f(;b=2)
ERROR: MethodError: no method matching f(; b::Int64)

Closest candidates are:
  f(; a) got unsupported keyword argument "b"
   @ Core REPL[13]:1

Stacktrace:
 [1] kwerr(kw::@NamedTuple{b::Int64}, args::Function)
   @ Core.Compiler ./error.jl:165
 [2] top-level scope
   @ REPL[14]:1
```
  • Loading branch information
Keno authored Jun 15, 2023
1 parent 5db2c27 commit 0f26966
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 6 deletions.
12 changes: 6 additions & 6 deletions base/namedtuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,6 @@ function NamedTuple{names, T}(nt::NamedTuple) where {names, T <: Tuple}
end
end

# Like NamedTuple{names, T} as a constructor, but omits the additional
# `convert` call, when the types are known to match the fields
@eval function _new_NamedTuple(T::Type{NamedTuple{NTN, NTT}} where {NTN, NTT}, args::Tuple)
$(Expr(:splatnew, :T, :args))
end

function NamedTuple{names}(nt::NamedTuple) where {names}
if @generated
idx = Int[ fieldindex(nt, names[n]) for n in 1:length(names) ]
Expand All @@ -161,6 +155,12 @@ NamedTuple{names, Union{}}(itr::Tuple) where {names} = throw(MethodError(NamedTu

end # if Base

# Like NamedTuple{names, T} as a constructor, but omits the additional
# `convert` call, when the types are known to match the fields
@eval function _new_NamedTuple(T::Type{NamedTuple{NTN, NTT}} where {NTN, NTT}, args::Tuple)
$(Expr(:splatnew, :T, :args))
end

length(t::NamedTuple) = nfields(t)
iterate(t::NamedTuple, iter=1) = iter > nfields(t) ? nothing : (getfield(t, iter), iter + 1)
rest(t::NamedTuple) = t
Expand Down
5 changes: 5 additions & 0 deletions test/compiler/AbstractInterpreter.jl
Original file line number Diff line number Diff line change
Expand Up @@ -348,3 +348,8 @@ let NoinlineModule = Module()
@test count(iscall((src, inlined_usually)), src.code) == 0
end
end

# Make sure that Core.Compiler has enough NamedTuple infrastructure
# to properly give error messages for basic kwargs...
Core.eval(Core.Compiler, quote f(;a=1) = a end)
@test_throws MethodError Core.Compiler.f(;b=2)

4 comments on commit 0f26966

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Executing the daily package evaluation, I will reply here when finished:

@nanosoldier runtests(isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your package evaluation job has completed - possible new issues were detected.
A full report can be found here.

@aviatesk
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nanosoldier runbenchmarks("inference", vs="@834aad4ab409f4ba65cbed2963b9ab6fa2770354")

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here.

Please sign in to comment.