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

Introduce aggressive pruning and the related 'uses' keyword for constants and functions #450

Merged
Show file tree
Hide file tree
Changes from all commits
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
50 changes: 18 additions & 32 deletions Source/Core/Absy.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1779,6 +1779,11 @@ public override void Emit(TokenTextWriter stream, int level)

public class Axiom : Declaration
{
public override string ToString()
{
return "Axiom: " + expression.ToString();
}

private Expr /*!*/
expression;

Expand Down Expand Up @@ -2472,6 +2477,8 @@ public class Constant : Variable
// that the parental situation is unconstrained.
public readonly ReadOnlyCollection<ConstantParent /*!*/> Parents;

public IEnumerable<Axiom> DefinitionAxioms { get; }

[ContractInvariantMethod]
void ObjectInvariant()
{
Expand All @@ -2483,33 +2490,20 @@ void ObjectInvariant()
public readonly bool ChildrenComplete;

public Constant(IToken /*!*/ tok, TypedIdent /*!*/ typedIdent)
: base(tok, typedIdent)
: this(tok, typedIdent, true)
{
Contract.Requires(tok != null);
Contract.Requires(typedIdent != null);
Contract.Requires(typedIdent.Name != null && (!typedIdent.HasName || typedIdent.Name.Length > 0));
Contract.Requires(typedIdent.WhereExpr == null);
this.Unique = true;
this.Parents = null;
this.ChildrenComplete = false;
}

public Constant(IToken /*!*/ tok, TypedIdent /*!*/ typedIdent, bool unique)
: base(tok, typedIdent)
: this(tok, typedIdent, unique, null, false, null, new List<Axiom>())
{
Contract.Requires(tok != null);
Contract.Requires(typedIdent != null);
Contract.Requires(typedIdent.Name != null && typedIdent.Name.Length > 0);
Contract.Requires(typedIdent.WhereExpr == null);
this.Unique = unique;
this.Parents = null;
this.ChildrenComplete = false;
}

public Constant(IToken /*!*/ tok, TypedIdent /*!*/ typedIdent,
bool unique,
IEnumerable<ConstantParent /*!*/> parents, bool childrenComplete,
QKeyValue kv)
IEnumerable<ConstantParent /*!*/> parents = null, bool childrenComplete = false,
QKeyValue kv = null,
IEnumerable<Axiom> definitionAxioms = null)
: base(tok, typedIdent, kv)
{
Contract.Requires(tok != null);
Expand All @@ -2520,12 +2514,10 @@ public Constant(IToken /*!*/ tok, TypedIdent /*!*/ typedIdent,
this.Unique = unique;
this.Parents = parents == null ? null : new ReadOnlyCollection<ConstantParent>(parents.ToList());
this.ChildrenComplete = childrenComplete;
this.DefinitionAxioms = definitionAxioms ?? Enumerable.Empty<Axiom>();
}

public override bool IsMutable
{
get { return false; }
}
public override bool IsMutable => false;

public override void Emit(TokenTextWriter stream, int level)
{
Expand Down Expand Up @@ -3301,22 +3293,16 @@ public class Function : DeclWithFormals
public NAryExpr DefinitionBody; // Only set if the function is declared with {:define}
public Axiom DefinitionAxiom;

public IList<Axiom> otherDefinitionAxioms;
public IList<Axiom> otherDefinitionAxioms = new List<Axiom>();
public IEnumerable<Axiom> DefinitionAxioms =>
(DefinitionAxiom == null ? Enumerable.Empty<Axiom>() : new[]{ DefinitionAxiom }).Concat(otherDefinitionAxioms);

public IEnumerable<Axiom> OtherDefinitionAxioms
{
get { return otherDefinitionAxioms; }
}
public IEnumerable<Axiom> OtherDefinitionAxioms => otherDefinitionAxioms;

public void AddOtherDefinitionAxiom(Axiom axiom)
{
Contract.Requires(axiom != null);

if (otherDefinitionAxioms == null)
{
otherDefinitionAxioms = new List<Axiom>();
}

otherDefinitionAxioms.Add(axiom);
}

Expand Down
2 changes: 1 addition & 1 deletion Source/Core/AbsyQuant.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public List<Variable> /*!*/

public Expr /*!*/ Body
{
get { return _Body; }
get => _Body;
set
{
if (Immutable)
Expand Down
32 changes: 24 additions & 8 deletions Source/Core/BoogiePL.atg
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ BoogiePL
Implementation im;
Implementation/*!*/ nnim;
.)
{ Consts<out vs> (. foreach(Bpl.Variable/*!*/ v in vs){
{ Consts<out ds> (. foreach(Bpl.Declaration/*!*/ v in ds){
Contract.Assert(v != null);
Pgm.AddTopLevelDeclaration(v);
}
Expand Down Expand Up @@ -395,9 +395,11 @@ Types<.List<Bpl.Type>/*!*/ ts.>


/*------------------------------------------------------------------------*/
Consts<.out List<Variable>/*!*/ ds.>
Consts<.out List<Declaration>/*!*/ ds.>
= (. Contract.Ensures(Contract.ValueAtReturn(out ds) != null); IToken/*!*/ y; List<TypedIdent>/*!*/ xs;
ds = new List<Variable>();
ds = new List<Declaration>();
var axioms = new List<Axiom>();
Axiom ax;
bool u = false; QKeyValue kv = null;
bool ChildrenComplete = false;
List<ConstantParent/*!*/> Parents = null; .)
Expand All @@ -407,6 +409,7 @@ Consts<.out List<Variable>/*!*/ ds.>
]
IdsType<out xs>
[ OrderSpec<out ChildrenComplete, out Parents> ]
( ";" | "uses" "{" { Axiom<out ax>(. axioms.Add(ax); .) } "}")
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes uses a reserved keyword in Boogie. Any previous program that used uses as an identifier will no longer parse. I think this is okay, but we should make sure to bump the Boogie version accordingly (to a new minor number, probably).

If there's concern about making uses a keyword, an alternative would be to add it as a contextual keyword. That's a little tricky here, because

function F(x: int): MyType uses {
  x + 5
}

could mean one thing with

type MyType = int;

and something different with

type MyType T = int;
type uses;

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 bumped the minor version number

(. bool makeClone = false;
foreach(TypedIdent/*!*/ x in xs){
Contract.Assert(x != null);
Expand All @@ -424,11 +427,14 @@ Consts<.out List<Variable>/*!*/ ds.>
ParentsClone = Parents;
}
makeClone = true;

ds.Add(new Constant(y, x, u, ParentsClone, ChildrenComplete, kv));

var constant = new Constant(y, x, u, ParentsClone, ChildrenComplete, kv, axioms);
ds.Add(constant);
}
foreach(var axiom in axioms) {
ds.Add(axiom);
}
.)
";"
.

OrderSpec<.out bool ChildrenComplete, out List<ConstantParent/*!*/> Parents.>
Expand Down Expand Up @@ -462,13 +468,15 @@ Function<.out List<Declaration>/*!*/ ds.>
IToken/*!*/ typeParamTok;
var typeParams = new List<TypeVariable>();
var arguments = new List<Variable>();
var axioms = new List<Axiom>();
TypedIdent/*!*/ tyd;
TypedIdent retTyd = null;
Bpl.Type/*!*/ retTy;
QKeyValue argKv = null;
QKeyValue kv = null;
Expr definition = null;
Expr/*!*/ tmp;
Axiom ax;
.)
"function" { Attribute<ref kv> } Ident<out z>
[ TypeParams<out typeParamTok, out typeParams> ]
Expand All @@ -482,18 +490,26 @@ Function<.out List<Declaration>/*!*/ ds.>
|
":" Type<out retTy> (. retTyd = new TypedIdent(retTy.tok, TypedIdent.NoName, retTy); .)
)
( "{" Expression<out tmp> (. definition = tmp; .) "}" | ";" )
( "{" Expression<out tmp> (. definition = tmp; .) "}" [ "uses" "{" { Axiom<out ax>(. axioms.Add(ax); .) } "}" ]
| "uses" "{" { Axiom<out ax>(. axioms.Add(ax); .) } "}"
| ";"
)
(.
if (retTyd == null) {
// construct a dummy type for the case of syntax error
retTyd = new TypedIdent(t, TypedIdent.NoName, new BasicType(t, SimpleType.Int));
}
Function/*!*/ func = new Function(z, z.val, typeParams, arguments,
new Formal(retTyd.tok, retTyd, false, argKv), null, kv);
foreach(var axiom in axioms) {
ds.Add(axiom);
func.AddOtherDefinitionAxiom(axiom);
}

Contract.Assert(func != null);
ds.Add(func);
bool allUnnamed = true;
foreach(Formal/*!*/ f in arguments){
foreach(Formal/*!*/ f in arguments) {
Contract.Assert(f != null);
if (f.TypedIdent.HasName) {
allUnnamed = false;
Expand Down
93 changes: 85 additions & 8 deletions Source/Core/CommandLineOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -523,10 +523,7 @@ protected CommandLineOptions(string toolName, string descriptiveName)

private static CommandLineOptions clo;

public static CommandLineOptions /*!*/ Clo
{
get { return clo; }
}
public static CommandLineOptions /*!*/ Clo => clo;

public static void Install(CommandLineOptions options)
{
Expand Down Expand Up @@ -592,7 +589,80 @@ public bool PrintInstrumented {
public bool InstrumentWithAsserts = false;
public string ProverPreamble { get; set; }= null;
public bool WarnNotEliminatedVars = false;
public bool PruneFunctionsAndAxioms = false;

public enum PruneMode {
None,
Copy link
Contributor

Choose a reason for hiding this comment

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

The discussion about automatic pruning assumes that the reader already understands the basic algorithm for adding edges among various declarations. Is that description already present in some documentation? If not, please add it here. The description should mention the declarations among which edges are present and for each edge a short intuitive description of what that edge means.

Copy link
Collaborator Author

@keyboardDrummer keyboardDrummer Nov 22, 2021

Choose a reason for hiding this comment

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

I don't fully understand the automatic pruning option, so I can't easily provide the detailed description you ask for. I think the summary is that any reference between declarations, except for those in quantifiers with triggers, is considered bidirectional.

I would be okay with removing automatic pruning once Dafny uses UsesClauses pruning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, removing Automatic seems desirable since currently there are multiple overlapping ways of doing the same thing. Would removing Automatic also mean that :exclude_dep will be dropped?

@RustanLeino : Any objection to dropping Automatic once Dafny uses UsesClauses pruning. What is the timeline on that move?

Copy link
Collaborator Author

@keyboardDrummer keyboardDrummer Nov 22, 2021

Choose a reason for hiding this comment

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

Would removing Automatic also mean that :exclude_dep will be dropped?

Yes

What is the timeline on that move?

I'm working on it. About a month.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no objection to dropping the Automatic mode and the :exclude_dep attribute. Instead, we would only support the new uses clauses.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@shazqadeer can we merge this? I don't have a strong opinion on the name of the UsesClauses enum. I think the difference with Automatic pruning, is that it only automatically detects outgoing edges in axioms, so something like OnlyUseOutgoingAxiomEdges could be the name.


/**
* Automatic pruning will remove any declarations that do not reference,
* directly or indirectly, and are not referenced by, the implementation being verified,
* and declarations which cannot be used for verifying the current implementation,
* such as axioms with triggers that can not be satisfied.
*
* Automatic pruning detects incoming edges in axioms, for example:
*
* function A(int): int;
* axiom A(3) == 2;
*
* Will detect edges in both directions between the declaration of A and the axiom declaration.
* So if either declaration is live, both declarations are live.
*/
Automatic,

/**
* UsesDecls pruning will not automatically detect incoming edges in axioms.
* Instead it depends on uses clauses in the input program to determine the outgoing edges of functions and constants.
* Outgoing edges in axioms are still detected automatically.
*
* The reason to use UsesDecls over Automatic pruning is that the latter can miss pruning opportunities.
* Consider the following program:
*
* ```
* function F(int): int;
* function G(int): int;
*
* // declaration axiom for F
* axiom forall x: int :: F(x) == x * 2
*
* // declaration axiom for G
* axiom forall x: int :: G(x) == F(x) + 1
*
* procedure FMultipliesByTwo(x: int)
* ensures F(x) - x == x
* { }
* ```
*
* Automatic pruning will not remove any declarations when verifying the procedure FMultipliesByTwo,
* since it refers to F, which refers to the declaration axiom of G through an incoming edge in the axiom,
* which also has an outgoing edge to G.
*
* If we rewrite the previous program to
*
* ```
* function F(int) returns (int) uses {
* axiom forall x: int :: F(x) == x * 2
* }
* function G(int) returns (int) uses {
* axiom forall x: int :: G(x) == F(x) + 1
* }
*
* procedure FMultipliesByTwo(x: int)
* ensures F(x) - x == x
* { }
* ```
*
* And apply UsesDecls pruning, then G and its axiom will be removed when verifying FMultipliesByTwo.
*
* An alternative to using UsesDecls pruning, is to add an {:exclude_dep} attribute to a function or constant,
* which prevents axioms from detecting incoming edges from that declaration.
* To add outgoing edges to the function or constant, uses clauses should be used.
*
* Using Automatic pruning in combination with {:exclude_dep} can be useful if this provides good enough pruning,
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you mean:

If Automatic pruning in combination with {:exclude_dep} does not enable good enough pruning, consider migrating from Automatic to UsesClauses pruning.

Copy link
Collaborator Author

@keyboardDrummer keyboardDrummer Nov 22, 2021

Choose a reason for hiding this comment

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

Not exactly. I've changed the text slightly:

Using Automatic pruning in combination with {:exclude_dep} can be useful if this provides good enough pruning, or to migrate from None to UsesClauses pruning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Using Automatic pruning in combination with {:exclude_dep} can be useful if this provides good enough pruning, or to migrate from None to UsesClauses pruning.

I still don't understand what this sentence means, specifically the part after the comma. Perhaps you are trying to communicate two separate thoughts in one sentence by reusing words. If you write down those two thoughts separately in two sentences, I might be able to understand better and suggest a coalescing.

Can the part before the comma not be written more compactly as:
Using Automatic pruning in combination with {:exclude_dep} may provide good enough pruning, ...

Copy link
Collaborator Author

@keyboardDrummer keyboardDrummer Nov 22, 2021

Choose a reason for hiding this comment

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

I'm trying to say that there's two use-cases for the Automatic pruning option:

  • As a pruning method, although this misses pruning opportunities.
  • As a way of migrating from None to UsesClauses pruning.
    Automatic pruning, in combination with an {:exclude_dep} attribute added to each function and constant, behaves the same as UsesClauses pruning. By using Automatic pruning and incrementally adding {:exclude_dep} attributes, one can migrate from None to UsesClauses pruning.
    For each added {:exclude_dep} attribute, one might have to add uses clauses to define the edges that are no longer detected automatically.

* or to migrate from None to UsesDecls pruning.
*/
UsesDecls
}
public PruneMode Prune = PruneMode.None;

public enum InstrumentationPlaces
{
Expand Down Expand Up @@ -1681,6 +1751,12 @@ protected override bool ParseOption(string name, CommandLineOptionEngine.Command
ps.GetNumericArgument(ref KInductionDepth);
return true;

case "prune":
int number = 0;
ps.GetNumericArgument(ref number);
Prune = (PruneMode)number;
return true;

case "emitDebugInformation":
ps.GetNumericArgument(ref emitDebugInformation);
return true;
Expand Down Expand Up @@ -1738,7 +1814,6 @@ protected override bool ParseOption(string name, CommandLineOptionEngine.Command
ps.CheckBooleanFlag("trustInductiveSequentialization", ref trustInductiveSequentialization) ||
ps.CheckBooleanFlag("useBaseNameForFileName", ref UseBaseNameForFileName) ||
ps.CheckBooleanFlag("freeVarLambdaLifting", ref FreeVarLambdaLifting) ||
ps.CheckBooleanFlag("pruneFunctionsAndAxioms", ref PruneFunctionsAndAxioms) ||
ps.CheckBooleanFlag("warnNotEliminatedVars", ref WarnNotEliminatedVars)
)
{
Expand Down Expand Up @@ -2323,8 +2398,10 @@ Use the SMT theory of arrays (as opposed to axioms). Supported
only for monomorphic programs.
/reflectAdd In the VC, generate an auxiliary symbol, elsewhere defined
to be +, instead of +.
/pruneFunctionsAndAxioms
Prune declarations for each implementation
/prune:<n>
0 (default) - none
1 - automatic pruning
2 - aggressive pruning. Requires binding axioms to functions and constants using 'uses'
/relaxFocus Process foci in a bottom-up fashion. This way only generates
a linear number of splits. The default way (top-down) is more
aggressive and it may create an exponential number of splits.
Expand Down
2 changes: 1 addition & 1 deletion Source/Core/LambdaHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ void ObjectInvariant()

string FreshLambdaFunctionName()
{
return string.Format("lambda#{0}", lambdaid++);
return $"lambda#{lambdaid++}";
}

public override Expr VisitLambdaExpr(LambdaExpr lambda)
Expand Down
Loading