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

feat: Add support for disjunctive (“or”) patterns #2448

Merged
merged 19 commits into from
Jul 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion RELEASE_NOTES.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
# Upcoming

- feat: New option `-diagnosticsFormat:json` to print Dafny error messages as JSON objects (one per line). (https://github.com/dafny-lang/dafny/pull/2363)
- feat: New option `-diagnosticsFormat:json` to print Dafny error messages as JSON objects (one per line).
(https://github.com/dafny-lang/dafny/pull/2363)
- feat: Dafny now supports disjunctive (“or”) patterns in match statements and expressions. Cases are separated by `|` characters. Disjunctive patterns may not appear within other patterns and may not bind variables.
(https://github.com/dafny-lang/dafny/pull/2448)

# 3.7.3

Expand Down
12 changes: 12 additions & 0 deletions Source/Dafny/AST/DafnyAst.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12367,6 +12367,15 @@ public ExtendedPattern(IToken tok, bool isGhost = false) {
this.IsGhost = isGhost;
}
}

public class DisjunctivePattern : ExtendedPattern {
public readonly List<ExtendedPattern> Alternatives;
public DisjunctivePattern(IToken tok, List<ExtendedPattern> alternatives, bool isGhost = false) : base(tok, isGhost) {
Contract.Requires(alternatives != null && alternatives.Count > 0);
this.Alternatives = alternatives;
}
}

public class LitPattern : ExtendedPattern {
public readonly Expression OrigLit; // the expression as parsed; typically a LiteralExpr, but could be a NegationExpression

Expand Down Expand Up @@ -12430,6 +12439,9 @@ public class IdPattern : ExtendedPattern {
public List<ExtendedPattern> Arguments; // null if just an identifier; possibly empty argument list if a constructor call
public LiteralExpr ResolvedLit; // null if just an identifier

public bool IsWildcardPattern =>
Arguments == null && Id.StartsWith("_");

public void MakeAConstructor() {
this.Arguments = new List<ExtendedPattern>();
}
Expand Down
9 changes: 8 additions & 1 deletion Source/Dafny/AST/Printer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2870,13 +2870,20 @@ void PrintExtendedPattern(ExtendedPattern pat) {
wr.Write(")");
}
break;
case DisjunctivePattern dPat:
var patSep = "";
foreach (var arg in dPat.Alternatives) {
wr.Write(patSep);
PrintExtendedPattern(arg);
patSep = " | ";
}
break;
case LitPattern litPat:
wr.Write(litPat.ToString());
break;
}
}


private void PrintQuantifierDomain(List<BoundVar> boundVars, Attributes attrs, Expression range) {
Contract.Requires(boundVars != null);
string sep = "";
Expand Down
19 changes: 10 additions & 9 deletions Source/Dafny/Cloner.cs
Original file line number Diff line number Diff line change
Expand Up @@ -688,15 +688,16 @@ public MatchCaseStmt CloneMatchCaseStmt(MatchCaseStmt c) {
}

public ExtendedPattern CloneExtendedPattern(ExtendedPattern pat) {
if (pat is LitPattern) {
var p = (LitPattern)pat;
return new LitPattern(p.Tok, CloneExpr(p.OrigLit));
} else if (pat is IdPattern) {
var p = (IdPattern)pat;
return new IdPattern(p.Tok, p.Id, p.Arguments == null ? null : p.Arguments.ConvertAll(CloneExtendedPattern));
} else {
Contract.Assert(false);
return null;
switch (pat) {
case LitPattern p:
return new LitPattern(p.Tok, CloneExpr(p.OrigLit));
case IdPattern p:
return new IdPattern(p.Tok, p.Id, p.Arguments == null ? null : p.Arguments.ConvertAll(CloneExtendedPattern));
case DisjunctivePattern p:
return new DisjunctivePattern(p.Tok, p.Alternatives.ConvertAll(CloneExtendedPattern), p.IsGhost);
default:
Contract.Assert(false);
return null;
}
}
public NestedMatchCaseStmt CloneNestedMatchCaseStmt(NestedMatchCaseStmt c) {
Expand Down
13 changes: 12 additions & 1 deletion Source/Dafny/Dafny.atg
Original file line number Diff line number Diff line change
Expand Up @@ -3289,7 +3289,7 @@ BindingGuard<out Expression e, bool allowLambda>


/*------------------------------------------------------------------------*/
ExtendedPattern<.out ExtendedPattern pat.>
SingleExtendedPattern<.out ExtendedPattern pat.>
= (. IToken id; List<ExtendedPattern> arguments;
Expression lit; BoundVar bv;
pat = null;
Expand Down Expand Up @@ -3324,6 +3324,17 @@ ExtendedPattern<.out ExtendedPattern pat.>
.)
.

/*------------------------------------------------------------------------*/
ExtendedPattern<.out ExtendedPattern pat.>
= (. List<ExtendedPattern> branches = null;
ExtendedPattern branch = null; .)
SingleExtendedPattern<out branch>
{ "|" (. branches ??= new() { branch }; .)
SingleExtendedPattern<out branch> (. branches.Add(branch); .)
}
(. pat = branches == null ? branch : new DisjunctivePattern(branches[0].Tok, branches); .)
.

/*------------------------------------------------------------------------*/
MatchStmt<out Statement/*!*/ s>
= (. Contract.Ensures(Contract.ValueAtReturn(out s) != null);
Expand Down
104 changes: 77 additions & 27 deletions Source/Dafny/Resolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,20 +44,11 @@ private bool VisibleInScope(Declaration d) {
return useCompileSignatures || d.IsVisibleInScope(moduleInfo.VisibilityScope);
}

FreshIdGenerator defaultTempVarIdGenerator;
FreshIdGenerator defaultTempVarIdGenerator = new FreshIdGenerator();

string FreshTempVarName(string prefix, ICodeContext context) {
var decl = context as Declaration;
if (decl != null) {
return decl.IdGenerator.FreshId(prefix);
}

// TODO(wuestholz): Is the following code ever needed?
if (defaultTempVarIdGenerator == null) {
defaultTempVarIdGenerator = new FreshIdGenerator();
}

return defaultTempVarIdGenerator.FreshId(prefix);
var gen = context is Declaration decl ? decl.IdGenerator : defaultTempVarIdGenerator;
return gen.FreshId(prefix);
}

interface IAmbiguousThing<Thing> {
Expand Down Expand Up @@ -11948,6 +11939,7 @@ private abstract class RBranch {
public List<ExtendedPattern> Patterns;

public RBranch(IToken tok, int branchid, List<ExtendedPattern> patterns) {
Contract.Requires(patterns.All(p => !(p is DisjunctivePattern)));
this.Tok = tok;
this.BranchID = branchid;
this.Patterns = patterns;
Expand All @@ -11965,7 +11957,8 @@ public RBranchStmt(IToken tok, int branchid, List<ExtendedPattern> patterns, Lis
}

public RBranchStmt(int branchid, NestedMatchCaseStmt x, Attributes attrs = null) : base(x.Tok, branchid, new List<ExtendedPattern>()) {
this.Body = new List<Statement>(x.Body); // Resolving the body will insert new elements.
Contract.Requires(!(x.Pat is DisjunctivePattern)); // No nested or patterns
this.Body = new List<Statement>(x.Body); // Resolving the body will insert new elements.
this.Attributes = attrs;
this.Patterns.Add(x.Pat);
}
Expand Down Expand Up @@ -12085,7 +12078,7 @@ private void LetBind(RBranch branch, IdPattern var, Expression genExpr) {
// If cp is not a wildcard, replace branch.Body with let cp = expr in branch.Body
// Otherwise do nothing
private void LetBindNonWildCard(RBranch branch, IdPattern var, Expression expr) {
if (!var.Id.StartsWith("_")) {
if (!var.IsWildcardPattern) {
LetBind(branch, var, expr);
}
}
Expand Down Expand Up @@ -12191,7 +12184,7 @@ private void PrintRBranches(MatchingContext context, List<Expression> matchees,
* PairPB contains, for each branches, its head pattern and the rest of the branch.
*/
private SyntaxContainer CompileRBranchConstant(MatchTempInfo mti, MatchingContext context, Expression currMatchee, List<Expression> matchees, List<Tuple<ExtendedPattern, RBranch>> pairPB) {
// Decreate the count for each branch (increases back for each occurence later on)
// Decrease the count for each branch (increases back for each occurence later on)
foreach (var PB in pairPB) {
mti.UpdateBranchID(PB.Item2.BranchID, -1);
}
Expand Down Expand Up @@ -12502,6 +12495,49 @@ private SyntaxContainer CompileRBranch(MatchTempInfo mti, MatchingContext contex
}
}

private ExtendedPattern RemoveIllegalSubpatterns(ExtendedPattern pat, bool inDisjunctivePattern) {
switch (pat) {
case LitPattern:
return pat;
case IdPattern p:
if (inDisjunctivePattern && p.ResolvedLit == null && p.Arguments == null && !p.IsWildcardPattern) {
reporter.Error(MessageSource.Resolver, pat.Tok, "Disjunctive patterns may not bind variables");
return new IdPattern(p.Tok, FreshTempVarName("_", null), null, p.IsGhost);
}
var args = p.Arguments?.ConvertAll(a => RemoveIllegalSubpatterns(a, inDisjunctivePattern));
return new IdPattern(p.Tok, p.Id, p.Type, args, p.IsGhost) { ResolvedLit = p.ResolvedLit };
case DisjunctivePattern p:
reporter.Error(MessageSource.Resolver, pat.Tok, "Disjunctive patterns are not allowed inside other patterns");
return new IdPattern(p.Tok, FreshTempVarName("_", null), null, p.IsGhost);
default:
Contract.Assert(false);
return null;
}
}

private IEnumerable<ExtendedPattern> FlattenDisjunctivePatterns(ExtendedPattern pat) {
// TODO: Once we rewrite the pattern-matching compiler, we'll handle disjunctive patterns in it, too.
// For now, we handle top-level disjunctive patterns by duplicating the corresponding cases here, and disjunctive
// sub-patterns are unsupported.
return pat is DisjunctivePattern p
? p.Alternatives.ConvertAll(a => RemoveIllegalSubpatterns(a, inDisjunctivePattern: true))
: Enumerable.Repeat(RemoveIllegalSubpatterns(pat, inDisjunctivePattern: false), 1);
}

private IEnumerable<NestedMatchCaseExpr> FlattenNestedMatchCaseExpr(NestedMatchCaseExpr c) {
var cloner = new Cloner();
foreach (var pat in FlattenDisjunctivePatterns(c.Pat)) {
yield return new NestedMatchCaseExpr(c.Tok, pat, c.Body, c.Attributes);
}
}

private IEnumerable<NestedMatchCaseStmt> FlattenNestedMatchCaseStmt(NestedMatchCaseStmt c) {
var cloner = new Cloner();
foreach (var pat in FlattenDisjunctivePatterns(c.Pat)) {
yield return new NestedMatchCaseStmt(c.Tok, pat, new List<Statement>(c.Body), c.Attributes);
}
}

private void CompileNestedMatchExpr(NestedMatchExpr e, ResolveOpts opts) {
if (e.ResolvedExpression != null) {
//post-resolve, skip
Expand All @@ -12511,12 +12547,13 @@ private void CompileNestedMatchExpr(NestedMatchExpr e, ResolveOpts opts) {
Console.WriteLine("DEBUG: CompileNestedMatchExpr for match at line {0}", e.tok.line);
}

MatchTempInfo mti = new MatchTempInfo(e.tok, e.Cases.Count, opts.codeContext, DafnyOptions.O.MatchCompilerDebug);
var cases = e.Cases.SelectMany(FlattenNestedMatchCaseExpr).ToList();
Copy link
Member

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?

Copy link
Member Author

@cpitclaudel cpitclaudel Jul 20, 2022

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.

Copy link
Member

@keyboardDrummer keyboardDrummer Jul 21, 2022

Choose a reason for hiding this comment

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

I wrote it this way to give other code access to the disjunctive patterns.

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?

Copy link
Member Author

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?

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 think it would be a big benefit if we can avoid adding code to Resolver.cs.

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.

Copy link
Member

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… ?

I meant moving the code to a separate file and not calling it from Resolver.cs, except for possibly adding a DisjunctCaseRewriter to the rewriters list. Would that be possible?

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.

I don't know the pattern matching transformations well enough to comment.

Copy link
Member Author

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 the rewriters list. Would that be possible?

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?

Copy link
Member Author

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.

MatchTempInfo mti = new MatchTempInfo(e.tok, cases.Count, opts.codeContext, DafnyOptions.O.MatchCompilerDebug);

// create Rbranches from MatchCaseExpr and set the branch tokens in mti
List<RBranch> branches = new List<RBranch>();
for (int id = 0; id < e.Cases.Count; id++) {
var branch = e.Cases.ElementAt(id);
for (int id = 0; id < cases.Count; id++) {
var branch = cases.ElementAt(id);
branches.Add(new RBranchExpr(id, branch, branch.Attributes));
mti.BranchTok[id] = branch.Tok;
}
Expand All @@ -12535,8 +12572,8 @@ private void CompileNestedMatchExpr(NestedMatchExpr e, ResolveOpts opts) {
if (mti.BranchIDCount[id] <= 0) {
reporter.Warning(MessageSource.Resolver, mti.BranchTok[id], "this branch is redundant ");
scope.PushMarker();
CheckLinearNestedMatchCase(e.Source.Type, e.Cases.ElementAt(id), opts);
ResolveExpression(e.Cases.ElementAt(id).Body, opts);
CheckLinearNestedMatchCase(e.Source.Type, cases.ElementAt(id), opts);
ResolveExpression(cases.ElementAt(id).Body, opts);
scope.PopMarker();
}
}
Expand Down Expand Up @@ -12565,13 +12602,14 @@ private void CompileNestedMatchStmt(NestedMatchStmt s, ResolveOpts opts) {
Console.WriteLine("DEBUG: CompileNestedMatchStmt for match at line {0}", s.Tok.line);
}

var cases = s.Cases.SelectMany(FlattenNestedMatchCaseStmt).ToList();
// initialize the MatchTempInfo to record position and duplication information about each branch
MatchTempInfo mti = new MatchTempInfo(s.Tok, s.EndTok, s.Cases.Count, codeContext, DafnyOptions.O.MatchCompilerDebug, s.Attributes);
MatchTempInfo mti = new MatchTempInfo(s.Tok, s.EndTok, cases.Count, codeContext, DafnyOptions.O.MatchCompilerDebug, s.Attributes);

// create Rbranches from NestedMatchCaseStmt and set the branch tokens in mti
List<RBranch> branches = new List<RBranch>();
for (int id = 0; id < s.Cases.Count; id++) {
var branch = s.Cases.ElementAt(id);
for (int id = 0; id < cases.Count; id++) {
var branch = cases.ElementAt(id);
branches.Add(new RBranchStmt(id, branch, branch.Attributes));
mti.BranchTok[id] = branch.Tok;
}
Expand All @@ -12589,8 +12627,8 @@ private void CompileNestedMatchStmt(NestedMatchStmt s, ResolveOpts opts) {
if (mti.BranchIDCount[id] <= 0) {
reporter.Warning(MessageSource.Resolver, mti.BranchTok[id], "this branch is redundant");
scope.PushMarker();
CheckLinearNestedMatchCase(s.Source.Type, s.Cases.ElementAt(id), opts);
s.Cases.ElementAt(id).Body.ForEach(s => ResolveStatement(s, codeContext));
CheckLinearNestedMatchCase(s.Source.Type, cases.ElementAt(id), opts);
cases.ElementAt(id).Body.ForEach(s => ResolveStatement(s, codeContext));
scope.PopMarker();
}
}
Expand All @@ -12615,7 +12653,7 @@ private void CheckLinearVarPattern(Type type, IdPattern pat, ResolveOpts opts) {

if (scope.FindInCurrentScope(pat.Id) != null) {
reporter.Error(MessageSource.Resolver, pat.Tok, "Duplicate parameter name: {0}", pat.Id);
} else if (pat.Id.StartsWith("_")) {
} else if (pat.IsWildcardPattern) {
// Wildcard, ignore
return;
} else {
Expand Down Expand Up @@ -12653,6 +12691,7 @@ private void CheckLinearVarPattern(Type type, IdPattern pat, ResolveOpts opts) {
}

// pat could be
// 0 - A DisjunctivePattern
// 1 - An IdPattern (without argument) at base type
// 2 - A LitPattern at base type
// 3* - An IdPattern at tuple type representing a tuple
Expand All @@ -12663,7 +12702,17 @@ private void CheckLinearExtendedPattern(Type type, ExtendedPattern pat, ResolveO
return;
}

if (!type.IsDatatype) { // Neither tuple nor datatype
if (pat is DisjunctivePattern dp) {
foreach (var alt in dp.Alternatives) {
// Pushing a scope silences the “duplicate parameter” error in
// `CheckLinearVarPattern`. This is acceptable because disjunctive
// patterns are not allowed to bind variables (the corresponding
// error is raised in `RemoveDisjunctivePatterns`).
scope.PushMarker();
CheckLinearExtendedPattern(type, alt, opts);
scope.PopMarker();
}
} else if (!type.IsDatatype) { // Neither tuple nor datatype
if (pat is IdPattern id) {
if (id.Arguments != null) {
// pat is a tuple or constructor
Expand Down Expand Up @@ -12716,6 +12765,7 @@ private void CheckLinearExtendedPattern(Type type, ExtendedPattern pat, ResolveO
return;
} else { // matching a datatype value
if (!(pat is IdPattern)) {
Contract.Assert(pat is LitPattern);
reporter.Error(MessageSource.Resolver, pat.Tok, "Constant pattern used in place of datatype");
}
IdPattern idpat = (IdPattern)pat;
Expand Down
20 changes: 20 additions & 0 deletions Test/patterns/OrPatternErrors.dfy
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
}
}
6 changes: 6 additions & 0 deletions Test/patterns/OrPatternErrors.dfy.expect
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
Loading