Skip to content

Inductive cumulativity supports sort cumulativity (Prop <= Type) - #21773

Closed
SkySkimmer wants to merge 1 commit into
rocq-prover:masterfrom
SkySkimmer:cumul-sort-poly
Closed

Inductive cumulativity supports sort cumulativity (Prop <= Type)#21773
SkySkimmer wants to merge 1 commit into
rocq-prover:masterfrom
SkySkimmer:cumul-sort-poly

Conversation

@SkySkimmer

@SkySkimmer SkySkimmer commented Mar 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@SkySkimmer
SkySkimmer requested review from a team as code owners March 18, 2026 11:34
@SkySkimmer SkySkimmer added the needs: changelog entry This should be documented in doc/changelog. label Mar 18, 2026
@SkySkimmer
SkySkimmer requested review from a team as code owners March 18, 2026 11:34
@SkySkimmer SkySkimmer added the request: full CI Use this label when you want your next push to trigger a full CI. label Mar 18, 2026
@SkySkimmer
SkySkimmer requested a review from a team as a code owner March 18, 2026 11:34
@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 Mar 18, 2026
@SkySkimmer SkySkimmer mentioned this pull request Mar 18, 2026
Closed
4 tasks
@SkySkimmer SkySkimmer added this to the 9.3+rc1 milestone Mar 18, 2026
@SkySkimmer SkySkimmer added the request: full CI Use this label when you want your next push to trigger a full CI. label Mar 18, 2026
@SkySkimmer
SkySkimmer requested a review from a team as a code owner March 18, 2026 11:39
@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 Mar 18, 2026
SkySkimmer added a commit to SkySkimmer/coq-elpi that referenced this pull request Mar 18, 2026
@SkySkimmer
Adapt to rocq-prover/rocq#21773 (sort polymorphic cumulativity)
7bf2237
SkySkimmer added a commit to SkySkimmer/MetaRocq that referenced this pull request Mar 18, 2026
@SkySkimmer
Adapt to rocq-prover/rocq#21773 (sort poly cumulativity)
defb2c2
@SkySkimmer SkySkimmer added the request: full CI Use this label when you want your next push to trigger a full CI. label Mar 18, 2026
@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 Mar 18, 2026
@SkySkimmer SkySkimmer added the request: full CI Use this label when you want your next push to trigger a full CI. label Mar 18, 2026
@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 Mar 18, 2026
@ppedrot ppedrot self-assigned this Mar 18, 2026
@SkySkimmer SkySkimmer added kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. request: full CI Use this label when you want your next push to trigger a full CI. and removed needs: changelog entry This should be documented in doc/changelog. labels Mar 18, 2026
@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 Mar 18, 2026
@SkySkimmer SkySkimmer added the needs: fixing The proposed code change is broken. label Mar 19, 2026
@SkySkimmer

SkySkimmer commented Mar 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I think the following two files fail with this PR (I haven't quite narrowed it down to being this PR rather than another one in my stack of patches):

first one fails in master (050e23e) and 9.2 (#21781)
second one succeeds in the PR

@SkySkimmer SkySkimmer added request: full CI Use this label when you want your next push to trigger a full CI. and removed needs: fixing The proposed code change is broken. labels Mar 20, 2026
@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 Mar 20, 2026
@SkySkimmer
SkySkimmer requested a review from ppedrot March 20, 2026 16:36
ppedrot
ppedrot approved these changes Mar 23, 2026
View reviewed changes

@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.

Sounds fine now (famous last words).

@ppedrot

ppedrot commented Mar 23, 2026

Copy link
Copy Markdown
Member

I'm leaving a bit of time before merging for others to chime in and double check that we did not miss a case both in the code and in the theory.

@github-actions github-actions Bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Mar 24, 2026
@SkySkimmer SkySkimmer added the request: full CI Use this label when you want your next push to trigger a full CI. label Mar 24, 2026
yannl35133
yannl35133 reviewed Mar 24, 2026
View reviewed changes

@yannl35133 yannl35133 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This feature needs documentation (universe cumulative inductives also lack a formal specification in the reference manual).

Comment thread kernel/indtypes.ml Outdated
Comment on lines +580 to +583
let mk nsec variance =
(Array.sub variance nsec (Array.length variance - nsec)),
(Array.sub variance 0 nsec)
in

@yannl35133 yannl35133 Mar 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't that Array.chop flipped?

Comment thread kernel/indTyping.ml Outdated
Comment on lines +604 to +605
let quals = Array.map2 (fun a b -> a,b) quals qvariances in
let univs = Array.map2 (fun a b -> a,b) univs uvariances in

@yannl35133 yannl35133 Mar 24, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
let quals = Array.map2 (fun a b -> a,b) quals qvariances in
let univs = Array.map2 (fun a b -> a,b) univs uvariances in
let quals = Array.combine quals qvariances in
let univs = Array.combine univs uvariances in

@yannl35133 yannl35133 added the needs: documentation Documentation was not added or updated. label Mar 24, 2026
@coqbot-app coqbot-app Bot removed request: full CI Use this label when you want your next push to trigger a full CI. needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. labels Mar 24, 2026
@github-actions github-actions Bot added the needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. label Mar 27, 2026
JasonGross pushed a commit to JasonGross/coq-elpi that referenced this pull request Mar 29, 2026
@SkySkimmer @JasonGross
Adapt to rocq-prover/rocq#21773 (sort polymorphic cumulativity)
b1b60bf
@coqbot-app

coqbot-app Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

The "needs: rebase" label was set more than 30 days ago. If the PR is not rebased in 30 days, it will be automatically closed.

@coqbot-app coqbot-app Bot added the stale This PR will be closed unless it is rebased. label Apr 27, 2026
@SkySkimmer

SkySkimmer commented Apr 28, 2026

Copy link
Copy Markdown
Contributor Author

The inconsistency scared me enough that I'm not sure if we want to continue this, especially considering we intend to remove Prop <= Type eventually.

@coqbot-app

coqbot-app Bot commented May 27, 2026

Copy link
Copy Markdown
Contributor

This PR was not rebased after 30 days despite the warning, it is now closed.

@coqbot-app coqbot-app Bot closed this May 27, 2026
@coqbot-app coqbot-app Bot removed this from the 9.3+rc1 milestone May 27, 2026
This was referenced Jun 22, 2026
Closed
Open
@claude-fable-5-theorem claude-fable-5-theorem mentioned this pull request Jul 24, 2026
Open
@JasonGross JasonGross mentioned this pull request Aug 12, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yannl35133 yannl35133 yannl35133 left review comments
@ppedrot ppedrot ppedrot approved these changes

Assignees

@ppedrot ppedrot

Labels

kind: enhancement Enhancement to an existing user-facing feature, tactic, etc. needs: documentation Documentation was not added or updated. needs: rebase Should be rebased on the latest master to solve conflicts or have a newer CI run. stale This PR will be closed unless it is rebased.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@SkySkimmer @ppedrot @JasonGross @yannl35133