Skip to content

Conversation

@ppedrot
Copy link
Collaborator

@ppedrot ppedrot commented Aug 30, 2024

In some cases, unquoting could generate terms that were ill-typed, typically when unquoting [I @hole] for some template polymorphic inductive type [I]. Similarly universe levels were not computed at all, which could have led to bizarre failures.

To ensure that the unquoted term makes sense at all, we simply perform a call to Typing before returning it. This has a non-trivial cost but since building the term is already linear, this should not hopefully matter that much in practice.

cc @tabareau

In some cases, unquoting could generate terms that were ill-typed, typically
when unquoting [I @hole] for some template polymorphic inductive type [I].
Similarly universe levels were not computed at all, which could have led
to bizarre failures.

To ensure that the unquoted term makes sense at all, we simply perform a
call to Typing before returning it. This has a non-trivial cost but since
building the term is already linear, this should not hopefully matter that
much in practice.
@ppedrot ppedrot force-pushed the typecheck-unquote branch from d809598 to 7bfa901 Compare August 30, 2024 15:35
@ppedrot
Copy link
Collaborator Author

ppedrot commented Aug 30, 2024

CI green and this works on my branch, so let's merge.

@ppedrot ppedrot merged commit 3067209 into MetaRocq:main Aug 30, 2024
@ppedrot ppedrot deleted the typecheck-unquote branch August 30, 2024 17:05
@JasonGross
Copy link
Contributor

Does this fix #842 ? What about #853 ? #862 ?

@JasonGross
Copy link
Contributor

@ppedrot
Copy link
Collaborator Author

ppedrot commented Aug 30, 2024

This only fixes the Ltac binding for denote_term and does not do anything for the vernac commands or the monad evaluator. Typing unconditionally in the constr denoter uncovers bugs in some parts where the environment passed is not the correct one. That is, it produces unbound rels errors. This can be fixed but it requires more work and I wanted the least disruptive change to proceed with rocq-prover/rocq#19228.

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.

2 participants