Commit graph

2 commits

Author SHA1 Message Date
Kyle Miller
28cf146d00
fix: make sure monad lift coercion elaborator has no side effects (#6024)
This PR fixes a bug where the monad lift coercion elaborator would
partially unify expressions even if they were not monads. This could be
taken advantage of to propagate information that could help elaboration
make progress, for example the first `change` worked because the monad
lift coercion elaborator was unifying `@Eq _ _` with `@Eq (Nat × Nat)
p`:
```lean
example (p : Nat × Nat) : p = p := by
  change _ = ⟨_, _⟩ -- used to work (yielding `p = (p.fst, p.snd)`), now it doesn't
  change ⟨_, _⟩ = _ -- never worked
```
As such, this is a breaking change; you may need to adjust expressions
to include additional implicit arguments.
2024-11-13 16:22:31 +00:00
Leonardo de Moura
3bd39ed8b6
perf: a isDefEq friendly Fin.sub (#4421)
The performance issue at #4413 is due to our `Fin.sub` definition.
```
def sub : Fin n → Fin n → Fin n
  | ⟨a, h⟩, ⟨b, _⟩ => ⟨(a + (n - b)) % n, mlt h⟩
```
Thus, the following runs out of stack space
```
example (a : UInt64) : a - 1 = a :=
  rfl
```
at the `isDefEq` test
```
(a.val.val + 18446744073709551615) % 18446744073709551616 =?= a.val.val
```

From the user's perspective, this timeout is unexpected since they are
using small numerals, and none of the other `Fin` basic operations (such
as `Fin.add` and `Fin.mul`) suffer from this problem.

This PR implements an inelegant solution for the performance issue. It
redefines `Fin.sub` as
```
def sub : Fin n → Fin n → Fin n
  | ⟨a, h⟩, ⟨b, _⟩ => ⟨((n - b) + a) % n, mlt h⟩
```
This approach is unattractive because it relies on the fact that
`Nat.add` is defined using recursion on the second argument.

The impact on this repo was small, but we want to evaluate the impact on
Mathlib.

closes #4413
2024-06-11 17:18:11 +00:00