-
Notifications
You must be signed in to change notification settings - Fork 75
feat: Add support for multiple invariants in CLI commands #1662
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
feat: Add support for multiple invariants in CLI commands #1662
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, some preliminary comments. I'm getting some compilation issues, also reported by the CI. Was this working for you?
quint/src/cliCommands.ts
Outdated
const guessedMainModule = guessMainModule(prev) | ||
const mainName = prev.args.main || guessedMainModule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change is not necessary, as guessMainModule
already returns prev.args.main
if provided.
const guessedMainModule = guessMainModule(prev) | |
const mainName = prev.args.main || guessedMainModule | |
const mainName = guessMainModule(prev) |
quint/src/cliCommands.ts
Outdated
const guessedMainModule = guessMainModule(prev) | ||
const mainName = prev.args.main || guessedMainModule |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const guessedMainModule = guessMainModule(prev) | |
const mainName = prev.args.main || guessedMainModule | |
const mainName = guessMainModule(prev) |
a7d8ca7
to
8e0c3de
Compare
@bugarela I have made the changes. Could you kindly check and let me know if everything is alright? |
Hi, sorry for the delay. Seems like there are still compilation issues. Are you able to successfully run |
Hi @bugarela. I did run npm run compile locally after making the changes. Let me check again and I will let you know as soon as I can. Thank You. |
…om/jesicaMao/quint into feat/multiple-invariants-reporting
@bugarela please check if everything is alright now. |
Hi @jesicaMao! Awesome, it seems to work now 🎉 thanks for seeing this through. I can take this on from here and get it merged today/tomorrow if that's ok for you. Again, thank you!! |
Ok, I'm testing things locally and turns out this is never loading the final state of the trace to evaluate the invariant there. It is evaluating the invariant on a completely new state (new evaluator), so it doesn't really work for invariants that reference the state (which are all real invariants in practice):
(this was supposed to print "n < 10") I have a pretty good idea how to fix this, but it is not something very trivial. I'll find sometime between this and next week to try it. I think the changes you made to the UI are super helpful already and a great head start, but this evaluation part is tricky. |
@bugarela Thanks a lot for your valuable feedback. I am overwhelmed to know my changes were useful. Please let me if I need to change anything else. |
bee7430
to
10af83f
Compare
CHANGELOG.md
for any new functionalityREADME.md
updated for any listed functionalityThis PR implements the feature requested in issue #1573 to support checking multiple invariants and reporting which specific ones were violated.
Changes include:
Added support for the --invariants CLI option to accept a list of invariants
Combined multiple invariants with logical AND for evaluation
Implemented detailed reporting of which specific invariants were violated
Updated documentation with examples showing how to use the feature
Added support for both simulator and verification modes
Fixes #1573