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

Spurious universe inconsistencies in Coq 8.19 with abstracted proofs. #18636

Closed
silene opened this issue Feb 7, 2024 · 4 comments · Fixed by #18640 or #18642
Closed

Spurious universe inconsistencies in Coq 8.19 with abstracted proofs. #18636

silene opened this issue Feb 7, 2024 · 4 comments · Fixed by #18640 or #18642
Labels
kind: bug An error, flaw, fault or unintended behaviour. kind: regression Problems that were not present in previous versions. part: universes The universe system.
Milestone

Comments

@silene
Copy link
Contributor

silene commented Feb 7, 2024

Some of my large proofs break in Coq 8.19. Inserting the following command at the failure point

exfalso ; clear ; assert True by abstract (exact I).

gives

Universe inconsistency. Cannot enforce Set = Foo.154 because Set < Foo.154.

Forcefully ending the proof with apply FALSE. Qed. succeeds without triggering an universe inconsistency. So, the inconsistency does not seem to be present in the current proof term, but instead to be in the proof term created by abstract (or in its application to the current goal).

Unfortunately, I do not have a small reproducer. (This can be triggered by compiling the master branch of CoqInterval.)

@silene silene added part: universes The universe system. kind: bug An error, flaw, fault or unintended behaviour. labels Feb 7, 2024
@silene
Copy link
Contributor Author

silene commented Feb 8, 2024

Here is a reduced testcase.

Polymorphic Axiom array@{u} : Type@{u} -> Type@{u}.
Polymorphic Axiom get : forall {A} (t : array A), A.

Lemma foo (t: array nat): True.
Proof.
assert True by abstract exact I.
assert (v := get t).
abstract exact I. (* Universe inconsistency. Cannot enforce Top.5 = Set because Set < Top.5. *)
Qed.

@silene silene added the kind: regression Problems that were not present in previous versions. label Feb 8, 2024
@ppedrot
Copy link
Member

ppedrot commented Feb 8, 2024

Probably a side-effect of #17745, cc @SkySkimmer

@SkySkimmer
Copy link
Contributor

The reason the first abstract changes the behaviour is that it changes which branch we take in

|| not (Safe_typing.is_empty_private_constants eff.Evd.seff_private))

Why we have this code is not clear but removing it (so it becomes just if not poly && keep_body_ucst_separate) causes undefined universe anomaly at

