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

fix: Termination override check for certain non-reference trait implementations #5087

Merged
merged 4 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
fix: Termination override check for certain non-reference trait imple…
…mentations
  • Loading branch information
fabiomadge committed Feb 14, 2024
commit 9ca52165a385b41e780160cb8dab003fc3ff7395
12 changes: 11 additions & 1 deletion Source/DafnyCore/Verifier/BoogieGenerator.Methods.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1461,6 +1461,9 @@ private void AddOverrideTerminationChk(ICallable original, ICallable overryd, Bo
// Note, it is as if the trait's method is calling the class's method.
var contextDecreases = overryd.Decreases.Expressions;
var calleeDecreases = original.Decreases.Expressions;
var T = (TraitDecl) (overryd is Function f1 ? f1.EnclosingClass : ((Method)overryd).EnclosingClass);
var I = (TopLevelDeclWithMembers) (original is Function f2 ? f2.EnclosingClass : ((Method)original).EnclosingClass);

// We want to check: calleeDecreases <= contextDecreases (note, we can allow equality, since there is a bounded, namely 1, number of dynamic dispatches)
if (Contract.Exists(contextDecreases, e => e is WildcardExpr)) {
// no check needed
Expand All @@ -1474,9 +1477,12 @@ private void AddOverrideTerminationChk(ICallable original, ICallable overryd, Bo
var callee = new List<Expr>();
var caller = new List<Expr>();

FunctionCallSubstituter sub = null;

for (int i = 0; i < N; i++) {
Expression e0 = calleeDecreases[i];
Expression e1 = Substitute(contextDecreases[i], null, substMap, typeMap);
sub ??= new FunctionCallSubstituter(substMap, typeMap, T, I);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You can leave this as is, but I don't think the extra lines here are worth the saving of creating sub only once and only if needed. If you agree, you could either always initialize sub before the loop (which is more costly only when N is 0) or create a new sub in each iteration (which is more costly only if N is 2 or more). I'd probably go for the latter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried inlining the construction of the substituter, but even I think it a little much for a single line. Without it, we're only saving a single line.

Expression e1 = sub.Substitute(contextDecreases[i]);
if (!CompatibleDecreasesTypes(e0.Type, e1.Type)) {
N = i;
break;
Expand All @@ -1486,6 +1492,10 @@ private void AddOverrideTerminationChk(ICallable original, ICallable overryd, Bo
types1.Add(e1.Type.NormalizeExpand());
callee.Add(etran.TrExpr(e0));
caller.Add(etran.TrExpr(e1));
var canCall = etran.CanCallAssumption(e1);
if (canCall != Bpl.Expr.True) {
builder.Add(new Bpl.AssumeCmd(e1.tok, canCall));
}
}

var decrCountT = contextDecreases.Count;
Expand Down
14 changes: 7 additions & 7 deletions Source/DafnyCore/Verifier/FunctionCallSubstituter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
namespace Microsoft.Dafny {
public class FunctionCallSubstituter : Substituter {
public readonly TraitDecl Tr;
public readonly TopLevelDeclWithMembers Cl;
public readonly TopLevelDeclWithMembers Impl;

// We replace all occurrences of the trait version of the function with the class version. This is only allowed if
// the receiver is `this`. We underapproximate this by looking for a `ThisExpr`, which misses more complex
// expressions that evaluate to one.
public FunctionCallSubstituter(Dictionary<IVariable, Expression /*!*/> /*!*/ substMap, Dictionary<TypeParameter, Type> typeMap,
TraitDecl parentTrait, TopLevelDeclWithMembers cl)
: base(new ThisExpr(cl.tok) { Type = UserDefinedType.FromTopLevelDecl(cl.tok, cl) }, substMap, typeMap) {
this.Tr = parentTrait;
this.Cl = cl;
TraitDecl parentTrait, TopLevelDeclWithMembers impl)
: base(new ThisExpr(impl.tok) { Type = UserDefinedType.FromTopLevelDecl(impl.tok, impl) }, substMap, typeMap) {
Tr = parentTrait;
Impl = impl;
}

public override Expression Substitute(Expression expr) {
Expand All @@ -24,10 +24,10 @@ public override Expression Substitute(Expression expr) {
Function function;
if ((e.Function.EnclosingClass == Tr || Tr.InheritedMembers.Contains(e.Function)) &&
e.Receiver.Resolved is ThisExpr && receiver.Resolved is ThisExpr &&
Cl.Members.Find(m => m.OverriddenMember == e.Function) is { } f) {
Impl.Members.Find(m => m.OverriddenMember == e.Function) is { } f) {
receiver = new ThisExpr((TopLevelDeclWithMembers)f.EnclosingClass);
function = (Function)f;
typeApplicationAtEnclosingClass = receiver.Type.AsParentType(Cl).TypeArgs.ToList();
typeApplicationAtEnclosingClass = receiver.Type.AsParentType(Impl).TypeArgs.ToList();
} else {
function = e.Function;
}
Expand Down
1 change: 1 addition & 0 deletions docs/dev/news/5087.fix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Termination override check for certain non-reference trait implementations