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

[Merged by Bors] - fix: set big-operator parsing precedence to 67 #1723

Closed
wants to merge 2 commits into from

Conversation

dwrensha
Copy link
Member

@dwrensha dwrensha commented Jan 20, 2023

Sets all big-operator precedences to 67, in keeping with the existing library note and with lean 3.

Online discussions, such as https://math.stackexchange.com/q/185538/30839
seem to suggest that `∏` and `∑` should have the same precedence,
and that this should be somewhere between `*` and `+`.
The latter have precedence levels `70` and `65` respectively,
and we therefore choose the level `67`.

Fixes downstream code that relied on the wrong precedences. In all cases, the change in this PR makes the code match the parenthesization of the mathlib3 version.

I came across this problem when I tried to write

lemma sum_range_square (n : ℕ) :
    ∑i in Finset.range n, (i+1)^2 = n * (n + 1) * (2*n + 1)/6 := sorry

and I got the error

failed to synthesize instance
  AddCommMonoid Prop

This PR fixes that error, so that my example successfully parses as expected.

@@ -119,14 +119,14 @@ macro_rules (kind := bigprod)

/-- `∑ x in s, f x` is notation for `Finset.sum s f`. It is the sum of `f x`,
where `x` ranges over the finite set `s`. -/
syntax (name := bigsumin) "∑ " extBinder "in " term "," term : term
syntax (name := bigsumin) "∑ " extBinder "in " term "," term:51 : term
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The library note around lines 84~100 above suggests that we use precedence level 67 everywhere. Should that also work in Lean 4?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Done.

@dwrensha dwrensha changed the title fix: set precedence for bigsumin and bigprodin macros fix: set big-operator parsing precedence to 67 Jan 20, 2023
@jcommelin
Copy link
Member

Thanks 🎉

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Jan 20, 2023
bors bot pushed a commit that referenced this pull request Jan 20, 2023
Sets all big-operator precedences to 67, in keeping with the existing library note and with lean 3. https://github.com/leanprover-community/mathlib4/blob/82cf1905168dc57e7d95ce5576b7991b31e4f6ea/Mathlib/Algebra/BigOperators/Basic.lean#L89-L93

Fixes downstream code that relied on the wrong precedences. In all cases, the change in this PR makes the code match the parenthesization of the mathlib3 version.

I came across this problem when I tried to write
```lean
lemma sum_range_square (n : ℕ) :
    ∑i in Finset.range n, (i+1)^2 = n * (n + 1) * (2*n + 1)/6 := sorry
```
and I got the error 
```
failed to synthesize instance
  AddCommMonoid Prop
```

This PR fixes that error, so that my example successfully parses as expected.
@bors
Copy link

bors bot commented Jan 20, 2023

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix: set big-operator parsing precedence to 67 [Merged by Bors] - fix: set big-operator parsing precedence to 67 Jan 20, 2023
@bors bors bot closed this Jan 20, 2023
@bors bors bot deleted the big-operators-precedence branch January 20, 2023 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-to-merge This PR has been sent to bors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants