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

replace by tac should not mask tac's failure message #17959

Closed
JasonGross opened this issue Aug 15, 2023 · 2 comments · Fixed by #17964
Closed

replace by tac should not mask tac's failure message #17959

JasonGross opened this issue Aug 15, 2023 · 2 comments · Fixed by #17964
Labels
kind: user messages Improvement of error messages, new warnings, etc. part: tactics
Milestone

Comments

@JasonGross
Copy link
Member

Description of the problem

Goal True.
  assert_succeeds replace True with False.
  Fail replace True with False; [ | exact I ].
  (* The command has indeed failed with message:
Ltac call to "exact (uconstr)" failed.
The term "I" has type "True" while it is expected to have type "False = True".
*)
  replace True with False by exact I.
  (* Error:
Ltac call to "replace (uconstr) with (constr) (clause_dft_concl) (by_arg_tac)" failed.
No applicable tactic.
   *)

Coq Version

8.19+alpha (61ee398)

@JasonGross JasonGross added kind: user messages Improvement of error messages, new warnings, etc. part: tactics labels Aug 15, 2023
@SkySkimmer
Copy link
Contributor

It seems to be because replace x with y by tac runs first [assumption | symmetry;assumption | tac] on the subgoal and first eats the error. (first eating the error makes sense IMO)
cf

coq/tactics/equality.ml

Lines 653 to 657 in 61ee398

(tclFIRST
[assumption;
tclTHEN (apply sym) assumption;
try_prove_eq
])

I think when a tactic is explicitly given by by we shouldn't try assumption, (ie replace x with y tries assumption but replace x with y by tac doesn't).

Ot maybe we shouldn't try assumption at all but that would be more likely to break scripts in the wild.

We can also avoid using first and use tclORELSE0 instead to avoid eating the error without changing behaviour.

@SkySkimmer
Copy link
Contributor

BTW I don't get the "ltac call .. failed" message, I guess you have ltac backtrace in coqrc or some such thing

@SkySkimmer SkySkimmer changed the title replace (uconstr) with (constr) (clause_dft_concl) (by_arg_tac) should not mask by_arg_tac's failure message replace by tac should not mask tac's failure message Aug 16, 2023
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Aug 17, 2023
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Aug 17, 2023
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Aug 17, 2023
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Sep 6, 2023
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Sep 8, 2023
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Sep 8, 2023
SkySkimmer added a commit to SkySkimmer/coq that referenced this issue Sep 11, 2023
@coqbot-app coqbot-app bot added this to the 8.19+rc1 milestone Sep 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: user messages Improvement of error messages, new warnings, etc. part: tactics
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants