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

termination_by? over generalizes the decreasing term #4230

Closed
3 tasks done
hargoniX opened this issue May 20, 2024 · 1 comment · Fixed by #4329
Closed
3 tasks done

termination_by? over generalizes the decreasing term #4230

hargoniX opened this issue May 20, 2024 · 1 comment · Fixed by #4329
Assignees
Labels
bug Something isn't working

Comments

@hargoniX
Copy link
Contributor

Prerequisites

Please put an X between the brackets as you perform the following steps:

Description

The following (not quite correct theorem) causes an annoyance with termination_by?:

def copy (curr : Nat) (input : Array Nat) (output : Array Nat) : Array Nat :=
  if hcurr:curr < input.size then
    copy (curr + 1) input (output.push (input[curr]'hcurr))
  else
    output
termination_by input.size - curr

theorem foo (curr : Nat) (input : Array Nat) (output : Array Nat)
    : ∀ (idx : Nat) (hidx1 : idx < curr),
        (copy curr input output)[idx]'sorry
          =
        output[idx]'sorry := by
  intro idx hidx
  unfold copy
  split
  . rw [foo]
    . rw [Array.get_push_lt]
    . omega
  . rfl
termination_by?

Here termination_by? suggests:

Try this: termination_by idx => input.size - curr

Which, while correct, does contain the unused variable idx and will get flagged by the unused
variable linter. It would be great if it only did this for variables that are actually used in
the decreasing term.

Context

This was discovered as part of my work on LeanSAT where lots of theorems of this form (that are actually correct due to additional constraints on the indices) produced slightly "wrong" termination_by? terms.

Expected behavior: Suggest input.size - curr

Actual behavior: Suggest idx => input.size - curr

Versions

"4.8.0-rc1"

Impact

Add 👍 to issues you consider important. If others are impacted by this issue, please ask them to add 👍 to it.

@hargoniX hargoniX added the bug Something isn't working label May 20, 2024
@nomeata
Copy link
Collaborator

nomeata commented May 20, 2024

Thanks! I think when termination_by? was added, the unused variable linter wouldn’t work for termination_by

Probably just a matter of using the delaborator for binders correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants