-
Notifications
You must be signed in to change notification settings - Fork 268
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
feat: Add support for disjunctive (“or”) patterns #2448
Merged
Merged
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
de0ab09
feat: Add support for disjunctive (“or”) patterns
cpitclaudel 17a5a68
Merge branch 'master' into cpitclaudel_deep-or-patterns
keyboardDrummer 2ef4189
Rename dp and psep
cpitclaudel 53808c6
Rename RemoveDisjunctivePatterns and clean up its implementation
cpitclaudel afb8718
;
cpitclaudel ff2869c
Merge branch 'master' into cpitclaudel_deep-or-patterns
cpitclaudel 8973659
Merge branch 'master' into cpitclaudel_deep-or-patterns
cpitclaudel d326179
Refactor check for wildcard patterns
cpitclaudel 46b34e3
Merge branch 'master' into cpitclaudel_deep-or-patterns
cpitclaudel aa8d2b1
Move release notes to Upcoming section
cpitclaudel d6f9242
Fix IsWildcardPattern test
cpitclaudel 1c89a9d
Preserve ResolvedLit and add a test for that case
cpitclaudel cd7d728
Merge remote-tracking branch 'origin/master' into cpitclaudel_deep-or…
cpitclaudel a0c4848
Format
cpitclaudel b04bdf1
Preserve pattern type
cpitclaudel 48161bd
Merge branch 'master' into cpitclaudel_deep-or-patterns
cpitclaudel 5dbd839
Fix test output
cpitclaudel b39ac54
Don't clone the body of the matches (see dafny-lang/dafny#2334)
cpitclaudel 85bdb90
Merge branch 'master' into cpitclaudel_deep-or-patterns
cpitclaudel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
// RUN: %dafny_0 /compile:0 "%s" > "%t" | ||
// RUN: %diff "%s.expect" "%t" | ||
|
||
module SanityChecks { | ||
datatype T = A(int) | B(nat) | C(bool) | ||
|
||
method Variables(t: T) { | ||
match t | ||
case A(n) // Error: Or-patterns may not bind variables | ||
| B(n) => // Error: Or-patterns may not bind variables | ||
case _ => | ||
} | ||
|
||
method Nesting(t: T) { | ||
match t | ||
case A(1 | 2 | _) => // Error: Or-patterns are not allowed inside other patterns | ||
case B(0 | _) // Error: Or-patterns are not allowed inside other patterns | ||
| C(_ | _ | _) => // Error: Or-patterns are not allowed inside other patterns | ||
} | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
OrPatternErrors.dfy(9,13): Error: Disjunctive patterns may not bind variables | ||
OrPatternErrors.dfy(10,13): Error: Disjunctive patterns may not bind variables | ||
OrPatternErrors.dfy(16,13): Error: Disjunctive patterns are not allowed inside other patterns | ||
OrPatternErrors.dfy(17,13): Error: Disjunctive patterns are not allowed inside other patterns | ||
OrPatternErrors.dfy(18,13): Error: Disjunctive patterns are not allowed inside other patterns | ||
5 resolution/type errors detected in OrPatternErrors.dfy |
Oops, something went wrong.
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.
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.
What is the right time for flattening the disjunctive patterns? Can't that be done before resolution as a separate rewriter?
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.
Possibly, but I'm not sure it's what we want: I wrote it this way to give other code access to the disjunctive patterns. I think of this flattening as an implementation detail of the pattern compiler.
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.
Didn't we want to use many small lowering transformations when possible, to make each individual transformation simpler? I think it would be a big benefit if we can avoid adding code to
Resolver.cs
.Also, in what scenario do you envision someone needs access to disjunctive patterns? And if they need it, why couldn't they sit before the lowering transformation?
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.
Indeed, and we have that here, right? There's just a handful of calls in the resolver to new code that pre-processes the pattern.
I can move it to a new file, but all of the other pattern matching compilation code is there, so… ?
Originally, the pattern matching compiler was implemented all in the parser. It was moved to the resolver, and in fact it should really be moved to the compiler. In fact, in the long run, disjunctive patterns should be supported at any level of nesting, so the nice separation we have right now won't survive. But for now it's good to have the separated implementation.
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 meant moving the code to a separate file and not calling it from Resolver.cs, except for possibly adding a
DisjunctCaseRewriter
to therewriters
list. Would that be possible?I don't know the pattern matching transformations well enough to comment.
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.
Possibly? I think it might work, because this pass doesn't use type information. The problem is that pattern matching compilation happens in the middle of resolution, and it uses some type info.
Also, do we have the architecture in place for writing such a pass? I need something that traverses all statements and expressions in a program. Do we have a visitor that does that?
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.
Editing to add this following our conversation: it would be nice to add such a visitor, but one issue with rewriting this systematically would be that clients would lose access to the unresolved AST containing the full match structure.