Skip to content

feat: Allow primitive projections with postponed eta (no sort poly)#21438

Merged
coqbot-app[bot] merged 4 commits intorocq-prover:masterfrom
TDiazT:primitive-postponed-eta
Jan 9, 2026
Merged

feat: Allow primitive projections with postponed eta (no sort poly)#21438
coqbot-app[bot] merged 4 commits intorocq-prover:masterfrom
TDiazT:primitive-postponed-eta

Conversation

@TDiazT
Copy link
Contributor

@TDiazT TDiazT commented Dec 19, 2025

This PR relaxes the constraints on defining records with primitive projections, namely for Type and Prop, by postponing the check to conversion. Namely, it introduces a new argument has_eta to decide whether to check or reject during conversion. This is a prelude to #21416.

  • Records defined in Type/Prop with fields only in SProp can be defined and have NoEta, thus fail conversion.
  • Records defined at some sort s with fields only in SProp can be defined and have MaybeEta, but currently fail during conversion.
  • Records defined at some sort s with fields in other sorts are allowed and have MaybeEta.

Associated RFC : rocq-prover/rfcs#111.

Examples

Test cases added in success/record_postponed_eta.v

Set Primitive Projections.

Record RTypeToSProp (A : SProp) : Type := { f1 : A }.

(* Conversion when record is in Type and field in SProp fails correctly *)
Goal forall (A:SProp) (r2 : RTypeToSProp A), eq r2 {| f1 := r2.(f1 A) |}.
Proof. intros A r2. Fail reflexivity. Abort.
(* Unable to unify "{| f1 := f1 _ r2 |}" with "r2" *)
    
Record RPropToSProp (A : SProp) : Prop := {  f2 : A }.

(* Conversion when record is in Prop and field in SProp fails correctly *)
Goal forall (A:SProp) (r2 : RPropToSProp A), eq r2 {| f2 := r2.(f2 A) |}.
Proof. intros A r2. Fail reflexivity. Abort.
(* Unable to unify "{| f2 := f2 _ r2 |}" with "r2". *)

Set Universe Polymorphism.

Record RSToSProp@{s;u|s -> SProp} (A : SProp) : Type@{s;u} := {  f3 : A }.

