-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add Validatable trait #329
Conversation
… into validatable
CachingCMM.dfy needs a bit more work
… into validatable
Horray for generic traits and traits on traits!
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
// TODO-RS: Better name? | ||
twostate predicate ValidAndFresh() | ||
reads this, Repr | ||
ensures ValidAndFresh() ==> Valid() && fresh(Repr - old(Repr)) |
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.
Is there a benefit to writing ValidAndFresh()
as opposed to Valid() && Fresh()
? The former doesn't seem much more convenient, whereas the latter is easier to name (except perhaps for confusion between fresh()
and Fresh()
).
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.
It's questionable whether this is worth the trouble. I definitely DON'T think just "Fresh" is the right word anyway, so I'd rather come up with better or just drop this predicate. I'm more just observing this is a very common pattern.
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 is great, since it gives us a central place to talk about the standard dynamic-frames idiom. Two general questions I have are:
-
All members of trait
Validatable
are ghost. Yet, I imagine that there will be some code generated (in C# and Java) that says a class implements this empty interface. Does that have any negative consequences? (If so, we could think of some compiler directive that would say to ignore traits with only ghost members.) -
I'm thinking there may be a better name than
Validable
. Given theValidComponent
member, should the trait perhaps have a name likeComposite
?
// TODO-RS: The style guide actually implies this should be "repr", | ||
// but the convention is well-established at this point. |
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.
I think we should update the style guide. I suggest it be updated to say that "public" fields (or, at least, public ghost fields) be capitalized, where "public" means "intended to be used and understood by clients".
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.
Should that also align with the set of symbols you export then?
|
||
// Convenience predicate for when your object's validity depends on one | ||
// or more other objects. | ||
predicate ValidComponent(component: Validatable) |
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.
Having convenience predicates like this is fantastic!
ensures ValidComponent(component) ==> ( | ||
&& component in Repr | ||
&& component.Repr <= Repr | ||
&& this !in component.Repr | ||
&& component.Valid()) |
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 postcondition is not necessary, since it just restates the body and functions are transparent (non-opaque) in Dafny (unless you limit visibility with an export
clause, but you're not).
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.
I was convinced this was necessary for validation for some reason, perhaps it was confounded with whatever the real fix was. Will try removing.
// TODO-RS: Better name? | ||
twostate predicate ValidAndFresh() | ||
reads this, Repr | ||
ensures ValidAndFresh() ==> Valid() && fresh(Repr - old(Repr)) |
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.
Ditto. This postcondition is not necessary.
None that I can see. Empty "marker" interfaces are definitely already a thing (e.g. Java's
Agreed, although I think It's also worth noting that as defined, these kind of have to be "mutably validatable" because it forces If this is codifying the dynamic-frames idiom, should we call it |
On second thought that conflicts pretty badly with message body frames. :) |
Note that having this in a public Dafny standard library might help guide new users to use the idioms: dafny-lang/dafny#292 |
I've cut a standard library PR for this instead now, so it makes more sense to use that version if it gets in: dafny-lang/libraries#22 |
Closes #319
See issue for motivation.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.