Do not store recursive trees anymore in inductive blocks.#21901
Do not store recursive trees anymore in inductive blocks.#21901ppedrot wants to merge 1 commit intorocq-prover:masterfrom
Conversation
This data is now useless as it can be recovered from the compiled automaton.
Backwards compatible, can be merged already.
Backwards compatible, can be merged already.
| infos.is_general | ||
| || Rtree.is_infinite Declareops.eq_recarg | ||
| graph_def.Declarations.mind_recargs | ||
| || Inductiveops.mis_is_recursive graph_def |
There was a problem hiding this comment.
Why is this equivalent, or if not a correct replacement?
There was a problem hiding this comment.
Who cares, this is funind.
There was a problem hiding this comment.
I saw you made the same change for elpi.
| Array.exists one_is_rec (dest_subterms @@ Rtree.Kind.make mip.mind_recargs) | ||
| let ra = mip.mind_automaton in | ||
| let trans = Rtree.Automaton.transitions ra (Rtree.Automaton.initial ra) in | ||
| let check tr = match Rtree.Automaton.data ra tr with Mrec _ -> true | Norec -> false in |
There was a problem hiding this comment.
I'm not against it, but you're adding nesting over arrays as recursive here, is this fine?
There was a problem hiding this comment.
This was a clear oversight, there is no reason to treat array nesting any different than normal inductive nesting here.
There was a problem hiding this comment.
Do we know for sure that a transition to label Mrec must have a transition back to init? Or is this not the expected meaning of mis_is_recursive.
There was a problem hiding this comment.
Hmm, upon reading the code and testing some edge cases, it's not always the case. Nesting over an inductive that does not use its parameter still generates an Mrec node. But then I guess that the question is what it means for an inductive to be truly recursive. Is the following truly recursive for instance?
Inductive box (A : Type) := Box : box A -> box A.
Inductive foo := Foo : box foo -> foo.
There was a problem hiding this comment.
(I think that a reasonable spec is that an inductive is truly recursive if its associated minimal automaton has a loop, but then we have to understand why the callers where using this in the first place.)
There was a problem hiding this comment.
Even if it disappears by non-uniform expansion of nesting types? The following qualifies as recursive with your criterion:
Inductive box (A : Type) :=.
Inductive foo := Foo : box foo -> foo.but it's finite, even empty, and you can prove that without recursion.
There was a problem hiding this comment.
Inductive loop := Lopp : loop -> loop is empty but also recursive IMO, so emptiness cannot force nonrecursive
There was a problem hiding this comment.
BTW declareInd (outside the kernel) has a syntactic check to forbid declaration of recursive (by my above definition) inductives with Variant, but even if that check is disabled Variant foo := Foo : box foo -> foo. fails:
Error:
Bad inductive definition.
Raised at Indtypes.check_positivity_one.check_constructors.check_constr_rec in file "kernel/indtypes.ml", line 365, characters 16-65
There was a problem hiding this comment.
if that check is disabled
Variant t := c (x := t) : x -> x. (or simply side-stepped)
There was a problem hiding this comment.
But in any case, being recursive and being barred from being a Variant should be the same imo.
(Speaking of which, you didn't answer: does the inductive type I gave above qualify as "truly recursive"?
I don't have an opinion of what should count as recursive. I looked at all use-cases and found the following:
- Rocq-elpi's
coq.env.recursive?which I don't understand - funind's business, which I don't want to understand
- firstorder's
formula.mlwhich apparently only expectsAnd,Orand recursive types, and that I don't understand - indrec and allScheme, for which being recursive means the eliminator needs to be a fixpoint
- hipattern's
match_with_tuple, used indescend_in_conjunctions, used to not loop into infinitely deep conjunctions.
yannl35133
left a comment
There was a problem hiding this comment.
I can accept on behalf of @rocq-prover/kernel-maintainers but real users should be the ones to comment on the changes this PR introduces.
This data is now useless as it can be recovered from the compiled automaton.