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

"Omit proof" on a Let in a section leads to warning #687

Closed
RalfJung opened this issue Jan 31, 2023 · 14 comments
Closed

"Omit proof" on a Let in a section leads to warning #687

RalfJung opened this issue Jan 31, 2023 · 14 comments
Labels

Comments

@RalfJung
Copy link

Consider this proof script:

Section test.
Let Fact: 0 = 0.
Proof. reflexivity. Qed.
End test.

When skipping over the entire script in "omit proofs" mode, PG prints a warning:

Warning: Let definition Fact declared as an axiom. [let-as-axiom,vernacular]
@erikmd
Copy link
Member

erikmd commented Feb 2, 2023

Dear @RalfJung, thanks for your feedback.

I can reproduce this behavior, but I wouldn't call this a "bug", as omit-proofs precisely ignores the whole proof script reflexivity. Qed. proof and replaces it with Admitted., we get:

Section test.
Let Fact: 0 = 0.
Proof. Admitted.
(* Warning: Let definition Fact declared as an axiom. [let-as-axiom,vernacular]
Fact is declared *)

so I guess we can just mark this issue as wontfix / upstream.

@erikmd erikmd added resolved: wontfix issue: upstream Issue handled upstream labels Feb 2, 2023
@RalfJung
Copy link
Author

RalfJung commented Feb 2, 2023

I think it is a bug to get a warning here. Enabling the "omit proofs" feature should just make things go faster, it shouldn't cause any other side-effects like this.

Though I can imagine that fixing this bug might be hard due to Coq constraints.

@erikmd
Copy link
Member

erikmd commented Feb 2, 2023

This is just an upstream warning.
Actually, I can understand this warning that in the presence of an Admitted. proof and a local definition (Let),
there is some risk that the generated axioms forgets or doesn't list in good order the section's hypothesis.

@erikmd
Copy link
Member

erikmd commented Feb 2, 2023

But precisely, this can be handled by the Proof using … clause :)
I'll come with an example.

@RalfJung
Copy link
Author

RalfJung commented Feb 2, 2023

Let me just say that the user experience here is quite bad: I have a huge file that used to always work fine, and now suddenly I get this strange warning. I spend a bunch of time to figure out what this is about (there's no line number or anything that would even tell me where in my proof script the issue arises), only to realize that actually my proof script is completely fine and the warning is caused by "omit proofs".

It's an upstream warning about code I didn't write, and a warning that doesn't indicate anything wrong with my code. If that's not a bug I don't know what is.^^

@erikmd
Copy link
Member

erikmd commented Feb 2, 2023

Hi! Yes I understand your point.

Actually I'm not really sure of the "usefulness" caused by this upstream warning.

In my second comment, I had guessed it could be related to the risk that Coq generalizes section hypothesese too eagerly.
I'm not sure of the intent, but that looks plausible.

But in this case, lemmas fact2 and fact3 below involving Proof using … shouldn't raise this warning as the generalization at section-closing-time is properly handled in this case.

Section test.
Hypothesis hyp : True.
Hypothesis hyp2 : True /\ True.
Let fact1: 0 = 0.
Proof. Admitted.
(* > Proof. Admitted.
>        ^^^^^^^^^
Warning: Let definition fact1 declared as an axiom. [let-as-axiom,vernacular]
fact1 is declared
*)

Let fact2: 0 = 0.
Proof using hyp2. Admitted. (* only use hyp2 *)
(* > Proof using hyp2. Admitted.
>                   ^^^^^^^^^
Warning: Let definition fact2 declared as an axiom. [let-as-axiom,vernacular]
fact2 is declared
*)

Let fact3: 0 = 0.
Proof using Type. Admitted. (* placeholder to use no section hyp *)
(* > Proof using Type. Admitted.
>                   ^^^^^^^^^
Warning: Let definition fact3 declared as an axiom. [let-as-axiom,vernacular]
fact3 is declared
*)

End test.
Check fact1.
(* fact1 *)
(*      : True -> True /\ True -> 0 = 0 *)
Check fact2.
(* fact2 *)
(*      : True /\ True -> 0 = 0 *)
Check fact3.
(* fact3 *)
(*      : 0 = 0 *)

@erikmd
Copy link
Member

erikmd commented Feb 2, 2023

To address the usability issue, I think Coq should disable the warning, at least when a Proof using … clause is included, or ideally always, for simplicity. Feel free to open an issue upstream if you can!

In the meantime, you can just disable this warning in your _CoqProject;
and I'm going to add a comment in the PG documentation…

I'm sorry if the experience is not as smooth as expected currently, but actually doing C-u C-c C-RET is precisely intended as a syntactic facility for replacing on-the-fly whole proofs with Admitted, which is not an exotic workflow… (I was doing this everytime before omit-proofs… without a shortcut 😅)

@erikmd
Copy link
Member

erikmd commented Feb 2, 2023

In the meantime, you can just disable this warning in your _CoqProject;

for the record, the exact snippet is:

_CoqProject

-arg -w -arg -let-as-axiom

@RalfJung
Copy link
Author

RalfJung commented Feb 2, 2023

Let's see what the Coq devs say in coq/coq#17199. I don't quite understand what the warning is about either...

@hendriktews
Copy link
Collaborator

hendriktews commented Feb 2, 2023 via email

@RalfJung
Copy link
Author

RalfJung commented Feb 2, 2023

Disabling omitting proofs for Let makes sense to me.

@erikmd
Copy link
Member

erikmd commented Feb 3, 2023

Disabling omitting proofs for Let makes sense to me.

Yes, this seems to be the natural solution after the discussion in coq/coq#17199 (comment)

However, note that as pointed out by the coq devs, your use case of Let…Qed is somehow deprecated! at the very best, the cooked proof terms are not opaque.

So I see another solution for you, which does not require changing PG:
replace Let…Qed. with either Lemma…Qed. or Let…Defined.

And I realize all this discussion took place thanks to this warning… which was useful actually :)

@RalfJung
Copy link
Author

RalfJung commented Feb 3, 2023

Yeah we had no idea this was deprecated, it has worked fine for us.^^

@hendriktews
Copy link
Collaborator

See also #689.

hendriktews added a commit to hendriktews/PG that referenced this issue Mar 26, 2023
Add support for recognizing proof-script commands that prevent the
omission of proofs that follow them directly, such as a Let
declaration with a proof script in Coq.

Fixes ProofGeneral#687
hendriktews added a commit to hendriktews/PG that referenced this issue Apr 14, 2023
Add support for recognizing proof-script commands that prevent the
omission of proofs that follow them directly, such as a Let
declaration with a proof script in Coq.

Fixes ProofGeneral#687
hendriktews added a commit to hendriktews/PG that referenced this issue Jan 17, 2024
Add support for recognizing proof-script commands that prevent the
omission of proofs that follow them directly, such as a Let
declaration with a proof script in Coq.

Fixes ProofGeneral#687
hendriktews added a commit to hendriktews/PG that referenced this issue Jan 22, 2024
Add support for recognizing proof-script commands that prevent the
omission of proofs that follow them directly, such as a Let
declaration with a proof script in Coq.

Fixes ProofGeneral#687
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants