Navigation Menu

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

Fixes #15451: incompletenesses in guard condition regarding cofix and primitive projections #15498

Merged

Conversation

herbelin
Copy link
Member

@herbelin herbelin commented Jan 17, 2022

Kind: Fix

Two issues:

  • the case of a cofix was missing when detecting the presence of a potential iota-redex to reduce
  • the stack was discarded when reducing a primitive projection

Fixes #15451

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

@herbelin herbelin added kind: fix This fixes a bug or incorrect documentation. part: kernel part: inductives Inductive types, fixpoints, etc. labels Jan 17, 2022
@herbelin herbelin added this to the 8.15.1 milestone Jan 17, 2022
@herbelin herbelin requested a review from a team as a code owner January 17, 2022 18:55
herbelin added a commit to herbelin/github-coq that referenced this pull request Jan 17, 2022
herbelin added a commit to herbelin/github-coq that referenced this pull request Jan 17, 2022
@herbelin herbelin force-pushed the master+fix15451-incompleteness-guard-condition branch from 0a42adf to 9026195 Compare January 17, 2022 19:50
| Construct _ ->
check_rec_call renv []
| Construct _ | CoFix _ ->
check_rec_call renv stack
(Term.applist (mkProj(Projection.unfold p,c), l))
| _ -> Exninfo.iraise exn
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not make this explicit so we definitely don't forget any cases?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. We could but there are a lot of cases to add. Wondering what others would do themselves.

Copy link
Member

Choose a reason for hiding this comment

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

I think that in the kernel at least, it is considered good practice to fully enumerate the constructors of constr.

Copy link
Member

Choose a reason for hiding this comment

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

I agree, the catch-all bit me once in the guard as well when we introduced primitive projections.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK, done.

@mattam82 mattam82 self-assigned this Jan 18, 2022
@herbelin herbelin force-pushed the master+fix15451-incompleteness-guard-condition branch from 9026195 to 359dafd Compare January 18, 2022 16:52
@ppedrot ppedrot assigned ppedrot and unassigned mattam82 Jan 28, 2022
@ppedrot
Copy link
Member

ppedrot commented Jan 28, 2022

This one is fairly simple and there is little risk of anything going wrong, so let me assign and merge.

@ppedrot
Copy link
Member

ppedrot commented Jan 28, 2022

@coqbot merge now

@coqbot-app coqbot-app bot merged commit b40983c into coq:master Jan 28, 2022
@coqbot-app coqbot-app bot added this to Request 8.15.1 inclusion in Coq 8.15 Jan 28, 2022
SkySkimmer pushed a commit to SkySkimmer/coq that referenced this pull request Mar 18, 2022
(cherry picked from commit 359dafd)
SkySkimmer added a commit to SkySkimmer/coq that referenced this pull request Mar 18, 2022
…dition regarding cofix and primitive projections
@coqbot-app coqbot-app bot moved this from Request 8.15.1 inclusion to Shipped in 8.15.1 in Coq 8.15 Mar 18, 2022
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: inductives Inductive types, fixpoints, etc. part: kernel
Projects
No open projects
Coq 8.15
  
Shipped in 8.15.1
4 participants