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

RFC: fix #18650, parsing generator expressions containing macro calls #22943

Merged
merged 1 commit into from
Aug 3, 2017

Conversation

JeffBezanson
Copy link
Member

Previously @mac x for i = ... always parsed as a macro call wrapping a for loop, except inside square brackets (giving a comprehension). Now it will parse as a generator when it occurs inside a function call argument list.

This is very lightly breaking, since (1) it's very unusual to write a for loop inside an argument list, (2) most (all?) forms of this either gave a syntax error before, or will give a syntax error after this change.

I'm a bit on the fence about this. The new parsing is more useful, but means that in f(<something>) vs. (<something>), the <something> can parse differently.

@JeffBezanson JeffBezanson added breaking This change will break code parser Language parsing and surface syntax labels Jul 24, 2017
@ararslan
Copy link
Member

The new parsing is more useful, but means that in f(<something>) vs. (<something>), the <something> can parse differently.

That seems weird and surprising to me. I'm not sure it would be beneficial to do that, especially because just using parentheses for macro calls inside of generator expressions is clearer anyway.

@JeffBezanson JeffBezanson added the needs decision A decision on this change is needed label Jul 25, 2017
@JeffBezanson
Copy link
Member Author

@StefanKarpinski What do you think?

@StefanKarpinski
Copy link
Member

I think this is a better tradeoff. The sum(@m x for i = 1:n end) syntax is pretty strange and seems better to require parens for than the mean(@elapsed sleep(0.1) for _ = 1:0) syntax, which actually seems quite useful. If we ever wanted to support both we could have a parser with arbitrary backtracking :trollface:.

@JeffBezanson
Copy link
Member Author

Yeah, I think I agree. Either way you can always get what you want by adding enough parentheses, but this change makes the property "does what you want or else throws an error" hold in more cases.

@JeffBezanson JeffBezanson merged commit 5913f24 into master Aug 3, 2017
@JeffBezanson JeffBezanson deleted the jb/fix18650 branch August 3, 2017 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code needs decision A decision on this change is needed parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants