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

@edit a[1] no longer works #25474

Closed
KristofferC opened this issue Jan 9, 2018 · 5 comments
Closed

@edit a[1] no longer works #25474

KristofferC opened this issue Jan 9, 2018 · 5 comments
Assignees
Labels
bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version

Comments

@KristofferC
Copy link
Sponsor Member

KristofferC commented Jan 9, 2018

@edit a[1] = 1 still works. Seems to be because of:

julia> Meta.lower(@__MODULE__, :(a[1] = 1))
:(begin
      setindex!(a, 1, 1)
      return 1
  end)

julia> Meta.lower(@__MODULE__, :(a[1])) # Wat
:($(Expr(:thunk, CodeInfo(:(begin
      SSAValue(0) = getindex(a, 1)
      return SSAValue(0)
  end)))))
@KristofferC KristofferC added bug Indicates an unexpected problem or unintended behavior regression Regression in behavior compared to a previous version labels Jan 9, 2018
@KristofferC KristofferC changed the title @edit a[1] no longer works @edit a[1] no longer works Jan 9, 2018
@JeffBezanson
Copy link
Sponsor Member

Calling lower is probably no longer a good idea here. Note that the setindex case works because the code for @edit painstakingly looks for that exact result from lower. I think it will be easier to manually write cases for syntax forms like these that have a (theoretically) simple correspondence with function calls.

@KristofferC
Copy link
Sponsor Member Author

KristofferC commented Jan 10, 2018

FWIW, I think it has been there since the feature originally got introduced, 5 years ago:85fdc92#diff-c20eb2eead14027fab4293e10bacf73bR110

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 10, 2018

I think we could still call Meta.lower, but then scan the results looking for 1. the call argument to return (following SSAValues) 2. that all other operations are in some whitelist (conditionals, variables, calls, constants, labels, metadata, etc.)

@JeffBezanson JeffBezanson added this to the 1.0.x milestone Jan 11, 2018
@tkoolen
Copy link
Contributor

tkoolen commented Jul 16, 2018

In similar vein, @edit [3], @edit [3 4], and @edit [3; 4] are broken.

Is there a way to be sure that all of such cases are handled? #28122 is great, but I'm afraid this is a game of whack-a-mole.

Also, I have a package that was relying on expand (now Meta.lower, but the output is vastly different) to avoid having special cases for all of these. Could this be reinstated in some form or another?

@JeffBezanson
Copy link
Sponsor Member

Some of these are fixed but setindex! is still broken.

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 regression Regression in behavior compared to a previous version
Projects
None yet
Development

No branches or pull requests

4 participants