Skip to content

Fix de Bruijn index bug in inductive_of_mutfix.find_ind#22127

Open
JasonGross wants to merge 1 commit into
rocq-prover:masterfrom
JasonGross:fix-22125-find-ind-debruijn
Open

Fix de Bruijn index bug in inductive_of_mutfix.find_ind#22127
JasonGross wants to merge 1 commit into
rocq-prover:masterfrom
JasonGross:fix-22125-find-ind-debruijn

Conversation

@JasonGross

@JasonGross JasonGross commented Jun 14, 2026

Copy link
Copy Markdown
Member

Summary

  • Fix a de Bruijn index off-by-one in inductive_of_mutfix.find_ind that caused rocq check (and the kernel guard checker) to fail on fixpoints whose structural argument type contains a de Bruijn reference (e.g., a section Let-bound type alias).
  • The bug was introduced by a46bbfb (9.3) ("Reorder and cleanup in inductive_of_mutfix.find_ind"), which moved push_rel before the find_inductive call. Since ty is valid in the pre-push context, de Bruijn indices in ty were resolved to wrong bindings in the post-push environment.
  • Added a regression test (test-suite/coqchk/bug_22125.v).

Fixes #22125.

Test plan

  • Minimal reproduction: Section S. Let T := list nat. Fixpoint my_length (l : T) ... End S. now passes rocqchk
  • All existing coqchk tests pass
  • Guard checker success/failure tests pass

🤖 Generated with Claude Code

@JasonGross JasonGross requested a review from a team as a code owner June 14, 2026 06:22
@coqbot-app coqbot-app Bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jun 14, 2026
@SkySkimmer

Copy link
Copy Markdown
Contributor

maybe also add a coqc test with

Definition my_length (T:=list nat) := fix my_length (l : T) : nat :=
  match l with
  | nil => O
  | cons _ l' => S (my_length l')
  end.

?

@JasonGross JasonGross force-pushed the fix-22125-find-ind-debruijn branch from deeb4f1 to a3514da Compare June 14, 2026 07:58

@ppedrot ppedrot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed.

@ppedrot ppedrot added this to the 9.3+rc1 milestone Jun 14, 2026
@ppedrot ppedrot added the kind: fix This fixes a bug or incorrect documentation. label Jun 14, 2026
@ppedrot ppedrot self-assigned this Jun 14, 2026
@ppedrot

ppedrot commented Jun 14, 2026

Copy link
Copy Markdown
Member

@coqbot run full ci

@coqbot-app coqbot-app Bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jun 14, 2026
@yannl35133

Copy link
Copy Markdown
Contributor

Why are the tests in two different files, in success? A single file in bugs should be checked by rocq check.

@JasonGross

JasonGross commented Jun 14, 2026

Copy link
Copy Markdown
Member Author

Why are the tests in two different files, in success? A single file in bugs should be checked by rocq check.

I added it in response to @SkySkimmer 's comment, but indeed I don't understand what the second file adds over the existing one (neither in content nor in mechanism of being tested). What is the thing that you believe is missing from the coqchk test @SkySkimmer ?

…type

In `inductive_of_mutfix.find_ind`, `find_inductive` was called on the
type of the structural argument `ty` using the environment *after*
`push_rel`, but `ty` is a term valid in the context *before* the push.
When `ty` contains de Bruijn indices (e.g. a reference to a section
Let-binding), they would be shifted by one, causing `find_inductive`
to resolve them to the wrong binding.

This was introduced by a46bbfb ("Reorder and cleanup in
inductive_of_mutfix.find_ind") which moved `push_rel` before the
`find_inductive` call. The original code correctly used the pre-push
environment.

Fixes rocq-prover#22125.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@JasonGross JasonGross force-pushed the fix-22125-find-ind-debruijn branch from a3514da to 21edf32 Compare June 14, 2026 18:35
@coqbot-app coqbot-app Bot added the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jun 14, 2026
@JasonGross

Copy link
Copy Markdown
Member Author

Files merged. Note that full ci passed before I rebased, but I guess we might as well @coqbot run full ci

@coqbot-app coqbot-app Bot removed the needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. label Jun 14, 2026
@yannl35133

Copy link
Copy Markdown
Contributor

The second test could fail in rocq c directly, instead of requiring rocq check to appear; it's going to be easier to debug if it gets broken again. I imagine there's nothing deeper.

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

Labels

kind: fix This fixes a bug or incorrect documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rocq check fails on coqprime

4 participants