-
Notifications
You must be signed in to change notification settings - Fork 267
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
fix: Make datatype cycle detection independent of auto-init #4997
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # Source/IntegrationTests/TestFiles/LitTests/LitTest/dafny0/TypeTests.dfy.expect # Source/IntegrationTests/TestFiles/LitTests/LitTest/git-issues/git-issue-2139.dfy.expect
…ing pre-resolution
ssomayyajula
approved these changes
Jan 23, 2024
RustanLeino
added a commit
to RustanLeino/dafny
that referenced
this pull request
Feb 6, 2024
Fixes dafny-lang#4983 Actually, this was fixed already in dafny-lang#4997
This was referenced Feb 6, 2024
RustanLeino
added a commit
that referenced
this pull request
Feb 7, 2024
This PR fixes and closes three reported bugs related to datatypes extending traits: * Reference-type traits used with `import opened` were not recognized as reference types (issue #4936). * Implicit conversions from a datatype to a parent trait had caused malformed Boogie (issue #4983). This was actually fixed in PR #4997, but the present PR adds tests for it. * Equality (and disequality) comparisons with a datatype (or codatatype) on the _left_ and a parent trait on the _right_ had caused malformed Boogie (issue #4994). Fixes #4936 Closes #4983 Fixes #4994 <small>By submitting this pull request, I confirm that my contribution is made under the terms of the [MIT license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR makes the "datatype has no instances" checks more liberal.
Fixes #4939
Description
An inductive datatype is allowed to be defined as an empty type. For example, in
Record
is an empty type, becauseSubset
is, sinceP(x)
is alwaysfalse
. But ifP(x)
was instead defined to betrue
for somex
's, thenRecord
would be nonempty. Determining whether or notRecord
is empty goes well beyond the syntactic checks of the type system.However, if a datatype is empty because of some "obvious" cycle among datatype definitions, then that is both detectable by syntactic checks and likely unintended by the programmer. By this PR, Dafny will search for such type declarations and give error messages if an inductive datatype is necessarily involved in such an "obvious" cycle.
Previous behavior
Dafny had attempted this check before, too, but then the rule was too strict. The check had been implemented as part of the machinery that computed "grounding constructors". That search consulted the auto-init status of the types that a datatype depends on. Since traits are of the "possibly empty" auto-init category, if every constructor of a datatype depended on a trait (that is, the constructors had a parameter whose type is a trait), then Dafny would report an error.
This PR removes the error reporting done by the machinery that computes grounding constructors. It adds a new, separate check in the resolver to determine the "obvious" cycles.
Grounding constructors
The name "grounding constructor" was originally to mean "a constructor that can be used to construct an instance of the datatype". This name is rather outdated, because Dafny's syntactic checks do not ensure datatypes to be nonempty anyway (case in point: type
Record
above). A name like "initializing constructor" would be more to the point--the compiler uses this constructor in a generated method for creating an arbitrary value of the type. (At run time, this method will never be called if the datatype really is empty, but since the compiler does not know this, it still has to generate valid target code for this method.)Alternative designs
One alternative design would be to change the new "because of a cycle, this datatype has no instances" error message to be just a warning. This alternative has some merit, since Dafny already supports empty types.
Another alternative would be to not perform the check at all, and to report neither an error nor a warning for datatypes with no instances. This would give more flexibility. However, since there are simple ways to write an empty type, a program where a datatype cycle causes a type to have no instances is most likely a mistake that the programmer would want to know about and fix.
By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.