In any case with this code and fea5fcc (indeed from #17745) we use an unminimized initial_euctx which has {foo.5} |= Set = foo.5 which is added to the global env (we are in univ monomorphic mode), and since monomorphic universes are declared > Set we get the error.

I guess short term we should revert fea5fcc and long term we should look into not checking is_empty_private_constants.

SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Feb 8, 2024
Fixed by reverting fea5fcc

The reason the first abstract changes the behaviour is that it changes which branch we take in https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/vernac/declare.ml#L1826

Why we have this code is not clear but removing it (so it becomes just `if not poly && keep_body_ucst_separate`) causes undefined universe anomaly at https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/test-suite/success/rewrite.v#L50

In any case with this code and fea5fcc (indeed from coq#17745) we use an unminimized initial_euctx which has `{foo.5} |= Set = foo.5` which is added to the global env (we are in univ monomorphic mode), and since monomorphic universes are declared `> Set` we get the error.

I guess short term we should revert fea5fcc and long term we should look into not checking is_empty_private_constants.
@SkySkimmer
Copy link
Contributor

revert at #18640

SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Feb 8, 2024
Morally we consider them part of the body univs.

Fix coq#18636 (alternative to coq#18640, but this probably isn't as good a
candidate for backporting)
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Feb 8, 2024
Fixed by reverting fea5fcc

The reason the first abstract changes the behaviour is that it changes which branch we take in https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/vernac/declare.ml#L1826

Why we have this code is not clear but removing it (so it becomes just `if not poly && keep_body_ucst_separate`) causes undefined universe anomaly at https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/test-suite/success/rewrite.v#L50

In any case with this code and fea5fcc (indeed from coq#17745) we use an unminimized initial_euctx which has `{foo.5} |= Set = foo.5` which is added to the global env (we are in univ monomorphic mode), and since monomorphic universes are declared `> Set` we get the error.

I guess short term we should revert fea5fcc and long term we should look into not checking is_empty_private_constants.
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Feb 8, 2024
Morally we consider them part of the body univs.

This makes it so we don't need to disregard keep_body_ucst_separate
when there are side effects.

Fix coq#18636 (alternative to coq#18640, but this probably isn't as good a
candidate for backporting)
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Feb 9, 2024
Fixed by reverting fea5fcc

The reason the first abstract changes the behaviour is that it changes which branch we take in https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/vernac/declare.ml#L1826

Why we have this code is not clear but removing it (so it becomes just `if not poly && keep_body_ucst_separate`) causes undefined universe anomaly at https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/test-suite/success/rewrite.v#L50

In any case with this code and fea5fcc (indeed from coq#17745) we use an unminimized initial_euctx which has `{foo.5} |= Set = foo.5` which is added to the global env (we are in univ monomorphic mode), and since monomorphic universes are declared `> Set` we get the error.

I guess short term we should revert fea5fcc and long term we should look into not checking is_empty_private_constants.
@coqbot-app coqbot-app bot added this to the 8.19.1 milestone Feb 19, 2024
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Feb 19, 2024
Fixed by reverting fea5fcc

The reason the first abstract changes the behaviour is that it changes which branch we take in https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/vernac/declare.ml#L1826

Why we have this code is not clear but removing it (so it becomes just `if not poly && keep_body_ucst_separate`) causes undefined universe anomaly at https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/test-suite/success/rewrite.v#L50

In any case with this code and fea5fcc (indeed from coq#17745) we use an unminimized initial_euctx which has `{foo.5} |= Set = foo.5` which is added to the global env (we are in univ monomorphic mode), and since monomorphic universes are declared `> Set` we get the error.

I guess short term we should revert fea5fcc and long term we should look into not checking is_empty_private_constants.

(cherry picked from commit 14685d5)
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Feb 20, 2024
Morally we consider them part of the body univs.

This makes it so we don't need to disregard keep_body_ucst_separate
when there are side effects.

Fix coq#18636 (alternative to coq#18640, but this probably isn't as good a
candidate for backporting)
louiseddp pushed a commit to louiseddp/coq that referenced this issue Feb 27, 2024
Fixed by reverting fea5fcc

The reason the first abstract changes the behaviour is that it changes which branch we take in https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/vernac/declare.ml#L1826

Why we have this code is not clear but removing it (so it becomes just `if not poly && keep_body_ucst_separate`) causes undefined universe anomaly at https://github.com/coq/coq/blob/3d86c1d4569b0a25a00677833b9f47c3059a8345/test-suite/success/rewrite.v#L50

In any case with this code and fea5fcc (indeed from coq#17745) we use an unminimized initial_euctx which has `{foo.5} |= Set = foo.5` which is added to the global env (we are in univ monomorphic mode), and since monomorphic universes are declared `> Set` we get the error.

I guess short term we should revert fea5fcc and long term we should look into not checking is_empty_private_constants.
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Mar 12, 2024
Morally we consider them part of the body univs.

This makes it so we don't need to disregard keep_body_ucst_separate
when there are side effects.

Fix coq#18636 (alternative to coq#18640, but this probably isn't as good a
candidate for backporting)
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Mar 13, 2024
Morally we consider them part of the body univs.

This makes it so we don't need to disregard keep_body_ucst_separate
when there are side effects.

Fix coq#18636 (alternative to coq#18640, but this probably isn't as good a
candidate for backporting)
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Mar 14, 2024
Morally we consider them part of the body univs.

This makes it so we don't need to disregard keep_body_ucst_separate
when there are side effects.

Fix coq#18636 (alternative to coq#18640, but this probably isn't as good a
candidate for backporting)
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Mar 14, 2024
Morally we consider them part of the body univs.

This makes it so we don't need to disregard keep_body_ucst_separate
when there are side effects.

Fix coq#18636 (alternative to coq#18640, but this probably isn't as good a
candidate for backporting)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug An error, flaw, fault or unintended behaviour. kind: regression Problems that were not present in previous versions. part: universes The universe system.
Projects
None yet
3 participants