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

Properly check if types may contain references #1924

Merged
merged 15 commits into from
Apr 28, 2022

Conversation

RustanLeino
Copy link
Collaborator

Previously, types were not properly checked for whether or not they may contain references. This PR fixes that.

Fixes #1419

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

cpitclaudel
cpitclaudel previously approved these changes Mar 22, 2022
Copy link
Member

@cpitclaudel cpitclaudel left a comment

Choose a reason for hiding this comment

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

For datatypes, why is it enough to check just the type parameters? The following looks off to me:

class C {}
datatype D = DC(c: C)
type X(!new) = D // No error

datatype Obs' = Obs'(s: set<object>)
predicate lotsOfObjects'() {
  exists o: Obs' :: |o.s| > 10 // No error
}

And also this:

class C {}

datatype Box<T> = Box(t: T)
type Box'(!new) = Box<C> // Rejected

datatype BoxC = BoxC(c: C)
type BoxC'(!new) = BoxC // No error

Am I missing something?

@RustanLeino
Copy link
Collaborator Author

@cpitclaudel Thank you. These are (still) bugs.

@cpitclaudel
Copy link
Member

Do we adress them in the same PR, or merge this one and open a separate ticket for the next?

The Schorr-Waite test programs are not allowed by the new rules. To sort this out, Dafny needs a more flexible mechanism to indicate that quantifiers only quantify over allocated state. I will add this mechanism in a separate PR and then return to this one.
@RustanLeino
Copy link
Collaborator Author

@cpitclaudel I addressed the further issues in this PR with the commit I just pushed. However, the fix causes errors that the old Schorr-Waite test programs have a function that depends on the allocation state. To sort this out, Dafny needs a more flexible mechanism to indicate that quantifiers only quantify over allocated state. I will add this mechanism in a separate PR and then return to this one.

These `:older` attributes on `ReachableVia` will once again allow the `SchorrWaite*.dfy` tests, as soon as dafny-lang#1936 is merged.
Source/Dafny/AST/DafnyAst.cs Outdated Show resolved Hide resolved
cpitclaudel
cpitclaudel previously approved these changes Mar 30, 2022
Copy link
Member

@cpitclaudel cpitclaudel left a comment

Choose a reason for hiding this comment

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

Very nice, thanks!

# Conflicts:
#	RELEASE_NOTES.md
@RustanLeino RustanLeino enabled auto-merge (squash) April 28, 2022 17:11
@RustanLeino RustanLeino merged commit f933262 into dafny-lang:master Apr 28, 2022
@RustanLeino RustanLeino deleted the issue-1419 branch April 28, 2022 20:45
cpitclaudel added a commit that referenced this pull request May 4, 2022
The new check is correct, but this is not fully satisfactory, as it removes a
useful debugging aid.

Ideally, it would be nice to be able to mark these extern classes as frozen, so
that they might be treated as `!new`.
cpitclaudel added a commit to dafny-lang/compiler-bootstrap that referenced this pull request Jun 28, 2022
The new check is correct, but this is not fully satisfactory, as it removes a
useful debugging aid.

Ideally, it would be nice to be able to mark these extern classes as frozen, so
that they might be treated as `!new`.
cpitclaudel added a commit to dafny-lang/compiler-bootstrap that referenced this pull request Jul 12, 2022
The new check is correct, but this is not fully satisfactory, as it removes a
useful debugging aid.

Ideally, it would be nice to be able to mark these extern classes as frozen, so
that they might be treated as `!new`.
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.

function value can depend on set of allocated objects
2 participants