Skip to content

improved warnings for uptobad #763

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

improved warnings for uptobad #763

wants to merge 1 commit into from

Conversation

mbbarbosa
Copy link
Contributor

This is a rough attempt at improving the warnings of the up to bad tactic.

Need advice what what the correct warning mechanism should be.

The current form helped me sort out a difficulty I had with a proof, but probably needs work.

@mbbarbosa mbbarbosa requested a review from bgregoir April 2, 2025 21:49
EqTest.for_instr env ~alpha i1 i2
if EqTest.for_instr env ~alpha i1 i2
then true
else (warn_uptobad env "instructions do not match"; false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should print the two instructions here, this can be done using EcPrinting.pp_instr.

EqTest.for_expr env a1 a2 &&
(if EqTest.for_expr env a1 a2
then true
else (warn_uptobad env "if statement conditions do not match"; false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same remark, with EcPrinting.pp_expr

@@ -185,7 +199,7 @@ let t_uptobad_r tc =
tc_error !!tc ~who:"byupto" "the event should have the form \"E /\ !bad\" or \"!bad\""
in
if not (f_upto_init env bad pr1.pr_fun pr2.pr_fun) then
tc_error !!tc ~who:"byupto" "the two function are not equal upto bad";
tc_error !!tc ~who:"byupto" "up to bad tactic failed";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried this, patch and I see not error message, excepted up to bad tactic failed.
Did you see something else?
I not use to the function EcEnv.notify. Can someone provide hint ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that proof general does not display the warnings when there is an error. Anyway, the notify mechanism should not be used for extending the error message. Assume that you do a try upto, then you will end with a lot of warning messages that you don't want to see.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should brainstorm a bit about how to manage the communication with the user when launching tactics that do complex analysis. This includes upto, but also circuit-based analysis, sim, etc.
Clearly we need a bit more than yes/no as an answer because these tactics typically require some trial and error before the analysis goes through.
This could be a different mechanism than applying the tactic? Something in the same plane as search for example?

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.

4 participants