Skip to content

Fixes #19661: incorrect reduction of inner fixpoints with extra arguments in fixpoint guard checker#19671

Merged
coqbot-app[bot] merged 2 commits intorocq-prover:masterfrom
herbelin:master+fix19661-guard-with-uniform-parameters
Oct 15, 2024
Merged

Fixes #19661: incorrect reduction of inner fixpoints with extra arguments in fixpoint guard checker#19671
coqbot-app[bot] merged 2 commits intorocq-prover:masterfrom
herbelin:master+fix19661-guard-with-uniform-parameters

Conversation

@herbelin
Copy link
Copy Markdown
Member

@herbelin herbelin commented Oct 9, 2024

Was introduced in 140908d, PR #17986, 8.20.0.

Fixes / closes #19661

The original report raises an anomaly but it is not excluded that the bug allows to bypass the guard condition without failing (one would need two nested beta-redexes across a match, each respectively on an argument in the same inductive type, one having a well-founded body, the other not, and the first one being wrongly used in place of the second one or something like that). and seems otherwise kernel-safe (see next comment).

  • Added / updated test-suite.
  • Added changelog.

@herbelin herbelin added the kind: fix This fixes a bug or incorrect documentation. label Oct 9, 2024
@herbelin herbelin added this to the 8.20.1 milestone Oct 9, 2024
@herbelin herbelin requested review from a team as code owners October 9, 2024 19:50
@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 Oct 9, 2024
herbelin added a commit to herbelin/github-coq that referenced this pull request Oct 9, 2024
@ppedrot
Copy link
Copy Markdown
Member

ppedrot commented Oct 10, 2024

@herbelin the first commit has nothing to do with the sauerkraut.

@herbelin
Copy link
Copy Markdown
Member Author

it is not excluded that the bug allows to bypass the guard condition without failing

On second thought, the bug seems kernel-safe. Basically, what the bug does is to reduce (fix f x := phi(f,x)) constructor args into the incorrect phi(fix f x := phi(f,x),constructor) args args rather than phi(fix f x := phi(f,x),constructor) args. The extra arguments can eventually be substituted to a lambda in phi instead of this lambda remaining abstract. This means that more (ill-typed) reduction paths are possibly checkled guarded but all (well-typed) reduction paths that have to be checked guarded are still checked guarded.

@herbelin herbelin added part: kernel part: fixpoints About Fixpoint, fix and mutual statements labels Oct 10, 2024
…tch.

Commit 140908d (PR rocq-prover#17986, about the extrusion of uniform parameters)
was passing the wrong extra arguments of a match.
@herbelin herbelin added the request: full CI Use this label when you want your next push to trigger a full CI. label Oct 10, 2024
herbelin added a commit to herbelin/github-coq that referenced this pull request Oct 10, 2024
@herbelin herbelin force-pushed the master+fix19661-guard-with-uniform-parameters branch from 266f95c to 73a6319 Compare October 10, 2024 07:51
@coqbot-app coqbot-app bot removed request: full CI Use this label when you want your next push to trigger a full CI. needs: full CI The latest GitLab pipeline that ran was a light CI. Say "@coqbot run full ci" to get a full CI. labels Oct 10, 2024
Co-authored-by: Jim Fehrle <jim.fehrle@gmail.com>
@herbelin herbelin added the request: full CI Use this label when you want your next push to trigger a full CI. label Oct 10, 2024
@herbelin herbelin force-pushed the master+fix19661-guard-with-uniform-parameters branch from 73a6319 to b9e6960 Compare October 10, 2024 08:01
@coqbot-app coqbot-app bot removed the request: full CI Use this label when you want your next push to trigger a full CI. label Oct 10, 2024
@ppedrot ppedrot self-assigned this Oct 10, 2024
@herbelin herbelin changed the title Fixes #19661: passing wrong arguments in guard checker in the presence of beta-redexes across nested match. Fixes #19661: incorrect reduction of inner fixpoints with extra arguments in fixpoint guard checker Oct 10, 2024
Copy link
Copy Markdown
Member

@ppedrot ppedrot left a comment

Choose a reason for hiding this comment

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

Probably fine, as the new code does seem to correspond to the intended semantics. Nonetheless this is the guard condition so as usual fine prints may apply.

@ppedrot
Copy link
Copy Markdown
Member

ppedrot commented Oct 12, 2024

I'm leaving a bit of time for other kernel experts to weigh in.

@ppedrot
Copy link
Copy Markdown
Member

ppedrot commented Oct 15, 2024

@coqbot merge now

@coqbot-app coqbot-app bot merged commit 6aaac0a into rocq-prover:master Oct 15, 2024
proux01 pushed a commit to proux01/rocq that referenced this pull request Oct 16, 2024
Co-authored-by: Jim Fehrle <jim.fehrle@gmail.com>
(cherry picked from commit b9e6960)
proux01 added a commit to proux01/rocq that referenced this pull request Oct 16, 2024
…uction of inner fixpoints with extra arguments in fixpoint guard checker
proux01 added a commit to proux01/rocq that referenced this pull request Oct 17, 2024
…uction of inner fixpoints with extra arguments in fixpoint guard checker
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. part: fixpoints About Fixpoint, fix and mutual statements part: kernel

Projects

Status: ...

Development

Successfully merging this pull request may close these issues.

Anomaly when trying to detect decreasing fixpoint argument in 8.20

3 participants