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.

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

Evaluation order in LHS struct destructuring #44013

Closed
Keno opened this issue Feb 2, 2022 · 2 comments
Closed

Evaluation order in LHS struct destructuring #44013

Keno opened this issue Feb 2, 2022 · 2 comments
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage)

Comments

@Keno
Copy link
Member

Keno commented Feb 2, 2022

Consider:

julia> struct Foo
          x
          f
       end

julia> f = Foo(1, 2)
Foo(1, 2)

we have:

julia> (; x, f) = f
2

but

julia> (; f, x) = f
ERROR: type Int64 has no field x
Stacktrace:
 [1] getproperty(x::Int64, f::Symbol)
   @ Base ./Base.jl:38
 [2] top-level scope
   @ REPL[6]:1

Also in the first case, the overall return is the new value of f, but I believe it would be more consistent to return the old value of f. I think the syntax should work in phases, where it collects all the values into ssa values and only then applies the updates.

@Keno Keno added the parser Language parsing and surface syntax label Feb 2, 2022
@JeffBezanson JeffBezanson added compiler:lowering Syntax lowering (compiler front end, 2nd stage) and removed parser Language parsing and surface syntax labels Feb 2, 2022
@JeffBezanson
Copy link
Sponsor Member

Yes, that is how everything else works:

julia> x = [3,4];

julia> x, y = x
2-element Vector{Int64}:
 3
 4

julia> x
3

julia> y
4

This is a bug.

@simeonschaub simeonschaub self-assigned this Feb 2, 2022
@simeonschaub simeonschaub added the bug Indicates an unexpected problem or unintended behavior label Feb 2, 2022
@Keno
Copy link
Member Author

Keno commented Feb 2, 2022

I guess the correct way to say it would be that the rhs should be assigned to an ssavalue first rather than being reused.

LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Feb 22, 2022
KristofferC pushed a commit that referenced this issue Feb 23, 2022
LilithHafner pushed a commit to LilithHafner/julia that referenced this issue Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Development

No branches or pull requests

3 participants