Skip to content

Conversation

@ssomayyajula
Copy link
Contributor

@ssomayyajula ssomayyajula commented Jul 20, 2025

What was changed?

This PR introduces class and trait invariants as described in the updated reference manual (in this PR).

How has this been tested?

Tests in Source/IntegrationTests/TestFiles/LitTests/LitTest/invariants.

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

ssomayyajula and others added 30 commits May 14, 2025 13:40
TODO: source under SyntaxDeserializer needs to be properly regenerated
TODO: tests for parsing and resolving need to be completed
…dingly

- Wellformedness checks simplified, since they do not have to be scoped to an enclosing declaration
- VerifyInvariants renamed to CheckInvariants and is automatically checked when a MemberDecl is coerced to an Invariant
- Refinement is now implemented, solving issue in InheritedMemberError.dfy
- Printing now implemented
…om invariants to be done as a visitor-based check
@ssomayyajula
Copy link
Contributor Author

Do we want to mention this as a new feature in the release notes for 4.11.0?

@robin-aws
Copy link
Member

Do we want to mention this as a new feature in the release notes for 4.11.0?

Absolutely at least add a docs/dev/news/*.feat note! Whether this makes it into 4.11 is a separate question (although I'm inclined towards yes - I think since we delayed it enough I'd rather start the code freeze over from master)

Copy link
Member

@MikaelMayer MikaelMayer left a comment

Choose a reason for hiding this comment

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

I will comment only the error messages, but otherwise it looks great !

Copy link
Member

@robin-aws robin-aws left a comment

Choose a reason for hiding this comment

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

Just some initial comments on the reference manual for now

Copy link
Member

@keyboardDrummer keyboardDrummer left a comment

Choose a reason for hiding this comment

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

I think it would be good to have a performance related test.

Something like:

class HasInvariant {
  var valid: boolean
  invariant valid
}
class HasNoInvariant {
  var valid: boolean
}

method processHasInvariant(h: HasInvariant) {
  useInvariant(h);
}

method useInvariant(h: HasInvariant) {
  assert h.valid;
}

method processHasNoInvariant(h: HasNoInvariant) {
  useHasNoInvariant(h);
}

method useHasNoInvariant(h: HasNoInvariant) {}

and then verify that the resource usage of processHasInvariant and processHasNoInvariant are equal: that processHasInvariant is agnostic to the existence of the invariant.

Copy link
Member

@MikaelMayer MikaelMayer left a comment

Choose a reason for hiding this comment

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

Initial comments as I'm not done but it can still help

Copy link
Member

@MikaelMayer MikaelMayer left a comment

Choose a reason for hiding this comment

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

Finished commenting

ssomayyajula and others added 3 commits August 19, 2025 18:36
- Open set is empty everywhere
- Invariant is inductive over program transitions and constructors are the initial condition
- Consequently, invariants must hold by the end of the first constructor phase
- VCs removed to retain performance of non-invariant-utilizing code (see Performance.dfy)
@ssomayyajula
Copy link
Contributor Author

Note for myself: don't delete the branch after merging.

Copy link
Member

@keyboardDrummer keyboardDrummer left a comment

Choose a reason for hiding this comment

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

Without a test that proves that code that does not use a type invariant, but does use the type, does not have its verification performance affected by the invariant's existence, I don't think we should merge this.

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