Add object field to KeyError

[tecosaur]

Adding an optional field to KeyError that records the object the KeyError relates to allows for more sophisticated error messaging that can suggest potentially intended keys.

As a first pass, I've passed the object in question into every instance of KeyError that I've seen so far, but I wouldn't be surprised if this may need to be omitted in some performance-critical cases where the potential of the object escaping the function inhibits key optimisations.

[bvdmitri]

Suggested change

	getindex(::EnvDict, k::AbstractString) = access_env(k->throw(KeyError(ENV, k)), k)
	getindex(ENV::EnvDict, k::AbstractString) = access_env(k->throw(KeyError(ENV, k)), k)

[tecosaur]

Thanks for catching this!

[bvdmitri]

The assumption of ::EnvDict referring to the global ENV is also present in other functions (not written by you). I wonder if it deserve a separate issue? I guess, EnvDict structure is a private API, still, if someone creates its own instance it basically won't really work properly with other functions anyway.

[fingolfin]

EnvDict is a singleton type, so it is fine to refer to "the global ENV" as it is the only instance that can exist anyway.

[vtjnash]

Closes #35955?

[tecosaur]

Not yet, I think once we have this type and use it for nicer messages we can close that issue.

[fingolfin]

Seems OK to me

[aplavin]

I wonder what's the planned direction for exceptions in Julia?
Currently, BoundsError is probably the only heavy one that captures large objects. Some tweaks to make BoundsError lighter are suggested in #56167 (or don't capture the object at all, #43738).
This PR would make KeyError another heavy exception that captures arbitrarily large objects. Seems somewhat contradictory to the direction BoundsError is moving?..

[oscardssmith]

The distinction here (to me) is that BoundsError is an error that we don't expect the compiler to generally remove, while most other error types we expect to be removable at compile time. FieldError won't spuriously be in the generated code for a.f unless the user typos (and won't exist spuriously at all unless a user is doing something like getfield(a, f) where f is a Symbol that the compiler can't figure out the value of). BoundsError is a fairly atypical error in that proving existence or non-existence is relatively difficult, so it is important that potential BoundsErrors don't prevent optimizations for the cases where they won't be executed.

[aplavin]

FieldError won't spuriously be in the generated code <...>

Hmm, maybe I'm missing something, but this PR seems to be about KeyError, not FieldError. And runtime dictionary keys are the common case (unlike field names, there I agree).

[DilumAluthge]

Do we need to patch @test_throws along the lines of #56231? Otherwise, presumably we're at risk of running into #54082 again?

[DilumAluthge]

Also, CI is currently failing on this PR, and (based on a quick glance) it looks like the CI failures are related to the contents of this PR. Therefore, I'll remove the merge me label.

[tecosaur]

Ah yep, seems like a few tests also need to be touched up. I can get to those within the next day or two, but I'm not sure about

compiler/irpasses                                (17) |         failed at 2024-10-27T19:24:14.064
Test Failed at /cache/build/tester-amdci5-10/julialang/julia-master/julia-80d647bfe4/share/julia/test/compiler/irpasses.jl:1568
  Expression: count((x->begin
                #= /cache/build/tester-amdci5-10/julialang/julia-master/julia-80d647bfe4/share/julia/test/compiler/irpasses.jl:1568 =#
                isexpr(x, :invoke)
            end), code) == 0
   Evaluated: 2 == 0

I'll take a look at it, but if anybody might have insight to share on that test, it would be appreciated.

[oscardssmith]

@aplavin that's a good point. I think I'd prefer it if this PR generated a summary

[tecosaur]

Hmm, I see the concern with capturing the object, but I'd also be concerned about the overhead from doing the work to produce a helpful message up-front.

With BoundsError, just capturing the axis is a nice middle ground, but I can't see a similarly nice middleground for KeyError (keys(obj) seems similarly heavy-weight).