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.

Sign up for GitHub

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

Jump to bottom

Internal errors caused by broken Varargs #51228

Closed
maleadt opened this issue Sep 7, 2023 · 4 comments · Fixed by #51300 or #51317
Closed

Internal errors caused by broken Varargs #51228

maleadt opened this issue Sep 7, 2023 · 4 comments · Fixed by #51300 or #51317
Labels
bug Indicates an unexpected problem or unintended behavior types and dispatch Types, subtyping and method dispatch

Comments

@maleadt
Copy link
Member

maleadt commented Sep 7, 2023

I'm not sure we care, given that this Vararg is very wrong, but I guess it maybe shouldn't throw an internal error:

f(x=whatever) = 1
f(::Vararg{1}) = 2
f()
julia> f()
Internal error: encountered unexpected error in runtime:
MethodError(f=Core.Compiler.widenconst, args=(1,), world=0x00000000000015a5)
jl_method_error_bare at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-release-1-dot-10/src/gf.c:2203
jl_method_error at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-release-1-dot-10/src/gf.c:2221
jl_lookup_generic_ at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-release-1-dot-10/src/gf.c:3052 [inlined]
ijl_apply_generic at /Users/julia/.julia/scratchspaces/a66863c6-20e8-4ff4-8a62-49f30b1f605e/agent-cache/default-honeycrisp-R17H3W25T9.0/build/default-honeycrisp-R17H3W25T9-0/julialang/julia-release-1-dot-10/src/gf.c:3067
tuple_tfunc at ./compiler/tfuncs.jl:1927
most_general_argtypes at ./compiler/inferenceresult.jl:151
most_general_argtypes at ./compiler/inferenceresult.jl:109 [inlined]
matching_cache_argtypes at ./compiler/inferenceresult.jl:27 [inlined]
InferenceResult at ./compiler/types.jl:88 [inlined]
typeinf_edge at ./compiler/typeinfer.jl:919
...
ERROR: UndefVarError: `whatever` not defined
@maleadt maleadt added bug Indicates an unexpected problem or unintended behavior types and dispatch Types, subtyping and method dispatch labels Sep 7, 2023
@maleadt
Copy link
Member Author

maleadt commented Sep 7, 2023 •
edited
Loading

A similar case:

f(x=whatever...) = 1
f(x::T, y::Vararg{1}) where {T} = 2
f()
Internal error: encountered unexpected error in runtime:
TypeError(func=:UnionAll, context="", expected=Type, got=1)
ijl_type_error_rt at /cache/build/default-amdci5-3/julialang/julia-release-1-dot-10/src/rtutils.c:119
ijl_type_error at /cache/build/default-amdci5-3/julialang/julia-release-1-dot-10/src/rtutils.c:127
ijl_type_unionall at /cache/build/default-amdci5-3/julialang/julia-release-1-dot-10/src/jltypes.c:861
rewrap_unionall at ./essentials.jl:388
rewrap_unionall at ./essentials.jl:388
rewrap_unionall at ./essentials.jl:396 [inlined]
most_general_argtypes at ./compiler/inferenceresult.jl:140
most_general_argtypes at ./compiler/inferenceresult.jl:109 [inlined]
matching_cache_argtypes at ./compiler/inferenceresult.jl:27 [inlined]
InferenceResult at ./compiler/types.jl:88 [inlined]
typeinf_edge at ./compiler/typeinfer.jl:919
...
ERROR: UndefVarError: `whatever` not defined

@maleadt maleadt changed the title Internal error: tuple tfunc cannot handle broken Vararg Internal errors caused by broken Varargs Sep 7, 2023
vtjnash added a commit that referenced this issue Sep 7, 2023
@vtjnash
implement validation of Vararg type parameter on construction
ca69bd6 
It does not seem necessary to allow this, though it does lead to this
printing issue to disallow it:

    julia> Tuple{1, 1, 1, 1}
    NTuple{4, 1}

Fix #51228
vtjnash added a commit that referenced this issue Sep 7, 2023
@vtjnash
implement validation of Vararg type parameter on construction
62c9f64 
It does not seem necessary to allow this, though it does lead to this
printing issue to disallow it:

    julia> Tuple{1, 1, 1, 1}
    NTuple{4, 1}

Fix #51228
@maleadt
Copy link
Member Author

maleadt commented Sep 7, 2023

Naively adding a bail-out path that returns Bottom if the param isn't a Type (which I'm not sure is legal) gets me a little further, but it then fails during inlining:

