Skip to content

Use type wrapper directly rather than typename in FieldError #58507

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.

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

Merged

Conversation

MasonProtter
Copy link
Contributor

@MasonProtter MasonProtter commented May 23, 2025

Motivation

FieldError currently prints nameof(exc.type) which just gives a simple symbol for a type's name when a user tries to access a non-existant field.

  1. In https://discourse.julialang.org/t/better-error-message-for-modified-structs-in-julia-1-12/129265 it was argued that this is confusing when a type is redefined since the user isn't told that they're using an old instance, e.g.
julia> struct Point end

julia> p = Point();

julia> struct Point{T}
           x::T
           y::T
       end

julia> p.x
ERROR: FieldError: type Point has no field `x`; Point has no fields at all.
Stacktrace:
 [1] getproperty(x::@world(Point, 38528:38531), f::Symbol)
   @ Base ./Base_compiler.jl:54
 [2] top-level scope
   @ REPL[9]:1

The argument is that this should say something like FieldError: type @world(Point, 38528:38531) has no field x. Though, I will note that the world-info is available in the stacktrace, so it is in some sense redundant.

  1. Relatedly, namespacing info is also lost because of the same mechanism.
julia> module Foo
       struct Point end
       p = Point()
       end;

julia> Foo.p.y
ERROR: FieldError: type Point has no field `y`; Point has no fields at all.
Stacktrace:
 [1] getproperty(x::Main.Foo.Point, f::Symbol)
   @ Base ./Base_compiler.jl:54
 [2] top-level scope
   @ REPL[11]:1

Similarly, it says Point rather than Main.Foo.Point in the error message. This info is also available in the stacktrace.

This PR

This PR simply replaced nameof with .name.wrapper to get the unparameterized version of the type so that it can be printed with the @world decorations and namespace as needed:

julia> struct Point end

julia> p = Point();

julia> struct Point{T}
           x::T
           y::T
       end

julia> p.x
ERROR: FieldError: type @world(Point, 38558:38559) has no field `x`; @world(Point, 38558:38559) has no fields at all.
Stacktrace:
 [1] getproperty(x::@world(Point, 38558:38559), f::Symbol)
   @ Base ./Base_compiler.jl:54
 [2] top-level scope
   @ REPL[24]:1

and

julia> module Foo
       struct Point end
       p = Point()
       end;

julia> Foo.p.y
ERROR: FieldError: type Main.Foo.Point has no field `y`; Main.Foo.Point has no fields at all.
Stacktrace:
 [1] getproperty(x::Main.Foo.Point, f::Symbol)
   @ Base ./Base_compiler.jl:54
 [2] top-level scope
   @ REPL[20]:1

Discussion / doubts

I'm a bit conflicted on whether or not this is a good idea. On one hand, it clutters the error message with info you could get by simply reading the stacktrace. On the other hand, many users are allergic to reading stacktraces and report confusion with things like this.

I'm interested if people have thoughts on the mechanism here, or if there's a better way to surface the worldage problem / namespacing stuff via hints.

@MasonProtter MasonProtter force-pushed the fielderror-changing-fields branch from 8d1a053 to 052b3b7 Compare May 23, 2025 11:31
@giordano giordano added the error messages Better, more actionable error messages label May 23, 2025
@MasonProtter
Copy link
Contributor Author

MasonProtter commented May 23, 2025

One further reason in favour of this which was mentioned on Discourse is that the stacktrace info is not always present if there was inlining:

julia> mutable struct Point
           x::Float64
           y::Float64
       end

julia> const a = Point(1.0, 2.0);

julia> mutable struct Point
           x::Float64
           y::Float64
           z::Float64
       end

julia> @inline f() = a.z;

Before this PR:

julia> f()
ERROR: FieldError: type Point has no field `z`, available fields: `x`, `y`
Stacktrace:
 [1] getproperty
   @ ./Base_compiler.jl:54 [inlined]
 [2] f()
   @ Main ./REPL[4]:1
 [3] top-level scope
   @ REPL[5]:1

After this PR:

julia> f()
ERROR: FieldError: type @world(Point, 38523:38526) has no field `z`, available fields: `x`, `y`
Stacktrace:
 [1] getproperty
   @ ./Base_compiler.jl:54 [inlined]
 [2] f()
   @ Main ./REPL[4]:1
 [3] top-level scope
   @ REPL[8]:1

@oscardssmith oscardssmith added the triage This should be discussed on a triage call label Jun 5, 2025
@MasonProtter
Copy link
Contributor Author

Triage is in favour

@MasonProtter MasonProtter added merge me PR is reviewed. Merge when all tests are passing backport 1.12 Change should be backported to release-1.12 and removed triage This should be discussed on a triage call labels Jun 5, 2025
@IanButterworth IanButterworth merged commit 347fb7c into JuliaLang:master Jun 6, 2025
10 of 12 checks passed
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Jun 6, 2025
KristofferC pushed a commit that referenced this pull request Jun 6, 2025
@KristofferC KristofferC mentioned this pull request Jun 6, 2025
60 tasks
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Jun 7, 2025
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Jun 7, 2025
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Jun 7, 2025
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Jun 7, 2025
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Jun 7, 2025
aviatesk added a commit to aviatesk/JET.jl that referenced this pull request Jun 7, 2025
nilesh646 pushed a commit to nilesh646/julia that referenced this pull request Jun 17, 2025
@KristofferC KristofferC removed the backport 1.12 Change should be backported to release-1.12 label Jul 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
error messages Better, more actionable error messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants