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

fix #37393: interpolation in for outer #37402

Merged
merged 1 commit into from
Oct 17, 2020

Conversation

simeonschaub
Copy link
Member

This also simplifies the logic a bit. This does now throw an error during lowering for for outer +i = 1:3; end, which was technically valid before, but I don't think that should be a problem?
fixes #37393

@simeonschaub simeonschaub force-pushed the fix_37393 branch 2 times, most recently from aaae557 to 36473b1 Compare September 4, 2020 18:28
@JeffBezanson
Copy link
Member

The simplification is nice but a bit too good to be true --- we allowed for outer in x so that outer still generally works as a variable (not stealing it as a keyword).

@JeffBezanson JeffBezanson added the parser Language parsing and surface syntax label Sep 4, 2020
@simeonschaub
Copy link
Member Author

Oh, you are right! I misunderstood what parse-pipe< does. I think we should be able to rewrite most of parse-iteration-spec using parse-eq* though. Do you agree with that approach?

@simeonschaub
Copy link
Member Author

I also can't quite figure out what this line is for:

((and (eq? lhs ':) (closing-token? t))

Is this even needed anymore?

@JeffBezanson
Copy link
Member

I think the : line is for the discontinued comprehension syntax:

julia> [i for i = 1:10, :]
ERROR: syntax: comprehension syntax with `:` ranges has been removed around REPL[4]:1

parse-eq* might work, hard to say without trying it :)

@simeonschaub
Copy link
Member Author

parse-eq* might work, hard to say without trying it :)

It almost works. Unfortunately, the current parse-iteration-spec allows

[i for i
in 1:3]

as valid syntax, which I guess was just an oversight, but since it's actually used in

, I guess we still need to allow that?

@simeonschaub
Copy link
Member Author

Ok, this should now be ready to review. It keeps the (somewhat questionable) current behavior regarding newlines, while still fixing #37393.

@simeonschaub
Copy link
Member Author

@JeffBezanson Would you mind taking another look at this?

@JeffBezanson JeffBezanson added the merge me PR is reviewed. Merge when all tests are passing label Oct 15, 2020
@simeonschaub
Copy link
Member Author

I am going to rebase once #37247 is merged

@JeffBezanson JeffBezanson merged commit 912cd3f into JuliaLang:master Oct 17, 2020
@DilumAluthge DilumAluthge removed the merge me PR is reviewed. Merge when all tests are passing label Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Quasiquotation in for outer $var... expressions does not work
3 participants