Inductive eq@{s s'; u} (A : Type@{s;u}) (a : A) : A -> Prop :=  eq_refl : eq A a a.

(* Conversion when record is in Type and field in SProp fails correctly *)
Goal forall (A:SProp) (r2 : RSToSProp@{Type;0} A), eq r2 {| f3 := r2.(f3 A) |}.
Proof. intros A r2. Fail reflexivity. Abort.
(* Unable to unify "{| f3 := f3 _ r2 |}" with "r2". *)

Main API Changes

  • declarations.mli : PrimRecord is now defined with a record, including a new argument of type has_eta.

Overlays


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

  • Added / updated documentation.

    • Documented any new / changed user messages.
    • Updated documented syntax by running make doc_gram_rsts.
  • Opened overlay pull requests.

@TDiazT TDiazT requested review from a team as code owners December 19, 2025 11:45
@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 Dec 19, 2025
@TDiazT TDiazT force-pushed the primitive-postponed-eta branch 4 times, most recently from 1c98298 to 190bfdd Compare December 19, 2025 12:58
@yannl35133
Copy link
Contributor

@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 Dec 19, 2025
TDiazT added a commit to TDiazT/metarocq that referenced this pull request Dec 21, 2025
TDiazT added a commit to TDiazT/metarocq that referenced this pull request Dec 21, 2025
TDiazT added a commit to TDiazT/metarocq that referenced this pull request Dec 21, 2025
@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 Dec 21, 2025
assert (Result.is_ok @@ Option.get not_prim_or_has_eta);
PrimRecord { id = rid.(i); projections; relevances; tys; has_eta = Result.get_ok @@ Option.get not_prim_or_has_eta}
| None | Some (Error _) -> assert false
| Some Ok has_eta ->
Copy link
Member

Choose a reason for hiding this comment

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

The year is 2026 and I discover a new OCaml syntax. Since when is that pattern allowed without parentheses?

@ppedrot
Copy link
Member

ppedrot commented Jan 6, 2026

Some messages need a few tweaks, for instance:

Set Primitive Projections.

Record T (A : SProp) : Type := {
  p : A
}.

About T.

gives

[...]
T has primitive projections with eta conversion.
[...]

@TDiazT TDiazT force-pushed the primitive-postponed-eta branch from 0557dac to 82e6768 Compare January 6, 2026 10:21

Record noprojs := { y := 0 }.

Record norelevantprojs (A:SProp) := { z : A }.
Copy link
Member

Choose a reason for hiding this comment

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

Rather than removing the example, I think you should keep it and add an About command to witness the correct message here.

@ppedrot ppedrot added this to the 9.2+rc1 milestone Jan 6, 2026
@TDiazT TDiazT requested a review from a team as a code owner January 8, 2026 10:48
@ppedrot
Copy link
Member

ppedrot commented Jan 8, 2026

@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 Jan 8, 2026
@ppedrot ppedrot removed the needs: changelog entry This should be documented in doc/changelog. label Jan 8, 2026
refactor: Cleanup prim projection assertion and misc

refactor: Remove MaybeEta
@TDiazT TDiazT force-pushed the primitive-postponed-eta branch from 7e00cfe to 6b296f2 Compare January 8, 2026 21:37
@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 Jan 8, 2026
TDiazT added a commit to TDiazT/metarocq that referenced this pull request Jan 8, 2026
@ppedrot
Copy link
Member

ppedrot commented Jan 9, 2026

@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 Jan 9, 2026
@ppedrot
Copy link
Member

ppedrot commented Jan 9, 2026

Why is the CI broken now?

@TDiazT
Copy link
Contributor Author

TDiazT commented Jan 9, 2026

Why is the CI broken now?

Ah, wrong url for my metarocq repo...

The lean one I don't know yet.

@SkySkimmer
Copy link
Contributor

Full CI from 2 weeks ago had the same lean importer error https://gitlab.inria.fr/coq/coq/-/jobs/6619036
The lean importer declares as primitive record any inductive in Type with 0 indices 1 constructor.
I guess the inductive mentioned in the error got turned into fakerecord and now is a primitive record without eta. Then the generated eliminator must now be non dependent which causes the error.
We could try adding a "exists relevant field" check at https://github.com/rocq-community/rocq-lean-import/blob/166733bac6841a04d92cce0226748560bf77433c/src/lean.ml#L1309 (kinda dual to the "all fields irrelevant" check in the _ :: _, true case below).

@@ -0,0 +1 @@
overlay metarocq https://github.com/TDiazT/metarocq primitive-postponed-eta 21438
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
overlay metarocq https://github.com/TDiazT/metarocq primitive-postponed-eta 21438
overlay metarocq https://github.com/TDiazT/metacoq primitive-postponed-eta 21438

or rename your repo (renaming your repo will prevent such errors in the future)

TDiazT added a commit to TDiazT/rocq-lean-import that referenced this pull request Jan 9, 2026
@TDiazT TDiazT force-pushed the primitive-postponed-eta branch from 6b296f2 to 9664efe Compare January 9, 2026 14:08
@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 Jan 9, 2026
@TDiazT
Copy link
Contributor Author

TDiazT commented Jan 9, 2026

Full CI from 2 weeks ago had the same lean importer error https://gitlab.inria.fr/coq/coq/-/jobs/6619036 The lean importer declares as primitive record any inductive in Type with 0 indices 1 constructor. I guess the inductive mentioned in the error got turned into fakerecord and now is a primitive record without eta. Then the generated eliminator must now be non dependent which causes the error. We could try adding a "exists relevant field" check at https://github.com/rocq-community/rocq-lean-import/blob/166733bac6841a04d92cce0226748560bf77433c/src/lean.ml#L1309 (kinda dual to the "all fields irrelevant" check in the _ :: _, true case below).

Yep, that seems to work. I am not really understanding what it is that this piece of code is doing though (in the importer).
If the inductive is in Type and has at least one relevant field, then we are turning it into a record with primitive projections (returning Some, etc?), if not then we are turning it into a fake record (and thus recovering the previous behavior)? Is that it?

@ppedrot
Copy link
Member

ppedrot commented Jan 9, 2026

@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 Jan 9, 2026
SkySkimmer added a commit to rocq-community/rocq-lean-import that referenced this pull request Jan 9, 2026
@SkySkimmer
Copy link
Contributor

If the inductive is in Type and has at least one relevant field, then we are turning it into a record with primitive projections (returning Some, etc?), if not then we are turning it into a fake record (and thus recovering the previous behavior)? Is that it?

If not then it's a regular inductive, not fakerecord.
This code is trying to emulate lean 4's behaviour where all inductives with 1 constructor and no indices have definitional eta.
In rocq definitional eta needs the inductive to be declared as a primitive record and not be recursive etc, and primitive records without definitional eta don't have dependent elimination, so we try to declare as many primitive records with eta as possible and to avoid declaring primitive records without eta.

@ppedrot
Copy link
Member

ppedrot commented Jan 9, 2026

@coqbot merge now

@coqbot-app coqbot-app bot merged commit 54ff2cd into rocq-prover:master Jan 9, 2026
8 of 9 checks passed
@coqbot-app
Copy link
Contributor

coqbot-app bot commented Jan 9, 2026

@ppedrot: Please take care of the following overlays:

  • 21438-TDiazT-primitive-postponed-eta.sh

ppedrot added a commit to MetaRocq/metarocq that referenced this pull request Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: enhancement Enhancement to an existing user-facing feature, tactic, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants