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

Fail gracefully when attempting pair destructuring #49368

Merged
merged 4 commits into from
Jun 8, 2023
Merged

Fail gracefully when attempting pair destructuring #49368

merged 4 commits into from
Jun 8, 2023

Conversation

LilithHafner
Copy link
Member

@LilithHafner LilithHafner commented Apr 15, 2023 •
edited
Loading

This fixes an issue that has been reported at least 5 times (#37783, #39929, #42552, #43379, #48332) by patching a bad special case of #25744. It's technically breaking and/or a bugfix, depending on how you view it.

PR:

julia> x = 1 => 2
1 => 2

julia> a => b = x
ERROR: invalid method definition in Main: function Base.=> must be explicitly imported to be extended
Stacktrace:
 [1] top-level scope
   @ none:0
 [2] top-level scope
   @ REPL[41]:1

julia> 1+1
2

Before:

julia> x = 1 => 2
1 => 2

julia> a => b = x
Error showing value of type UnionAll:
ERROR: 
SYSTEM (REPL): showing an error caused an error
ERROR: 
SYSTEM (REPL): caught exception of type MethodError while trying to handle a nested exception; giving up

julia> 1+1
Unhandled Task ERROR: MethodError: Cannot `convert` an object of type Int64 to an object of type Symbol

Closest candidates are:
  convert(::Type{T}, ::T) where T
   @ Base Base.jl:64
  Symbol(::Any...)
   @ Base strings/basic.jl:229

Stacktrace:
  [1] Base.ImmutableDict{Symbol, Any}(parent::Base.ImmutableDict{Symbol, Any}, key::Int64, value::Int64)
    @ Base ./dict.jl:756
  [2] IOContext(io::IOContext{IOBuffer}, KV::Pair{Int64, Int64})
    @ Base ./show.jl:312
  [3] IOContext(io::IOContext{IOBuffer}, KV::Pair{Int64, Int64}, KVs::Pair{Int64, Int64})
    @ Base ./show.jl:390
  [4] handle_message(logger::Logging.ConsoleLogger, level::Base.CoreLogging.LogLevel, message::Any, _module::Any, group::Any, id::Any, filepath::Any, line::Any; kwargs::Base.Pairs{Symbol, V, Tuple{Vararg{Symbol, N}}, NamedTuple{names, T}} where {V, N, names, T<:Tuple{Vararg{Any, N}}})
    @ Logging ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/Logging/src/ConsoleLogger.jl:129
  [5] invokelatest(::Any, ::Any, ::Vararg{Any}; kwargs::Base.Pairs{Symbol, Tuple{TypeError, Vector{Union{Ptr{Nothing}, Base.InterpreterIP}}}, Tuple{Symbol}, NamedTuple{(:exception,), Tuple{Tuple{TypeError, Vector{Union{Ptr{Nothing}, Base.InterpreterIP}}}}}})
    @ Base ./essentials.jl:818
  [6] macro expansion
    @ ./logging.jl:365 [inlined]
  [7] prompt!(term::REPL.Terminals.TextTerminal, prompt::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:2742
  [8] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:2642
  [9] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/REPL.jl:1298
 [10] (::REPL.var"#62#68"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL ./task.jl:514

caused by: TypeError: in typeassert, expected AbstractChar, got a value of type Int64
Stacktrace:
  [1] rstrip(f::typeof(isspace), s::String)
    @ Base ./strings/util.jl:380
  [2] rstrip
    @ ./strings/util.jl:384 [inlined]
  [3] add_history(hist::REPL.REPLHistoryProvider, s::REPL.LineEdit.PromptState)
    @ REPL ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/REPL.jl:640
  [4] add_history(s::REPL.LineEdit.PromptState)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:1427
  [5] add_history(::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:242
  [6] commit_line(s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:2274
  [7] (::REPL.LineEdit.var"#116#172")(::REPL.LineEdit.MIState, ::Any, ::Vararg{Any})
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:2363
  [8] #invokelatest#2
    @ ./essentials.jl:816 [inlined]
  [9] invokelatest
    @ ./essentials.jl:813 [inlined]
 [10] (::REPL.LineEdit.var"#27#28"{REPL.LineEdit.var"#116#172", String})(s::Any, p::Any)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:1603
 [11] prompt!(term::REPL.Terminals.TextTerminal, prompt::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:2740
 [12] run_interface(terminal::REPL.Terminals.TextTerminal, m::REPL.LineEdit.ModalInterface, s::REPL.LineEdit.MIState)
    @ REPL.LineEdit ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/LineEdit.jl:2642
 [13] run_frontend(repl::REPL.LineEditREPL, backend::REPL.REPLBackendRef)
    @ REPL ~/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/share/julia/stdlib/v1.9/REPL/src/REPL.jl:1298
 [14] (::REPL.var"#62#68"{REPL.LineEditREPL, REPL.REPLBackendRef})()
    @ REPL ./task.jl:514
ERROR: fatal: error thrown and no exception handler available.
MethodError(f=Base.convert, args=(Symbol, 1), world=0x00000000000082af)
jl_method_error_bare at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
jl_method_error at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
ijl_apply_generic at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
ImmutableDict at ./dict.jl:756
IOContext at ./show.jl:312
unknown function (ip: 0x102b9005f)
ijl_apply_generic at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
display_error at ./client.jl:111
unknown function (ip: 0x114dcc1ab)
ijl_apply_generic at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
display_error at ./client.jl:114
unknown function (ip: 0x114dc40e7)
ijl_apply_generic at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
jl_f__call_latest at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
#invokelatest#2 at ./essentials.jl:816 [inlined]
invokelatest at ./essentials.jl:813 [inlined]
_start at ./client.jl:524
jfptr__start_36981 at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/sys.dylib (unknown line)
ijl_apply_generic at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
true_main at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)
jl_repl_entrypoint at /Users/x/.julia/juliaup/julia-1.9.0-rc2+0.aarch64.apple.darwin14/lib/julia/libjulia-internal.1.9.dylib (unknown line)

Fixes #37783
Fixes #39929
Fixes #42552
Fixes #43379
Fixes #48332

@LilithHafner LilithHafner added bugfix This change fixes an existing bug minor change Marginal behavior change acceptable for a minor release needs pkgeval Tests for all registered packages should be run with this change labels Apr 15, 2023
@Keno
Copy link
Member

Keno commented Apr 15, 2023

I really think we need to stop allowing method definition using infix syntax entirely. It's just a niche feature that nobody ever uses, but just keeps biting people.

@LilithHafner
Copy link
Member Author

@nanosoldier runtests()

@nanosoldier
Copy link
Collaborator

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

@LilithHafner
Copy link
Member Author

@nanosoldier runtests(["Tables", "BitInformation", "MarchingCubes", "MultiKDE", "Hyperopt", "DFOLS", "ScoreDrivenModels", "AstroChemistry", "BioAlignments", "RateLimiter", "VCFTools", "RectiGrids", "ScrapeSEC", "BoltzmannMachinesPlots", "GaussNewton", "RAPIDS", "FMMLIB2D", "TableTraitsUtils", "MonkeyLang", "GNSSReceiver", "IterationControl", "XTermColors", "EarthDataLab", "Casacore", "PlasmaEquilibriumToolkit", "UMAP", "TiledIteration", "Arblib", "ManoptExamples", "EconomicScenarioGenerators", "Spinnaker", "ParameterEstimation", "PlantBiophysics", "CamiXon", "InformationGeometry", "PyBraket", "QSM", "NSGAII", "RobustAdaptiveMetropolisSampler", "RoMEPlotting", "VisualizeMotifs", "ImplicitPlots", "SpinDoctor", "TensorBoardLogger", "SphericalHarmonicModes", "ConvexFit", "CommunicationsSequences", "ThermodynamicIntegration", "BondGraphs", "QPSReader", "ReactionNetworkImporters", "Pitaya", "MCMCDiagnosticTools"])

@nanosoldier
Copy link
Collaborator

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

@LilithHafner LilithHafner removed the needs pkgeval Tests for all registered packages should be run with this change label Apr 17, 2023
@LilithHafner
Copy link
Member Author

Tagging triage because this is technically breaking

@LilithHafner LilithHafner added the triage This should be discussed on a triage call label May 27, 2023
@oscardssmith
Copy link
Member

triage approves.

@oscardssmith oscardssmith removed the triage This should be discussed on a triage call label Jun 8, 2023
@LilithHafner LilithHafner added the merge me PR is reviewed. Merge when all tests are passing label Jun 8, 2023
@LilithHafner LilithHafner merged commit 746a15b into master Jun 8, 2023
@LilithHafner LilithHafner deleted the lh/pair-25744 branch June 8, 2023 21:52
@LilithHafner LilithHafner removed the merge me PR is reviewed. Merge when all tests are passing label Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
bugfix This change fixes an existing bug minor change Marginal behavior change acceptable for a minor release
Projects
None yet
Milestone
No milestone
4 participants
@LilithHafner @Keno @nanosoldier @oscardssmith