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

REGRESSION: ProofGeneral cannot step over Fail correctly #779

Closed
hendriktews opened this issue Jul 7, 2024 · 10 comments
Closed

REGRESSION: ProofGeneral cannot step over Fail correctly #779

hendriktews opened this issue Jul 7, 2024 · 10 comments
Assignees

Comments

@hendriktews
Copy link
Collaborator

This problem was originally reported as #597 and fixed in 2021.
Now with Coq 8.20+rc1, the message for Fail is

The command has indeed failed with message:
Tactic failure: Cannot solve this goal.
In nested Ltac calls to "now (tactic)" and "easy", last call failed.

And PG does not step over FAIL.
To reproduce, see ci/test_stepwise.v, assert to the end of the line containing FailTrace, and assert the next line.
(Before 8.20 the message printed by coq seemed to be

The command has indeed failed with message:
In nested Ltac calls to "now (tactic)" and "easy", last call failed.
Tactic failure: Cannot solve this goal.

)

@hendriktews hendriktews assigned hendriktews and Matafou and unassigned hendriktews Jul 7, 2024
@hendriktews
Copy link
Collaborator Author

Hi Pierre, you fixed #597 last time. Can you have a look? Maybe only Tactic failure: Cannot solve this goal needs to be added to coq--error-header-re-list. If you have not yet installed 8.20rc, I can test a patch.

@hendriktews
Copy link
Collaborator Author

Adding Tactic failure: to coq--error-header-re-list does not solve the problem. Reason: coq--error-header-re-list needs to contain In nested Ltac call for older Coq versions, therefore, there is always a mach for In nested Ltac call without message:\n in front of it. Even worse, when coq--error-header-re-list contains Tactic failure, then there is also a match without message:\n in front of it for older coq versions. If there is a match, proof-shell-handle-immediate-output believes coq reported an error.
To fix it, we would need an entry in coq--error-header-re-list that matches both

Tactic failure: Cannot solve this goal.
In nested Ltac calls to "now (tactic)" and "easy", last call failed.

and

Tactic failure: Cannot solve this goal.
In nested Ltac calls to "now (tactic)" and "easy", last call failed.

from the beginning.
Instead of adding more regex magic, I would suggest to add another generic variable, eg, proof-shell-no-error-regexp and modify proof-shell-handle-immediate-output to do

(and (proof-re-search-forward-safe proof-shell-error-regexp end t)
         (not (proof-re-search-forward-safe proof-shell-no-error-regexp end t)))

instead of only searching for proof-shell-error-regexp.

@SkySkimmer
Copy link
Contributor

Why is Tactic failure special? ie

Set Ltac Backtrace.
Goal False.
  Fail exact 0. (* fine *)
  Fail now auto. (* bad *)

@Matafou
Copy link
Contributor

Matafou commented Jul 8, 2024

The fix proposition by @hendriktews looks good to me.

@Matafou
Copy link
Contributor

Matafou commented Jul 8, 2024

@SkySkimmer because a previous ugly workaround.
@hendriktews do you propose a PR or should I?

@Matafou
Copy link
Contributor

Matafou commented Jul 8, 2024

I just pushed a PR.

@Matafou
Copy link
Contributor

Matafou commented Jul 8, 2024

@hendriktews did you initially detect the problem with the CI?

@hendriktews
Copy link
Collaborator Author

@hendriktews did you initially detect the problem with the CI?

yes, see the errors in #778

@Matafou
Copy link
Contributor

Matafou commented Jul 8, 2024

Very nice to have this detected so early.

@hendriktews
Copy link
Collaborator Author

yes, the honor is yours, for adding the test in 2021!

@Matafou Matafou closed this as completed in 7c8418f Jul 8, 2024
Matafou added a commit that referenced this issue Jul 8, 2024
…ail-correctly

Fixes #779 regression cannot step Fail correctly.
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

No branches or pull requests

3 participants