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

refactor: back rfl tactic primarily via apply_rfl #3718

Merged
merged 8 commits into from
Sep 25, 2024

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented Mar 19, 2024

building upon #3714, this (almost) implements the second half of #3302.

The main effect is that we now get a better error message when rfl fails. For

example : n+1+m = n + (1+m) := by rfl

instead of the wall of text

The rfl tactic failed. Possible reasons:
- The goal is not a reflexive relation (neither `=` nor a relation with a @[refl] lemma).
- The arguments of the relation are not equal.
Try using the reflexivity lemma for your relation explicitly, e.g. `exact Eq.refl _` or
`exact HEq.rfl` etc.
n m : Nat
⊢ n + 1 + m = n + (1 + m)

we now get

error: tactic 'rfl' failed, the left-hand side
  n + 1 + m
is not definitionally equal to the right-hand side
  n + (1 + m)
n m : Nat
⊢ n + 1 + m = n + (1 + m)

Unfortunately, because of very subtle differences in semantics (which transparency setting is used when reducing the goal and whether the “implicit lambda” feature applies) I could not make this simply the only rfl implementation. So rfl remains a macro and is still expanded to eq_refl (difference transparency setting) and exact Iff.rfl and exact HEq.rfl (implicit lambda) to not break existing code. This can be revised later, so this still closes: #3302.

A user might still be puzzled why to terms are not defeq. Explaining that better (“reduced to… and reduces to… etc.”) would also be great, but that’s not specific to rfl, so better left for some other time.

@nomeata
Copy link
Collaborator Author

nomeata commented Mar 19, 2024

(Likely needs a stage0 update after 8463000)

@nomeata nomeata force-pushed the joachim/iff-rfl-attrib branch from 09a7c21 to 4c5d6e3 Compare March 19, 2024 13:42
@nomeata nomeata changed the title refactor: let rfl tactic deal with Iff via @[refl] refactor: back rfl tactic only via apply_rfl Mar 19, 2024
@nomeata
Copy link
Collaborator Author

nomeata commented Mar 19, 2024

I am a bit confused now. After a stage0 update, so that

@[builtin_tactic Lean.Parser.Tactic.applyRfl] def evalApplyRfl

kicks in I would have expected that with

syntax (name := applyRfl) "rfl" : tactic

the new code is used reliably.

Nevertheless

theorem add_le_iff_le_sub (a b c : Int) : a + b ≤ c ↔ a ≤ c - b := by
  conv =>
    lhs
    rw [← Int.add_zero c, ← Int.sub_self (-b), Int.sub_eq_add_neg, ← Int.add_assoc, Int.neg_neg,
      Int.add_le_add_iff_right]
  -- rfl here works, so why does the rfl within `conv` no work?

fails with

unsolved goals
abc: Int
⊢ a ≤ c + -b ↔ a ≤ c - b

It seems to me that somehow the

   evalTactic (← `(tactic| try rfl))

in

@[builtin_tactic Lean.Parser.Tactic.Conv.conv] def evalConv : Tactic := fun stx => do

is broken.

Maybe I can work around that, do another stage0 update, and then it works…

@digama0
Copy link
Collaborator

digama0 commented Mar 19, 2024

Your change changed the grammar, so you need to do an update-stage0 after it. The problem is that you removed a syntax node kind for the old refl, but conv and others from stage0 were elaborated with syntax quotations using rfl and so they have the old refl syntax node kind baked in. After removing the old syntax node kind these tactics will therefore produce ill formed syntax (not to mention that you also changed the syntax for applyRfl so even new syntax will be ill-formed, although the identity of the atom in the syntax usually doesn't matter).

@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 19, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 19, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 19, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Mar 19, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Mar 19, 2024

Mathlib CI status (docs):

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 19, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 19, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc March 19, 2024 20:07 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Mar 19, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Mar 19, 2024
@nomeata
Copy link
Collaborator Author

nomeata commented Mar 19, 2024

I still see mathlib failures

 Error: ./././Mathlib/Analysis/NormedSpace/Star/ContinuousFunctionalCalculus/Instances.lean:223:35: error: type mismatch
  HEq.rfl
has type
  HEq ?m.240013 ?m.240013 : Prop
but is expected to have type
  IsSelfAdjoint 2 : Prop

which I did not expect. It seems that the macro_rules are duplicated in std4, proposing a fix at leanprover-community/batteries#702.

@Kha Kha removed the full-ci label May 24, 2024
@nomeata nomeata force-pushed the joachim/iff-rfl-attrib branch from 8be1056 to f33384c Compare September 13, 2024 08:59
@github-actions github-actions bot added the changes-stage0 Contains stage0 changes, merge manually using rebase label Sep 13, 2024
@nomeata nomeata force-pushed the joachim/iff-rfl-attrib branch from f33384c to 763ec08 Compare September 13, 2024 09:32
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 13, 2024
@nomeata nomeata force-pushed the joachim/iff-rfl-attrib branch from 763ec08 to f552ba3 Compare September 13, 2024 10:23
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 13, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 13, 2024
@leanprover-community-bot leanprover-community-bot added the builds-mathlib CI has verified that Mathlib builds against this PR label Sep 19, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc September 20, 2024 08:36 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 20, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 20, 2024
Remove fallback case of `rfl`, to expose good error message

Keep eq_refl in the rfl macro, but put it second

Improve error message

Update some tests

Typo
@nomeata nomeata force-pushed the joachim/iff-rfl-attrib branch from d11d548 to a5edae3 Compare September 21, 2024 09:29
@nomeata nomeata changed the title refactor: back rfl tactic only via apply_rfl refactor: back rfl tactic primarily via apply_rfl Sep 23, 2024
@nomeata nomeata changed the base branch from joachim/rfl-errors to master September 23, 2024 08:09
@github-actions github-actions bot removed the changes-stage0 Contains stage0 changes, merge manually using rebase label Sep 23, 2024
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc September 23, 2024 10:30 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 23, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 23, 2024
@nomeata
Copy link
Collaborator Author

nomeata commented Sep 23, 2024

(Edited…)

Compared the wrong revisions. Actually no significant change, as expected:
http://speed.lean-fro.org/mathlib4/compare/06e47b6a-c46f-4080-9596-b9b454a23113/to/67ef7d7c-5aa6-4d77-9c99-c9ef1755c289

leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 23, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 23, 2024
@nomeata nomeata marked this pull request as ready for review September 25, 2024 09:27
@nomeata nomeata requested a review from leodemoura as a code owner September 25, 2024 09:27
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc September 25, 2024 10:00 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Sep 25, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Sep 25, 2024
@nomeata nomeata added this pull request to the merge queue Sep 25, 2024
Merged via the queue into master with commit a3ca15d Sep 25, 2024
15 checks passed
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 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: A single rfl tactic implementation
5 participants