-
Notifications
You must be signed in to change notification settings - Fork 271
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: Compile run-time constraint checks for newtypes in comprehensions #4919
Conversation
# Conflicts: # Source/DafnyCore/Resolver/ModuleResolver.cs
# Conflicts: # Source/DafnyCore/AST/TypeDeclarations/NewtypeDecl.cs # Source/DafnyCore/Resolver/ModuleResolver.cs # Source/IntegrationTests/TestFiles/LitTests/LitTest/cli/diagnosticsFormats.legacy.dfy.expect # Source/IntegrationTests/TestFiles/LitTests/LitTest/dafny0/NativeTypeResolution.dfy.expect # Source/IntegrationTests/TestFiles/LitTests/LitTest/git-issues/git-issue-276.dfy.expect # Source/IntegrationTests/TestFiles/LitTests/LitTest/git-issues/git-issue-276a.dfy.expect
# Conflicts: # Source/DafnyCore/Resolver/ModuleResolver.cs # Source/IntegrationTests/TestFiles/LitTests/LitTest/cli/diagnosticsFormats.legacy.dfy.expect # Source/IntegrationTests/TestFiles/LitTests/LitTest/dafny0/NativeTypeResolution.dfy.expect # Source/IntegrationTests/TestFiles/LitTests/LitTest/git-issues/git-issue-276.dfy.expect # Source/IntegrationTests/TestFiles/LitTests/LitTest/git-issues/git-issue-276a.dfy.expect
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.
Can newtypes not be converted to subset types before code generation? Why is there back-end specific code for them?
No. Well, at least not all newtypes. Because a |
# Conflicts: # Source/DafnyCore/Resolver/SubsetConstraintGhostChecker.cs
I see. Still, I have a feeling lots of code generation elements are conflated at the moment. Would you mind helping me come up with a plan to extract much of the subset and newtype handling in code generation to a separate pass? I think it would already be a great step if we only write down what we'd like. I imagine we could have a pass "InstantiateComprehensionPredicateTypeFilters" that inlines the predicates from subset and newtypes in comprehensions, so that the implicit filter operation (if I understand correctly), is made explicit. Should this pass run as part of the resolver? Could it help with verification as well? A phase after the comprehension filter instantiation, could annotate matching newtypes with |
Yes. Great! There are many little things to improve. The main difference between subset types and newtypes is in the resolver. The compiler could do well to focus on what types are to be compiled as "nativeType" or, as you put it, "extern". There's also a slight (but really unnecessary) difference in the handling of subset types / newtypes, because a newtype may not have a |
How do you feel about what I described though, which I hope would make all back-ends completely agnostic to subset and newtypes? |
@keyboardDrummer Yes, I like your suggested plan. A pass to compute the predicates would fit nicely in the resolver, so that all compilers could use the information. Here are some thoughts from the top of my head:
|
Comprehensions can declare their bound variables to be of types that have constraint (e.g., subset types). These are (in the general case) compiled into run-time checks. However, this machinery did not pay attention to newtypes, which also have constraints. This PR fixes this, so that bound comprehension variables of
newtype
types also compile into run-time checks.The new functionality is tested in
git-issues/git-issue-697i.dfy
andgit-issues/git-issue-697j.dfy
.By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.