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

Refactor call desugaring and passive command creation #743

Open
atomb opened this issue Jun 2, 2023 · 5 comments
Open

Refactor call desugaring and passive command creation #743

atomb opened this issue Jun 2, 2023 · 5 comments
Assignees
Labels

Comments

@atomb
Copy link
Collaborator

atomb commented Jun 2, 2023

As @keyboardDrummer points out in his review comments on #741, ConditionGeneration.TurnIntoPassiveCmd and CallCmd.ComputeDesugaring are very complex and use regions to further break down the code. They might be more understandable and maintainable if they could be broken into multiple methods, each with clear names and purposes. However, given how central that code is to the correctness of Boogie, and how complex it is, we should probably do so carefully and with the involvement of several reviewers deeply familiar with the code.

In particular, I think @shazqadeer, @RustanLeino, @zvonimir/@rakamaric, and perhaps others should have a chance to chime in before we undertake this, and be involved in the review process.

@shazqadeer
Copy link
Contributor

Indeed, both methods are very long and deserve to be split up.

I noticed that TurnIntoPassiveCmd analyzes many attributes on PredicateCmd's such as "minimize", "maximize", "verified_under", etc. I examined the output of "boogie /attrHelp" and didn't find any documentation for these attributes. Does anybody know what they are supposed to do? Are they Dafny specific?

@atomb
Copy link
Collaborator Author

atomb commented Jun 5, 2023

I don't believe Dafny uses any of those attributes, and I'm not sure what they do, either.

@shazqadeer
Copy link
Contributor

Then we should delete them.

@RustanLeino , @gauravpartha , @alexanderjsummers: ok with deleting the attributes mentioned above?

@gauravpartha
Copy link
Collaborator

Then we should delete them.

@RustanLeino , @gauravpartha , @alexanderjsummers: ok with deleting the attributes mentioned above?

Yes, Viper doesn't use those attributes either.

@alexanderjsummers
Copy link

Sounds fine to me then! Thanks.

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

4 participants