at-atomic fields with at-kwdef and field defaults doesn't work

[quinnj]

Repro:

julia> Base.@kwdef mutable struct AtomicFoo{T}
       @atomic x::Int = 3
       @atomic y::T = :hey
       end
ERROR: LoadError: @atomic modify expression missing field access
Stacktrace:
 [1] error(s::String)
   @ Base ./error.jl:35
 [2] make_atomic(order::Any, a1::Any, op::Any, a2::Any)
   @ Base ./expr.jl:1160
 [3] make_atomic(order::Any, ex::Any)
   @ Base ./expr.jl:1152
 [4] var"@atomic"(__source__::LineNumberNode, __module__::Module, ex::Any)
   @ Base ./expr.jl:1110
 [5] #macroexpand#66
   @ ./expr.jl:122 [inlined]
 [6] macroexpand(m::Module, x::Any)
   @ Base ./expr.jl:120
 [7] var"@kwdef"(__source__::LineNumberNode, __module__::Module, expr::Any)
   @ Base ./util.jl:568
in expression starting at REPL[42]:1

Part of the complications here is that currently the @kwdef macro immediately calls macroexpand on its struct definition expression, thus evaluating the @atomic macro. Also that we're using the @atomic macro for all the regular atomic operations (field access, setpropery, modifyproperty, etc.).

I'm not quite sure what a good solution here is, since we probably want to preserve the @atomic behavior of providing a helpful error message, but allow the specification of a field default if we're in the context of a struct definition expression.

I wonder if there's a smarter way we could handle the @kwdef macro where we don't immediately macroexpand the entire inner definition, since that also provides a solution since we can manually handle the @atomic field::T = default expression and ensure it ends up correct in the returned expression (which is the approach I'm taking in my own definition).

I'm not sure what to do about supporting @static inside a struct definition though, since that really complicates the @kwdef handling if it's not already expanded. 🤷

[vtjnash]

It is because you want to declare an atomic field, not an atomic assignment:

julia> Base.@kwdef mutable struct AtomicFoo{T}
       (@atomic x::Int) = 3
       @atomic(y::T) = :hey
       end

julia> Base.isfieldatomic(AtomicFoo, 1) && Base.isfieldatomic(AtomicFoo, 2)
true

(FWIW, eventually the @atomic x = 3 will also be supported, meaning to declare and set the local/global/captured variable– or as seen in this case, a field–atomically. But that is not implemented in lowering right now.)