Skip to content

refactor: simplify grind canonicalizer and fix preprocessing issues#13149

Merged
leodemoura merged 7 commits intomasterfrom
lean-canon-refactor
Mar 27, 2026
Merged

refactor: simplify grind canonicalizer and fix preprocessing issues#13149
leodemoura merged 7 commits intomasterfrom
lean-canon-refactor

Conversation

@leodemoura
Copy link
Copy Markdown
Member

This PR simplifies the grind canonicalizer by removing dead state and unnecessary
complexity, and fixes two bugs discovered during the cleanup.

Changes

Canonicalizer cleanup:

  • Remove dead Canon.State.canon field — values were inserted but never read.
    The canonicalizer uses a transient HashMap local to each canonImpl invocation.
  • Remove proofCanon — it deduplicated Grind.nestedProof terms by mapping
    canonicalized propositions to a single representative, but different proofs may
    reference different hypotheses, making the result context-dependent and preventing
    cache sharing across goals.
  • Remove isDefEqBounded — a fallback that retried isDefEq at default transparency
    with a heartbeat budget. The one test that depended on it was actually masking a
    transparency bug in propagateCtorHomo.

Bug fixes:

  • Use withDefault for mkAppOptM in propagateCtorHomo (Ctor.lean) — the
    injectivity proof construction needs default transparency to unify implicit
    arguments of indexed inductive types like Vector.
  • Add Grind.abstractFn gadget to protect lambda abstractions created by
    abstractGroundMismatches? from beta reduction during preprocessing. Without
    this, Core.betaReduce in preprocessLight collapses (fun x => body) arg
    back to body[arg/x], undoing the abstraction that congruence closure needs.

Eta reduction infrastructure:

  • Lower etaReduceAll from MetaM to CoreM — it only performs structural
    operations, no MetaM needed.
  • Add etaReduceWithCache that takes and returns an explicit HashMap cache,
    enabling callers to thread a single cache across multiple expressions.

The net effect on Canon.State is removing 3 fields (canon, proofCanon)
and the isDefEqBounded function, along with the useIsDefEqBounded and
parent parameters from canonElemCore.

leodemoura and others added 7 commits March 26, 2026 06:50
The `Canon.State.canon : PHashMap Expr Expr` field was written to but
never read. The canonicalizer uses a transient `HashMap` local to each
`canonImpl` invocation for its expression cache, making the persistent
field pure dead state.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`proofCanon` deduplicated `Grind.nestedProof p _` terms by mapping
canonicalized propositions to a single representative. This prevented
cache sharing across goals because different proofs may reference
different hypotheses, making the result context-dependent.

Without `proofCanon`, structurally identical propositions with different
proof terms become distinct in congruence closure. This adds a small
number of extra terms but enables a simpler, shareable cache. The cost
shrinks further once `GetElemV` eliminates most `nestedProof` terms.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`propagateCtorHomo` constructs injectivity proofs via `mkAppOptM`, which
unifies implicit arguments at the current transparency. For indexed
inductive types like `Vector`, the implicit size argument may require
default transparency to unify (e.g., `({ toList := [] }.mapFinIdx f).size`
with `0`). Without this, `mkAppOptM` throws `throwAppTypeMismatch`.

Previously, `isDefEqBounded` in the canonicalizer masked this by eagerly
unifying such arguments during canonicalization. This fix addresses the
root cause, enabling the removal of `isDefEqBounded`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`abstractGroundMismatches?` constructs terms of the form
`(fun x => body) arg` for congruence closure. Beta reduction in
`preprocessLight` was collapsing these back to `body[arg/x]`, undoing
the abstraction.

Wrap the lambda with `Grind.abstractFn` (a non-reducible identity) so
that no preprocessing pass can reduce the application. Congruence closure
still detects the equality via congruence on the shared `abstractFn f`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`isDefEqBounded` was a fallback in `canonElemCore` that retried `isDefEq`
at default transparency with a heartbeat budget. The only test that
depended on it (`grind_lint_misc`) was fixed by using default
transparency in `propagateCtorHomo` (commit b26706b).

This also removes the `useIsDefEqBounded` and `parent` parameters from
`canonElemCore`, simplifying the canonicalizer interface.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`etaReduceAll` only performs structural eta reduction without any
`MetaM` operations. Lower it to `CoreM` using `Core.transform`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…cit cache

`etaReduceWithCache` takes and returns a `HashMap ExprPtr Expr` cache
explicitly, enabling the canonicalizer to thread a single cache across
multiple expressions. The traversal eta-reduces lambdas eagerly: if a
lambda is eta-reducible, it visits the reduced form directly instead of
descending into the binder types.

`etaReduceAll` is now a thin wrapper around `etaReduceWithCache`.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@leodemoura leodemoura added the changelog-tactics User facing tactics label Mar 27, 2026
@leodemoura leodemoura enabled auto-merge March 27, 2026 04:42
@leodemoura leodemoura added this pull request to the merge queue Mar 27, 2026
Merged via the queue into master with commit e55f69a Mar 27, 2026
23 checks passed
@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 Mar 27, 2026
@mathlib-lean-pr-testing
Copy link
Copy Markdown

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase a8bbc95d9fe665255a5c98fb402546233e760636 --onto 4786e082dca22873d14d2a5b9b7c8843380c6e78. You can force Mathlib CI using the force-mathlib-ci label. (2026-03-27 05:34:05)

@leanprover-bot
Copy link
Copy Markdown
Collaborator

Reference manual CI status:

  • ❗ Reference manual CI will not be attempted unless your PR branches off the nightly-with-manual branch. Try git rebase a8bbc95d9fe665255a5c98fb402546233e760636 --onto cfa8c5a036d6990635c6ec50b02d0e806995cec3. You can force reference manual CI using the force-manual-ci label. (2026-03-27 05:34:07)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog-tactics User facing tactics 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.

2 participants