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

feat: set priority in monadic class instances #5291

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

JovanGerb
Copy link
Contributor

@JovanGerb JovanGerb commented Sep 9, 2024

I added the high priority to these instances with monad transformers

  • MonadReaderOf ρ (ReaderT ρ m)
  • MonadWithReaderOf ρ (ReaderT ρ m)
  • MonadStateOf σ (StateT σ m)
  • MonadStateOf σ (EStateT ε σ m)
  • MonadExceptOf ε (ExceptT ε m)
  • MonadExceptOf ε (EStateT ε σ m)
  • MonadExceptOf ε (ExceptCpsT ε m)
  • MonadCallStackOf κ (CallStackT κ m)
  • MonadCycleOf κ (CycleT κ m)
  • MonadStore1Of k (β k) m

I added the low priority to this instance with a monad transformer

  • MonadExceptOf Unit (OptionT m).

I added the low priority to MonadExcept.instOrElse, because it needs to have lower priority than instOrElseOfAlternative (this change doesn't affect the synthesis order, because the order of the files in which the instances were declared was already 'correct').

In Prelude.lean, instead of setting the priority to high or low, I had to set it to 10000 or 100, which is the same.

The increased priority of MonadExceptOf ε (ExceptT ε m) affected a few files that had to be fixed.

Closes #4212

@JovanGerb JovanGerb changed the title set high priority in monadic class instances feat: set high priority in monadic class instances Sep 9, 2024
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Sep 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 9, 2024
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Sep 9, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

@JovanGerb
Copy link
Contributor Author

The test that broke had the number from the name generator offset by 8.
The line local infix:65 (priority := high) "*" => mul previously used to increment the name generator by 1560, and now this became 1568. I don't understand where these 8 extra names come from, but I equally don't understand why a simple infix declaration needs to use so ridiculously many names.

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 9, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 9, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 10, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 10, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 11, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 11, 2024
@leanprover-community-bot
Copy link
Collaborator

Mathlib CI status (docs):

@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 6aa0c46b04e78011af49eb4f272ea07e9173e584 --onto adfd6c090ef7d6bdcc8f69c5dd6f919b1b71cab3. (2024-09-12 11:37:11)

@leanprover-bot leanprover-bot added the P-medium We may work on this issue if we find the time label Sep 13, 2024
@JovanGerb JovanGerb changed the title feat: set high priority in monadic class instances feat: set priority in monadic class instances Sep 13, 2024
@kim-em
Copy link
Collaborator

kim-em commented Sep 26, 2024

!bench

@leanprover-bot
Copy link
Collaborator

Here are the benchmark results for commit 90ac0f7.
There were significant changes against commit 6aa0c46:

  Benchmark   Metric       Change
  ========================================
- rbmap       task-clock     2.2% (23.0 σ)
- rbmap       wall-clock     2.3% (24.8 σ)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builds-mathlib CI has verified that Mathlib builds against this PR P-medium We may work on this issue if we find the time toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RFC: type class synthesis order when outParam has multiple possible values
5 participants