ERROR: TypeError: in <:, expected Type, got a value of type Int64
Stacktrace:
  [1] ir_inline_unionsplit!(compact::Core.Compiler.IncrementalCompact, idx::Int64, argexprs::Vector{…}, union_split::Core.Compiler.UnionSplit, boundscheck::Symbol, todo_bbs::Vector{…}, params::Core.Compiler.OptimizationParams)
    @ Core.Compiler base/compiler/ssair/inlining.jl:589
  [2] batch_inline!(ir::Core.Compiler.IRCode, todo::Vector{…}, propagate_inbounds::Bool, params::Core.Compiler.OptimizationParams)
    @ Core.Compiler base/compiler/ssair/inlining.jl:712
  [3] ssa_inlining_pass!
    @ Core.Compiler ./compiler/ssair/inlining.jl:81 [inlined]
  [4] run_passes_ipo_safe(ci::Core.CodeInfo, sv::Core.Compiler.OptimizationState{…}, caller::Core.Compiler.InferenceResult, optimize_until::Nothing)
    @ Core.Compiler base/compiler/optimize.jl:765
  [5]
    @ Core.Compiler base/compiler/typeinfer.jl:970
  [6] code_ircode_by_type(tt::Type; world::UInt64, interp::Core.Compiler.NativeInterpreter, optimize_until::Nothing)
    @ Base ./reflection.jl:1586
  [7] code_ircode_by_type
    @ Base ./reflection.jl:1572 [inlined]
  [8] code_ircode(f::Any, types::Any; kwargs::@Kwargs{})
    @ Base ./reflection.jl:1563
  [9] code_ircode(f::Any, types::Any)
    @ Base ./reflection.jl:1558
 [10] top-level scope
    @ REPL[6]:1

I guess the compiler is just not great at handling these ill-formed types? Should/can we prevent construction of them?

@vtjnash
Copy link
Member

vtjnash commented Sep 7, 2023

I added that change, and it seems to pass tests: https://github.com/JuliaLang/julia/compare/jn/51228. It is probably a reasonable fix, though it means we need to fix printing of NTuple

@maleadt maleadt mentioned this issue Sep 8, 2023
Merged
maleadt pushed a commit that referenced this issue Sep 11, 2023
@vtjnash @maleadt
implement validation of Vararg type parameter on construction
ab551f9 
It does not seem necessary to allow this, though it does lead to this
printing issue to disallow it:

    julia> Tuple{1, 1, 1, 1}
    NTuple{4, 1}

Fix #51228
maleadt pushed a commit that referenced this issue Sep 13, 2023
@vtjnash @maleadt
implement validation of Vararg type parameter on construction
5e0df64 
It does not seem necessary to allow this, though it does lead to this
printing issue to disallow it:

    julia> Tuple{1, 1, 1, 1}
    NTuple{4, 1}

Fix #51228
vtjnash added a commit that referenced this issue Sep 13, 2023
@vtjnash
fix method definition error for bad vararg
93adbda 
We had the ordering of tests incorrect, so Vararg was not correctly
checked for validity during method definition.

Fixes #51228
@vtjnash vtjnash mentioned this issue Sep 13, 2023
Merged
vtjnash added a commit that referenced this issue Sep 13, 2023
@vtjnash
implement validation of Vararg type parameter on construction
ae1d71b 
It does not seem necessary to allow this, though it does lead to this
printing issue to disallow it:

    julia> Tuple{1, 1, 1, 1}
    NTuple{4, 1}

Fix #51228
vtjnash added a commit that referenced this issue Sep 13, 2023
@vtjnash
cleanup Vararg instantiation code
4e66163 
Various simplifications and improvements from investigating #51228.
Improves the logic for showing of NTuple to handle constant lengths.
Improves the logic for showing NTuple of bound length (e.g. NTuple
itself). Also makes a choice to avoid showing non-types as NTuple, but
instead try to write them out, to make it more visually obvious when the
parameters have been swapped.
vtjnash added a commit that referenced this issue Sep 13, 2023
@vtjnash
fix method definition error for bad vararg (#51300)
34e6035 
We had the ordering of tests incorrect, so Vararg was not correctly
checked for validity during method definition.

Fixes #51228
@maleadt
Copy link
Member Author

maleadt commented Sep 14, 2023

Looks like a slight variation still errors:

f(x=whatever) = 1
f(::Vararg{T,T}) where T = 2
f()
Internal error: encountered unexpected error in runtime:
MethodError(f=Core.Compiler.widenconst, args=(1,), world=0x00000000000015e7)
jl_method_error_bare at /home/tim/Julia/src/julia/src/gf.c:2206
jl_method_error at /home/tim/Julia/src/julia/src/gf.c:2224
jl_lookup_generic_ at /home/tim/Julia/src/julia/src/gf.c:3055 [inlined]
ijl_apply_generic at /home/tim/Julia/src/julia/src/gf.c:3070
tuple_tfunc at ./compiler/tfuncs.jl:1948
most_general_argtypes at ./compiler/inferenceresult.jl:157
most_general_argtypes at ./compiler/inferenceresult.jl:115 [inlined]
matching_cache_argtypes at ./compiler/inferenceresult.jl:27 [inlined]
InferenceResult at ./compiler/types.jl:88 [inlined]
typeinf_edge at ./compiler/typeinfer.jl:852
abstract_call_method at ./compiler/abstractinterpretation.jl:614
abstract_call_gf_by_type at ./compiler/abstractinterpretation.jl:89
abstract_call_known at ./compiler/abstractinterpretation.jl:2056
abstract_call at ./compiler/abstractinterpretation.jl:2138
abstract_call at ./compiler/abstractinterpretation.jl:2131
abstract_call at ./compiler/abstractinterpretation.jl:2287
abstract_eval_call at ./compiler/abstractinterpretation.jl:2302
abstract_eval_statement_expr at ./compiler/abstractinterpretation.jl:2312
abstract_eval_statement at ./compiler/abstractinterpretation.jl:2606
abstract_eval_basic_statement at ./compiler/abstractinterpretation.jl:2902
typeinf_local at ./compiler/abstractinterpretation.jl:3099
typeinf_nocycle at ./compiler/abstractinterpretation.jl:3187
_typeinf at ./compiler/typeinfer.jl:247
typeinf at ./compiler/typeinfer.jl:216

@maleadt maleadt reopened this Sep 14, 2023
vtjnash added a commit that referenced this issue Sep 14, 2023
@vtjnash
inference: avoid inferring unreachable code methods
c9303c0 
Avoids causing issues in matching_cache_argtypes/tuple_tfunc with
invalid contents appearing in the Tuple parameters after intersections
(such as Int, tuple of Symbol, etc).

Also sharpen valid_as_lattice / instanceof_tfunc to be able to filter
out and reject types that cannot appear as tags at runtime, except where
they are used for non-tag queries (like fieldtype_tfunc and subtype_tfunc).

Fixes #51228 (part 2)
@vtjnash vtjnash mentioned this issue Sep 14, 2023
Merged
KristofferC pushed a commit that referenced this issue Sep 15, 2023
@vtjnash
fix method definition error for bad vararg (#51300)
503a44f 
We had the ordering of tests incorrect, so Vararg was not correctly
checked for validity during method definition.

Fixes #51228

(cherry picked from commit 34e6035)
NHDaly pushed a commit that referenced this issue Sep 20, 2023
@vtjnash @NHDaly
fix method definition error for bad vararg (#51300)
386876f 
We had the ordering of tests incorrect, so Vararg was not correctly
checked for validity during method definition.

Fixes #51228
vtjnash added a commit that referenced this issue Sep 24, 2023
@vtjnash
cleanup Vararg instantiation code
f995dcd 
Various simplifications and improvements from investigating #51228.
Improves the logic for showing of NTuple to handle constant lengths.
Improves the logic for showing NTuple of bound length (e.g. NTuple
itself). Also makes a choice to avoid showing non-types as NTuple, but
instead try to write them out, to make it more visually obvious when the
parameters have been swapped.
JeffBezanson pushed a commit that referenced this issue Sep 25, 2023
@maleadt @vtjnash
improve NTuple show and cleanup code for Vararg and NTuple type param…
0287a00 
…eter construction (#51244)

Various simplifications and improvements from investigating #51228.
Improves the logic for showing of NTuple to handle constant lengths.
Improves the logic for showing NTuple of bound length (e.g. NTuple
itself). Also makes a choice to avoid showing non-types as NTuple, but
instead try to write them out, to make it more visually obvious when the
parameters have been swapped.

---------

Co-authored-by: Jameson Nash <vtjnash@gmail.com>
vtjnash added a commit that referenced this issue Sep 26, 2023
@vtjnash
inference: avoid inferring unreachable code methods
2cee898 
Avoids causing issues in matching_cache_argtypes/tuple_tfunc with
invalid contents appearing in the Tuple parameters after intersections
(such as Int, tuple of Symbol, etc).

Also sharpen valid_as_lattice / instanceof_tfunc to be able to filter
out and reject types that cannot appear as tags at runtime, except where
they are used for non-tag queries (like fieldtype_tfunc and subtype_tfunc).

Fixes #51228 (part 2)
vtjnash added a commit that referenced this issue Sep 26, 2023
@vtjnash
inference: avoid inferring unreachable code methods
33f391c 
Avoids causing issues in matching_cache_argtypes/tuple_tfunc with
invalid contents appearing in the Tuple parameters after intersections
(such as Int, tuple of Symbol, etc).

Also sharpen valid_as_lattice / instanceof_tfunc to be able to filter
out and reject types that cannot appear as tags at runtime, except where
they are used for non-tag queries (like fieldtype_tfunc and subtype_tfunc).

Fixes #51228 (part 2)
nalimilan pushed a commit that referenced this issue Nov 5, 2023
@vtjnash @nalimilan
fix method definition error for bad vararg (#51300)
2d0421e 
We had the ordering of tests incorrect, so Vararg was not correctly
checked for validity during method definition.

Fixes #51228

(cherry picked from commit 34e6035)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Indicates an unexpected problem or unintended behavior types and dispatch Types, subtyping and method dispatch
Projects
None yet
Milestone
No milestone
2 participants
@vtjnash @maleadt