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

allow standalone dotted ops, parse .op as (. op) #37583

Merged
merged 9 commits into from
Oct 6, 2020

Conversation

simeonschaub
Copy link
Member

A second attempt at #34156, should supersede #35706. This implements @JeffBezanson's suggestion in #35706 (comment), that we parse .op as (. op), when it isn't functioning as an operator. We then lower this to Base.BroadcastFunction(op), just as in #35706. Could we run PkgEval on this once tests pass to see whether this qualifies as a minor change?
closes #34156

base/broadcast.jl Outdated Show resolved Hide resolved
base/broadcast.jl Outdated Show resolved Hide resolved
@JeffBezanson
Copy link
Member

It didn't occur to me that with this, :.+ and :(.+) are different. A bit of a gotcha, but I'm not sure if we can do anything about it. Are there any dot operators that require the parentheses to be parsed after :? I think the answer is no, which is good since at least you can always just delete the parens if you need to.

@simeonschaub
Copy link
Member Author

simeonschaub commented Sep 15, 2020

You certainly know this better than me, but I think currently every operator, with or without the dot, should be its own token, so you should never actually need those parentheses. Even then, these could still be constructed by using the Symbol constructor directly.

@JeffBezanson JeffBezanson added the parser Language parsing and surface syntax label Sep 15, 2020
@simeonschaub simeonschaub changed the title WIP: allow standalone dotted ops, parse .op as (. op) allow standalone dotted ops, parse .op as (. op) Sep 16, 2020
@simeonschaub
Copy link
Member Author

From my side, if you don't have any further comments, this should now be ready to do a PkgEval run.

@simeonschaub
Copy link
Member Author

The test failures seem unrelated.

@simeonschaub
Copy link
Member Author

Would it be possible to do a PkgEval run now, so this can be discussed in triage tomorrow?

@simeonschaub
Copy link
Member Author

@JeffBezanson Bump. 🙂

@StefanKarpinski StefanKarpinski added the triage This should be discussed on a triage call label Oct 1, 2020
@JeffBezanson
Copy link
Member

👍 from triage.

@JeffBezanson JeffBezanson added the needs pkgeval Tests for all registered packages should be run with this change label Oct 1, 2020
@JeffBezanson
Copy link
Member

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Something went wrong when running your job:

NanosoldierError: error when preparing/pushing to report repo: failed process: Process(`git pull -X ours`, ProcessExited(128)) [128]

Unfortunately, the logs could not be uploaded.
cc @maleadt

@maleadt
Copy link
Member

maleadt commented Oct 3, 2020

PkgEval has migrated systems; let's try this again:

@nanosoldier runtests(ALL, vs = ":master")

@nanosoldier
Copy link
Collaborator

Your package evaluation job has completed - possible new issues were detected. A full report can be found here. cc @maleadt

@simeonschaub
Copy link
Member Author

The failures all look unrelated to this PR. 🥳

@JeffBezanson JeffBezanson removed triage This should be discussed on a triage call needs pkgeval Tests for all registered packages should be run with this change labels Oct 6, 2020
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.

Lower standalone dotted infix operators as anonymous functions
6 participants