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

"Validatable" trait for classes using the Valid()/Repr idiom #22

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

robin-aws
Copy link
Member

Encapsulates the idiom described in Chapters 8 and 9 of http://leino.science/papers/krml221.pdf as a shared common trait.

// A trait for objects with a Valid() predicate. Necessary in order to
// generalize some proofs, but also useful for reducing the boilerplate
// that most such objects need to include.
trait {:termination false} Validatable {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does that mean you're turning off the termination checker for this trait? Termination is an important correctness property, and I wouldn't feel comfortable using a standard library that adds such axioms without clear documentation on why it does so, why it's really needed, and why it's safe.
I haven't thought it through, but did you try to re-enable the termination checker and to prove termination using a decreasing Repr set?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah I wrote this so long ago I got used to seeing that. :) It's not turning off termination checking for this trait nor the classes inheriting it. It's only necessary in order to allow any class to extend this trait at all. This appears to be because allowing a class to extend a trait from a different module introduces unsoundness in the termination checking somehow - I'm only going by the error message here as this attribute doesn't appear to be documented.

This is still a good reason to block on doing better though. I know Rustan's talked about this unsoundness not being hard to fix so we'll at least document it better if not fix it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Rustan has kindly provided LOTS of detail on why we have {:termination false}: dafny-lang/dafny#1588 :)

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.

2 participants