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 ± and ∓ as unary operators #34200

Merged
merged 3 commits into from
Mar 10, 2020

Conversation

simeonschaub
Copy link
Member

@simeonschaub simeonschaub commented Dec 26, 2019

This came up on slack, that it would be nice to have ± as an allowed unary operator. This PR also adds . further plus-like operators, which looked enough like + and - to make sense in my mind, but I can also remove the weirder ones, if so desired. Probably should have tests, but I'm not quite sure what to test here exactly.

@JeffBezanson JeffBezanson added the breaking This change will break code label Dec 27, 2019
@JeffBezanson
Copy link
Member

Will have to decide how to transition the parsing of e.g. [1 ±2].

@tkf
Copy link
Member

tkf commented Dec 28, 2019

I suppose merging this PR would make #33859 more breaking?

@JeffBezanson JeffBezanson added the triage This should be discussed on a triage call label Dec 30, 2019
@JeffBezanson
Copy link
Member

Strictly speaking yes, but these operators are used much less often than + and -.

@stevengj
Copy link
Member

stevengj commented Jan 2, 2020

I'm skeptical that all of these operators should be unary. ± and , certainly, but is there any example in the literature of the other operators being unary? It's easier to be conservative here.

@JeffBezanson
Copy link
Member

Probably should not include ≏ "difference between", which doesn't make sense for 1 argument.

@JeffBezanson JeffBezanson added the parser Language parsing and surface syntax label Jan 30, 2020
@JeffBezanson
Copy link
Member

Let's cut this down to just ± and , which are obviously useful, and run PkgEval to see if anything breaks.

@JeffBezanson JeffBezanson removed the triage This should be discussed on a triage call label Jan 30, 2020
@simeonschaub simeonschaub changed the title allow more plus-like unary operators allow ± and ∓ as unary operators Feb 2, 2020
@simeonschaub
Copy link
Member Author

@JeffBezanson That seems like a good plan. Adding further operators can always be discussed later, so it makes sense to keep this more minimal. Could someone trigger PkgEval?

@StefanKarpinski
Copy link
Member

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

@StefanKarpinski
Copy link
Member

I think I did that right? 🤷‍♂

@nanosoldier
Copy link
Collaborator

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

@StefanKarpinski
Copy link
Member

StefanKarpinski commented Feb 4, 2020

None of the failures seem related to this change (a lot of random tests with no seed being set—tsk, tsk) and a couple of segfaults, which are troubling but unlikely to be due to this.

@fredrikekre fredrikekre added needs tests Unit tests are required for this change needs news A NEWS entry is required for this change labels Feb 4, 2020
@simeonschaub
Copy link
Member Author

What is the current state on this? Is this acceptable minor breakage for a next release, or do we need to deprecate this syntax in whitespace sensitive contexts for a intermediary release?

@stevengj
Copy link
Member

stevengj commented Mar 5, 2020

Seems fine for a minor release to me.

@StefanKarpinski
Copy link
Member

Just needs a rebase and merge after CI passes.

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 news A NEWS entry is required for this change needs tests Unit tests are required for this change parser Language parsing and surface syntax
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants