Don't emit primitive records for recursive single-constructor inductives - #62
Conversation
|
Correct fix of this might be the fix for #63 |
|
Do Lean recursive records have primitive eta? If not then they're different issues (#63 is a case where Lean has primitive eta but Rocq doesn't, this one may just be an incorrect translation) |
|
I see. It seems Lean recursive records don't have eta: https://github.com/leanprover/lean4/blob/master/src/kernel/type_checker.cpp#L800 |
|
Here is a .lean file and the corresponding .out file: inductive list.{u} (α : Sort u)
| nil : list α
| cons : α → list α → list α
inductive RecInd.{u} (A : Sort u)
| mk : A → list (RecInd A) → RecInd A
-- Definitional eta does NOT hold for RecInd
-- Propositional eta
theorem RecInd.eta (x : RecInd A) : RecInd.mk x.1 x.2 = x := by
fail_if_success rfl
cases x; rfl.out file |
|
@SkySkimmer Do you have thoughts on this failure? |
|
Seems like it generated a different eliminator than Lean. |
|
For testing this PR you can do an inductive that doesn't use nesting, eg inductive RecInd.{u} (A : Sort u)
| mk : A → RecInd A → RecInd A(this is an empty type but that shouldn't matter for the test) |
a33e6b3 to
21a61c0
Compare
|
@SkySkimmer this now passes CI, what do you think? |
| else | ||
| CErrors.user_err | ||
| Pp.(str "cannot project non record " ++ N.pp lean_ind) | ||
| let npar = mib.mind_nparams in |
There was a problem hiding this comment.
move this stuff to a separate function, it's getting too big to stay inline
There was a problem hiding this comment.
Are you thinking that we move the entire handling of the Proj case to a separate function, or just the case unfolding?
There was a problem hiding this comment.
just the unfolding looks reasonable I think
| let ls = mk_list list char_uinst char chars in | ||
| Constr.(mkApp (mkConstructU ((string, 1), UVars.Instance.empty), [| ls |])) | ||
|
|
||
| let unfold_proj_case env evd ~field ~ind ~indu ~mib ~mip ~args c = |
There was a problem hiding this comment.
ind is redundant with indu (ind = fst indu)
| let unfold_proj_case env evd ~field ~ind ~indu ~mib ~mip ~args c = | |
| let unfold_proj_case env evd ~field ~indu ~mib ~mip ~args c = |
and maybe add a comment like "c has type indu applied to args"
| let (ctx, cty0) = mip.Declarations.mind_nf_lc.(0) in | ||
| let cty_full = | ||
| Term.it_mkProd_or_LetIn cty0 ctx | ||
| in |
There was a problem hiding this comment.
this function could probably use a reformat now that it is less indented, for instance this letin could be 1 line AFAICT (but it's not the only possible reformat)
not sure how far we have drifted from what ocamlformat would produce, maybe running it but only keeping what it does to this new function would be best
A recursive single-constructor inductive cannot admit eta, so the kernel rejects the primitive-record encoding. The importer's [declare_ind] was coercing every single-constructor, no-index, >=1-relevant-field inductive into a primitive record unconditionally, which made nested-recursive shapes fail at declaration time. Walk the rebuilt constructor Prod chain and check whether the inductive's de Bruijn index (npars+1 in env_ind_params, lifted per binder) occurs in any field type. If so, fall through to the plain Inductive path. The codomain is intentionally skipped: it always mentions the inductive by construction, so testing it there would demote every single-constructor record and break downstream projection uses. Adds tests/rec_single_ctor.v as regression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Include RecInd.eta theorem in the Lean source and export dump to test that the imported type supports dependent pattern matching, as requested in review. Lean does not grant definitional eta to recursive records. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the auxiliary list inductive from the regression fixture and make RecInd directly recursive through its second constructor field. Regenerate the Lean export with lean4export using explicit names for RecInd and RecInd.eta, and keep the Rocq test focused on the same constructor pattern without list-specific branches.
When an inductive is not declared as a primitive record (e.g., because it is recursive), Lean-side projections (#EJ) can no longer use Rocq primitive projections. Instead, build an inline case/match expression that extracts the appropriate constructor field. This fixes the CI failure where RecInd.eta (which uses projections on the now-non-record RecInd type) could not be imported. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ind = fst indu, so derive it locally instead of passing both. Also add a doc comment clarifying that c has type indu applied to args. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pure whitespace/formatting changes, no semantic diff. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
0d3ffaa to
fd79d7e
Compare
|
@SkySkimmer Updated |
Summary
declare_indcoerced every Lean inductive that was single-constructor + no-indices + ≥1 relevant field into a Rocq primitive record, without checking recursion.is_recursiveguard that falls through to the plainInductivepath whenever the inductive occurs in any field type. Non-recursive records keep the primitive-record encoding.Change
src/lean.ml, insidedeclare_ind's record-coercion branch, walk the rebuilt constructor'sProdchain and askVars.noccurn k tper field, wherekstarts atnpars + 1(the inductive's Rel index inenv_ind_params) and is lifted by one per enclosing binder. The walk stops before the codomain — a constructor's codomain isInd params…by construction, so testing it would flag every single-constructor inductive as recursive and demote every record.