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

Fix #748 (coq.typecheck-indt-decl failing) #750

Merged
merged 1 commit into from
Jan 23, 2025
Merged

Conversation

proux01
Copy link
Contributor

@proux01 proux01 commented Jan 22, 2025

Fixes: #748

The count-prods-nored is not ideal, I would prefer using something like @redflags! coq.redflags.nored => coq.count-prods ... but this didn't worked when I tried.

proux01 added a commit to proux01/hierarchy-builder that referenced this pull request Jan 22, 2025
proux01 added a commit to proux01/hierarchy-builder that referenced this pull request Jan 22, 2025
proux01 added a commit to math-comp/hierarchy-builder that referenced this pull request Jan 22, 2025
@proux01 proux01 force-pushed the fix_748 branch 2 times, most recently from 3edfbe1 to 1e226ed Compare January 22, 2025 15:17
@proux01 proux01 marked this pull request as ready for review January 22, 2025 16:12
@proux01
Copy link
Contributor Author

proux01 commented Jan 22, 2025

CI green

@gares
Copy link
Contributor

gares commented Jan 22, 2025

There are a few things that are not ok. The one that puzzles me is that if you use coq apis you must load the context correctly, but the old count prods you modified does not. Although the errors should be fatal, so I really don't know how CI did not failed.

This seems to work here

pred coq.count-prods i:term, o:int.
coq.count-prods (prod N T B) C :- !,
  (@pi-decl N T x\ coq.count-prods (B x) C'), C is C' + 1.
coq.count-prods (let N T V B) C  :- !,
  (@pi-def N T V x\ coq.count-prods (B x) C).
coq.count-prods T C :- !,
  coq.reduction.lazy.whd T Tr,
  if (T == Tr) (C = 0) (coq.count-prods Tr C).

and I tried as follows

Definition foo := nat -> nat.
Elpi command c.
Elpi Query lp:{{
(@redflags! coq.redflags.nored => coq.count-prods {{ foo }} 0),
(                                 coq.count-prods {{ foo }} 1).
}}.

Using == in place of = should not really matter much.
I removed the label, since its main objective was to let one plug in some reduction.

@gares
Copy link
Contributor

gares commented Jan 22, 2025

I would remove the anchor point also in arity-> sort, and similarly to count prod move under let-ins without firing them (and use ==, again not foundamental).

proux01 added a commit to proux01/hierarchy-builder that referenced this pull request Jan 23, 2025
proux01 added a commit to math-comp/hierarchy-builder that referenced this pull request Jan 23, 2025
@proux01
Copy link
Contributor Author

proux01 commented Jan 23, 2025

@gares thanks for the review. It should now be taken into account and CI is green again.

This seems to work here

Indeed, it appears the issue I actually experienced wasn't with redflags but the missing @pi-decl you spotted.

@gares gares merged commit acee920 into LPCIC:master Jan 23, 2025
50 of 52 checks passed
@proux01
Copy link
Contributor Author

proux01 commented Jan 23, 2025

Thanks

@proux01 proux01 deleted the fix_748 branch January 23, 2025 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

coq.typecheck-indt-decl failing where it succeeds in plain Coq
2 participants