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

refactor: Clean up resolution passes 0 and 1 #3284

Merged
merged 60 commits into from
Dec 29, 2022
Merged
Show file tree
Hide file tree
Changes from 56 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
9aa45b9
Introduce IASTVisitorContext
RustanLeino Dec 21, 2022
e6d6bbe
Introduce ASTVisitor
RustanLeino Dec 21, 2022
f513f96
Introduce CallGraphASTVisitor
RustanLeino Dec 21, 2022
00f1ed8
Use IASTVisitorContext instead of ICodeContext as CallGraphBuilderCon…
RustanLeino Dec 21, 2022
363b50d
Move work from VisitFunction to VisitFunctionProper
RustanLeino Dec 21, 2022
5c01d60
Move work from VisitMethod to VisitMethodProper
RustanLeino Dec 21, 2022
6b7f96e
Change CallGraphBuilder visitor into a subclass of ASTVisitor
RustanLeino Dec 21, 2022
10069f5
Remove code that moved into ASTVisitor
RustanLeino Dec 21, 2022
b500fa2
Move visitor methods from CallGraphBuilder to CallGraphASTVisitor to
RustanLeino Dec 21, 2022
4598fd9
Generalize context parameter for ASTVisitor
RustanLeino Dec 21, 2022
1a082e9
Move visitation of user-provided types, subexpressions, and substatem…
RustanLeino Dec 22, 2022
74b96fc
Virtualize declaration/member visitation as well
RustanLeino Dec 22, 2022
35c70d7
Merge branch 'master' into AstVisitor
RustanLeino Dec 22, 2022
33bb66c
Move ASTVisitor and IASTVisitorContext into a separate file
RustanLeino Dec 23, 2022
10c2f32
Merge branch 'master' into AstVisitor
RustanLeino Dec 23, 2022
44dabc0
Visit StmtExpr’s substatements before its subexpressions
RustanLeino Dec 23, 2022
854ee25
Move type-determined checks a little later
RustanLeino Dec 22, 2022
8111b99
Improved names of identifiers
RustanLeino Dec 22, 2022
cc7caf5
Collect code into new ResolveNamesAndInferTypes method
RustanLeino Dec 22, 2022
4edf858
Improve code organization
RustanLeino Dec 22, 2022
a8a5766
Resolve attributes last
RustanLeino Dec 22, 2022
b179549
Combined some post-name-resolution visitation checks
RustanLeino Dec 23, 2022
a2b7d51
fix: Disallow “this” in non-instance attributes
RustanLeino Dec 23, 2022
af01ea3
Group similar checks into the same visitation pass
RustanLeino Dec 23, 2022
c79e628
Do CheckTypeInference together
RustanLeino Dec 23, 2022
5dd1182
Merge branch 'master' into check-type-inference
RustanLeino Dec 23, 2022
26baac4
Separate type-inference checking, bounds discovery, and ghost checking
RustanLeino Dec 24, 2022
e48f7ab
Adjust lines in test file
RustanLeino Dec 24, 2022
7638787
Split test into two modules
RustanLeino Dec 24, 2022
56fd78a
Adjust test to show both errors
RustanLeino Dec 24, 2022
cd6055e
Improve comments
RustanLeino Dec 24, 2022
ea2f677
Splits tests to get errors from what is now different passes
RustanLeino Dec 24, 2022
8647a91
Check type inference of the beginning of prefix lemmas earlier
RustanLeino Dec 24, 2022
2b41297
Update expected test output
RustanLeino Dec 24, 2022
0e5e4d3
Check CheckTypeInference… to only take IASTVisitorContext, not ICodeC…
RustanLeino Dec 24, 2022
df5a707
Improve CheckTypeInference for const
RustanLeino Dec 24, 2022
3924295
Improve code
RustanLeino Dec 24, 2022
86a3369
Improve organization of CheckTypeInference visitor
RustanLeino Dec 27, 2022
852f7fe
fix: Infer (==) for non-null iterator type
RustanLeino Dec 27, 2022
e0ec038
To reduce special cases, also include NonNullTypeDecl in cyclicity ch…
RustanLeino Dec 27, 2022
fead483
refactor: Use AST Visitor instead of BottomUp visitor
RustanLeino Dec 27, 2022
5f678a9
Make resolution/checking of attributes consistent
RustanLeino Dec 28, 2022
d2aafd0
Make recording of warnShadowing attribute consistent
RustanLeino Dec 28, 2022
805f70d
Better document the pre/postcondition of ResolveParameterDefaultValues
RustanLeino Dec 28, 2022
fe502c3
fix: Set scope correctly for resolving attributes of top-level declar…
RustanLeino Dec 28, 2022
87fb940
fix: Indent member block of newtypes
RustanLeino Dec 28, 2022
7dae2ae
refactor: Move “(==) unnecessary” check to later pass
RustanLeino Dec 28, 2022
477b418
Add post-visitor (bottom up) to ASTVisitor
RustanLeino Dec 28, 2022
c906a0d
Change CheckTypeInference (back) to be a bottom-up traversal
RustanLeino Dec 28, 2022
85c947d
Improve code
RustanLeino Dec 28, 2022
d413947
Merge branch 'master' into check-type-inference
RustanLeino Dec 28, 2022
413c75b
Move some pass-0 code from Resolver.cs to NameResolutionAndTypeInfere…
RustanLeino Dec 28, 2022
00db663
Move default-parameter substitution to pass 0 from pass 1
RustanLeino Dec 28, 2022
4a66545
Add user-visible release notes
RustanLeino Dec 28, 2022
0e62b8c
Rename CheckTypeInference_Visitor.cs TypeInferenceChecker.cs
RustanLeino Dec 28, 2022
2801491
Fix whitespace
RustanLeino Dec 28, 2022
cd81c32
Respond to review comments
RustanLeino Dec 28, 2022
2c7b341
Split Resolve…OneDeclaration and Resolve..MemberBodies into two metho…
RustanLeino Dec 28, 2022
0de1991
Report type-parameter errors earlier
RustanLeino Dec 28, 2022
570abb9
Merge branch 'master' into check-type-inference
RustanLeino Dec 29, 2022
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
54 changes: 36 additions & 18 deletions Source/DafnyCore/AST/AstVisitor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ protected virtual void VisitOneDeclaration(TopLevelDecl decl) {
}

if (decl is TopLevelDeclWithMembers cl) {
cl.Members.Iter(member => VisitMember(cl, member));
cl.Members.Iter(VisitMember);
}
}

Expand Down Expand Up @@ -89,19 +89,9 @@ private void VisitIterator(IteratorDecl iteratorDecl) {
}
}

protected virtual void VisitMember(TopLevelDeclWithMembers cl, MemberDecl member) {
if (member is ConstantField constantField) {
var context = GetContext(constantField, false);
VisitAttributes(constantField, constantField.EnclosingModule);
VisitUserProvidedType(constantField.Type, context);
if (constantField.Rhs != null) {
VisitExpression(constantField.Rhs, context);
}

} else if (member is Field field) {
var context = GetContext(new NoContext(cl.EnclosingModuleDefinition), false);
VisitAttributes(field, cl.EnclosingModuleDefinition);
VisitUserProvidedType(field.Type, context);
public void VisitMember(MemberDecl member) {
if (member is Field field) {
VisitField(field);

} else if (member is Function function) {
VisitFunction(function);
Expand Down Expand Up @@ -129,6 +119,18 @@ protected virtual void VisitMember(TopLevelDeclWithMembers cl, MemberDecl member
}
}

public virtual void VisitField(Field field) {
var enclosingModule = field.EnclosingClass.EnclosingModuleDefinition;
VisitAttributes(field, enclosingModule);

var context = GetContext(field as IASTVisitorContext ?? new NoContext(enclosingModule), false);
VisitUserProvidedType(field.Type, context);

if (field is ConstantField { Rhs: { } rhs }) {
VisitExpression(rhs, context);
}
}

public virtual void VisitFunction(Function function) {
var context = GetContext(function, false);

Expand Down Expand Up @@ -260,18 +262,26 @@ protected virtual void VisitExpression(Expression expr, VisitorContext context)

// Visit subexpressions
expr.SubExpressions.Iter(ee => VisitExpression(ee, context));

// Finish by calling the post-visitor method
Copy link
Member

@keyboardDrummer keyboardDrummer Dec 28, 2022

Choose a reason for hiding this comment

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

Comment seems redundant given the name PostVisitOneExpression

PostVisitOneExpression(expr, context);
}
}

/// <summary>
/// Visits the given expression.
/// Returns "true" to request that the caller keeps visiting all user-provided types, subexpressions, and substatements of "expr", and
/// returns "false" to tell the caller not to.
/// Returns "true" to request that the caller
/// - keeps visiting all user-provided types, subexpressions, and substatements of "expr", and
/// - then calls PostVisitOneExpression.
/// Returns "false" to tell the caller not to do those things.
/// </summary>
protected virtual bool VisitOneExpression(Expression expr, VisitorContext context) {
return true;
}

protected virtual void PostVisitOneExpression(Expression expr, VisitorContext context) {
}

protected virtual void VisitStatement(Statement stmt, VisitorContext context) {
if (VisitOneStatement(stmt, context)) {
// Visit user-provided types
Expand Down Expand Up @@ -317,17 +327,25 @@ protected virtual void VisitStatement(Statement stmt, VisitorContext context) {

// Visit substatements
stmt.SubStatements.Iter(ss => VisitStatement(ss, context));

// Finish by calling the post-visitor method
Copy link
Member

Choose a reason for hiding this comment

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

Comment seems redundant given the name PostVisitOneExpression

PostVisitOneStatement(stmt, context);
}
}

/// <summary>
/// Visits the given statement.
/// Returns "true" to request that the caller keeps visiting all user-provided types, subexpressions, and substatements of "stmt", and
/// returns "false" to tell the caller not to.
/// Returns "true" to request that the caller
/// - keeps visiting all user-provided types, subexpressions, and substatements of "stmt", and
/// - then calls PostVisitOneStatement.
/// Returns "false" to tell the caller not to do those things.
/// </summary>
protected virtual bool VisitOneStatement(Statement stmt, VisitorContext context) {
return true;
}

protected virtual void PostVisitOneStatement(Statement stmt, VisitorContext context) {
}
}

}
21 changes: 10 additions & 11 deletions Source/DafnyCore/AST/DafnyAst.cs
Original file line number Diff line number Diff line change
Expand Up @@ -345,22 +345,21 @@ public static bool Contains(Attributes attrs, string nm) {
/// - if the attribute is {:nm true}, then value==true
/// - if the attribute is {:nm false}, then value==false
/// - if the attribute is anything else, then value returns as whatever it was passed in as.
/// This method does NOT use type information of the attribute arguments, so it can safely
/// be called very early during resolution before types are available and names have been resolved.
/// </summary>
[Pure]
public static bool ContainsBool(Attributes attrs, string nm, ref bool value) {
Contract.Requires(nm != null);
foreach (var attr in attrs.AsEnumerable()) {
if (attr.Name == nm) {
if (attr.Args.Count == 1) {
var arg = attr.Args[0] as LiteralExpr;
if (arg != null && arg.Value is bool) {
value = (bool)arg.Value;
}
}
return true;
}
var attr = attrs.AsEnumerable().FirstOrDefault(attr => attr.Name == nm);
if (attr == null) {
return false;
}
return false;

if (attr.Args.Count == 1 && attr.Args[0] is LiteralExpr { Value: bool v }) {
value = v;
}
return true;
}

/// <summary>
Expand Down
1 change: 1 addition & 0 deletions Source/DafnyCore/AST/Printer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,7 @@ public void PrintTopLevelDecls(List<TopLevelDecl> decls, int indent, List<IToken
}
}
if (dd.Members.Count != 0) {
Indent(indent);
wr.WriteLine("{");
PrintMembers(dd.Members, indent + IndentAmount, fileBeingPrinted);
Indent(indent);
Expand Down
27 changes: 12 additions & 15 deletions Source/DafnyCore/ExpressionTester.cs
Original file line number Diff line number Diff line change
Expand Up @@ -230,33 +230,30 @@ private bool CheckIsCompilable(Expression expr, ICodeContext codeContext) {
return isCompilable;
} else if (expr is LambdaExpr lambdaExpr) {
return CheckIsCompilable(lambdaExpr.Body, codeContext);
} else if (expr is ComprehensionExpr comprehensionExpr) {
var uncompilableBoundVars = comprehensionExpr.UncompilableBoundVars();
} else if (expr is ComprehensionExpr) {
var e = (ComprehensionExpr)expr;
Copy link
Member

Choose a reason for hiding this comment

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

What's the advantage of the pattern

else if (expr is ComprehensionExpr) {
  var e = (ComprehensionExpr)expr;

over

if (expr is ComprehensionExpr comprehensionExpr) 

?

var uncompilableBoundVars = e.UncompilableBoundVars();
if (uncompilableBoundVars.Count != 0) {
if (ReportErrors == false) {
return false;
}

string what;
if (comprehensionExpr is SetComprehension comprehension) {
if (e is SetComprehension comprehension) {
what = comprehension.Finite ? "set comprehensions" : "iset comprehensions";
} else if (comprehensionExpr is MapComprehension mapComprehension) {
} else if (e is MapComprehension mapComprehension) {
what = mapComprehension.Finite ? "map comprehensions" : "imap comprehensions";
} else {
Contract.Assume(comprehensionExpr is QuantifierExpr); // otherwise, unexpected ComprehensionExpr (since LambdaExpr is handled separately above)
Contract.Assert(((QuantifierExpr)comprehensionExpr).SplitQuantifier == null); // No split quantifiers during resolution
Contract.Assume(e is QuantifierExpr); // otherwise, unexpected ComprehensionExpr (since LambdaExpr is handled separately above)
Contract.Assert(((QuantifierExpr)e).SplitQuantifier == null); // No split quantifiers during resolution
what = "quantifiers";
}
foreach (var bv in uncompilableBoundVars) {
reporter.Error(MessageSource.Resolver, comprehensionExpr, "{0} in non-ghost contexts must be compilable, but Dafny's heuristics can't figure out how to produce or compile a bounded set of values for '{1}'", what, bv.Name);
reporter?.Error(MessageSource.Resolver, e, "{0} in non-ghost contexts must be compilable, but Dafny's heuristics can't figure out how to produce or compile a bounded set of values for '{1}'", what, bv.Name);
isCompilable = false;
}
return false;
}
// don't recurse down any attributes
if (comprehensionExpr.Range != null) {
isCompilable = CheckIsCompilable(comprehensionExpr.Range, codeContext) && isCompilable;
if (e.Range != null) {
isCompilable = CheckIsCompilable(e.Range, codeContext) && isCompilable;
}
isCompilable = CheckIsCompilable(comprehensionExpr.Term, codeContext) && isCompilable;
isCompilable = CheckIsCompilable(e.Term, codeContext) && isCompilable;
return isCompilable;

} else if (expr is ChainingExpression chainingExpression) {
Expand Down
85 changes: 85 additions & 0 deletions Source/DafnyCore/Resolver/BoundsDiscovery.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,91 @@

namespace Microsoft.Dafny {
public partial class Resolver {
private class BoundsDiscoveryVisitor : ASTVisitor<IASTVisitorContext> {
private readonly ErrorReporter reporter;

public BoundsDiscoveryVisitor(ErrorReporter reporter) {
this.reporter = reporter;
}

public override IASTVisitorContext GetContext(IASTVisitorContext astVisitorContext, bool inFunctionPostcondition) {
return astVisitorContext;
}

protected override bool VisitOneStatement(Statement stmt, IASTVisitorContext context) {
if (stmt is ForallStmt forallStmt) {
forallStmt.Bounds = DiscoverBestBounds_MultipleVars(forallStmt.BoundVars, forallStmt.Range, true,
ComprehensionExpr.BoundedPool.PoolVirtues.Enumerable);
} else if (stmt is AssignSuchThatStmt assignSuchThatStmt) {
if (assignSuchThatStmt.AssumeToken == null) {
var varLhss = new List<IVariable>();
foreach (var lhs in assignSuchThatStmt.Lhss) {
var ide = (IdentifierExpr)lhs.Resolved; // successful resolution implies all LHS's are IdentifierExpr's
varLhss.Add(ide.Var);
}
assignSuchThatStmt.Bounds = DiscoverBestBounds_MultipleVars(varLhss, assignSuchThatStmt.Expr, true,
ComprehensionExpr.BoundedPool.PoolVirtues.None);
}
}

return base.VisitOneStatement(stmt, context);
}

protected override bool VisitOneExpression(Expression expr, IASTVisitorContext context) {
if (expr is ComprehensionExpr) {
var e = (ComprehensionExpr)expr;
// apply bounds discovery to quantifiers, finite sets, and finite maps
string what = e.WhatKind;
Expression whereToLookForBounds = null;
var polarity = true;
if (e is QuantifierExpr quantifierExpr) {
whereToLookForBounds = quantifierExpr.LogicalBody();
polarity = quantifierExpr is ExistsExpr;
} else if (e is SetComprehension setComprehension) {
whereToLookForBounds = setComprehension.Range;
} else if (e is MapComprehension) {
whereToLookForBounds = e.Range;
} else {
Contract.Assume(e is LambdaExpr); // otherwise, unexpected ComprehensionExpr
}
if (whereToLookForBounds != null) {
e.Bounds = DiscoverBestBounds_MultipleVars_AllowReordering(e.BoundVars, whereToLookForBounds, polarity, ComprehensionExpr.BoundedPool.PoolVirtues.None);
if (2 <= DafnyOptions.O.Allocated && (context is Function or ConstantField or RedirectingTypeDecl)) {
// functions are not allowed to depend on the set of allocated objects
foreach (var bv in ComprehensionExpr.BoundedPool.MissingBounds(e.BoundVars, e.Bounds, ComprehensionExpr.BoundedPool.PoolVirtues.IndependentOfAlloc)) {
var msgFormat = "a {0} involved in a {3} definition is not allowed to depend on the set of allocated references, but values of '{1}' may contain references";
if (bv.Type.IsTypeParameter || bv.Type.IsOpaqueType) {
msgFormat += " (perhaps declare its type, '{2}', as '{2}(!new)')";
}
msgFormat += " (see documentation for 'older' parameters)";
var declKind = context is RedirectingTypeDecl redir ? redir.WhatKind : ((MemberDecl)context).WhatKind;
reporter.Error(MessageSource.Resolver, e, msgFormat, e.WhatKind, bv.Name, bv.Type, declKind);
}
}

if ((e as SetComprehension)?.Finite == true || (e as MapComprehension)?.Finite == true) {
// the comprehension had better produce a finite set
if (e.Type.HasFinitePossibleValues) {
// This means the set is finite, regardless of if the Range is bounded. So, we don't give any error here.
// However, if this expression is used in a non-ghost context (which is not yet known at this stage of
// resolution), the resolver will generate an error about that later.
} else {
// we cannot be sure that the set/map really is finite
foreach (var bv in ComprehensionExpr.BoundedPool.MissingBounds(e.BoundVars, e.Bounds, ComprehensionExpr.BoundedPool.PoolVirtues.Finite)) {
reporter.Error(MessageSource.Resolver, e,
"the result of a {0} must be finite, but Dafny's heuristics can't figure out how to produce a bounded set of values for '{1}'",
e.WhatKind, bv.Name);
}
}
}
}

}

return base.VisitOneExpression(expr, context);
}
}

/// <summary>
/// For a list of variables "bvars", returns a list of best bounds, subject to the constraint "requiredVirtues", for each respective variable.
/// If no bound matching "requiredVirtues" is found for a variable "v", then the bound for "v" in the returned list is set to "null".
Expand Down
Loading