Skip to content

Constr-as-datatype functions now use the user name for references.#21882

Merged
coqbot-app[bot] merged 1 commit into
rocq-prover:masterfrom
ppedrot:term-data-use-userord
May 18, 2026
Merged

Constr-as-datatype functions now use the user name for references.#21882
coqbot-app[bot] merged 1 commit into
rocq-prover:masterfrom
ppedrot:term-data-use-userord

Conversation

@ppedrot

@ppedrot ppedrot commented Apr 3, 2026

Copy link
Copy Markdown
Member

This is a follow-up of #21863. Since it is clear that these functions are only used by plugins to manipulate terms as blobs, we should not try to be too clever by half, so we just ignore the aliasing quotient.

@ppedrot ppedrot added this to the 9.3+rc1 milestone Apr 3, 2026
@ppedrot ppedrot requested a review from a team as a code owner April 3, 2026 06:34
@ppedrot ppedrot added kind: cleanup Code removal, deprecation, refactorings, etc. request: full CI Use this label when you want your next push to trigger a full CI. labels Apr 3, 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 Apr 3, 2026
@ppedrot ppedrot added the needs: fixing The proposed code change is broken. label Apr 10, 2026
@SkySkimmer

Copy link
Copy Markdown
Contributor

Plugins really want the quotienting. Possible solutions:

  • access Global.env
  • functorize the API module ConstrData (_:sig val env : env end) : sig ... end (works if used at constant delta resolver, nontrivial overlay work?)
  • change to type t = env * constr and use one of the envs when comparing (but which side's env? I guess picking the side in which both constants appear when we encounter a constant? (if no side has both constants assert false)) probably easier overlay work
  • don't change ConstrData but provide a canonize_constr : env -> contr -> constr function and have plugins use it before adding/looking up (costly?)

@SkySkimmer

Copy link
Copy Markdown
Contributor

change to type t = env * constr and use one of the envs when comparing (but which side's env? I guess picking the side in which both constants appear when we encounter a constant? (if no side has both constants assert false)) probably easier overlay work

actually we can canonize each side using the respective env then compare the canonized kernames
maybe this solution is what we should do?

@ppedrot

ppedrot commented May 3, 2026

Copy link
Copy Markdown
Member Author

Yes, my plan to solve this is to canonize the term before storing it in the environment but then I need to understand what part of the code really relies on that.

@ppedrot ppedrot added the request: full CI Use this label when you want your next push to trigger a full CI. label May 6, 2026
@ppedrot ppedrot force-pushed the term-data-use-userord branch from 07f1d07 to 4b67af2 Compare May 6, 2026 09:06
@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 May 6, 2026
@ppedrot

ppedrot commented May 6, 2026

Copy link
Copy Markdown
Member Author

The metacoq CI can be fixed via a one-liner, given the rest of the CI I'm not sure it's worth it to try to restore the previous behaviour for congruence / firstorder.

ppedrot added a commit to MetaRocq/metarocq that referenced this pull request May 7, 2026
ppedrot added a commit to MetaRocq/metarocq that referenced this pull request May 7, 2026
This is a follow-up of rocq-prover#21863. Since it is clear that these functions
are only used by plugins to manipulate terms as blobs, we should not try
to be too clever by half, so we just ignore the aliasing quotient.
ppedrot added a commit to rocq-community/aac-tactics that referenced this pull request May 11, 2026
ppedrot added a commit to rocq-community/aac-tactics that referenced this pull request May 11, 2026
@ppedrot ppedrot added the request: full CI Use this label when you want your next push to trigger a full CI. label May 11, 2026
@ppedrot ppedrot force-pushed the term-data-use-userord branch from 4b67af2 to 90f587b Compare May 11, 2026 16:30
@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 May 11, 2026
@ppedrot ppedrot removed the needs: fixing The proposed code change is broken. label May 11, 2026
@ppedrot

ppedrot commented May 11, 2026

Copy link
Copy Markdown
Member Author

I normalized the terms in aac-tactics before storing them in the hashtable and it seems to have been enough for all tests to pass. In my opinion this PR is ready.

@SkySkimmer

Copy link
Copy Markdown
Contributor

@coqbot merge now

@coqbot-app coqbot-app Bot merged commit ae510f4 into rocq-prover:master May 18, 2026
8 checks passed
@ppedrot ppedrot deleted the term-data-use-userord branch May 18, 2026 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind: cleanup Code removal, deprecation, refactorings, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants