-
Notifications
You must be signed in to change notification settings - Fork 114
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
Implement verification coverage analysis #741
Changes from all commits
49d5d23
0ac0824
32398ee
5c4afaf
e945230
ac56302
a97032b
5919289
8ad7c16
e95aab5
3ce4244
950080b
7b6679b
b67d227
20e06a0
abcae74
6b48078
d1861c0
a07fe28
82e7e24
98dcf9f
d68b1f9
6d48d32
e43e4f2
94cf1ea
449364c
725fe2d
cd50eef
70cd55d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -280,6 +280,33 @@ public List<int> FindLayers() | |
} | ||
return layers.Distinct().OrderBy(l => l).ToList(); | ||
} | ||
|
||
// Look for {:name string} in list of attributes. | ||
public string FindStringAttribute(string name) | ||
{ | ||
return QKeyValue.FindStringAttribute(Attributes, name); | ||
} | ||
|
||
public void AddStringAttribute(IToken tok, string name, string parameter) | ||
{ | ||
Attributes = new QKeyValue(tok, name, new List<object>() {parameter}, Attributes); | ||
} | ||
|
||
public void CopyIdFrom(IToken tok, ICarriesAttributes src) | ||
{ | ||
var id = src.FindStringAttribute("id"); | ||
if (id is not null) { | ||
AddStringAttribute(tok, "id", id); | ||
} | ||
} | ||
|
||
public void CopyIdWithSuffixFrom(IToken tok, ICarriesAttributes src, string suffix) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
{ | ||
var id = src.FindStringAttribute("id"); | ||
if (id is not null) { | ||
AddStringAttribute(tok, "id", id + suffix); | ||
} | ||
} | ||
} | ||
|
||
[ContractClassFor(typeof(Absy))] | ||
|
@@ -410,13 +437,6 @@ public Expr FindExprAttribute(string name) | |
return res; | ||
} | ||
|
||
// Look for {:name string} in list of attributes. | ||
public string FindStringAttribute(string name) | ||
{ | ||
Contract.Requires(name != null); | ||
return QKeyValue.FindStringAttribute(this.Attributes, name); | ||
} | ||
|
||
// Look for {:name N} in list of attributes. Return false if attribute | ||
// 'name' does not exist or if N is not an integer constant. Otherwise, | ||
// return true and update 'result' with N. | ||
|
@@ -1810,7 +1830,7 @@ public byte[] MD5DependencyChecksum | |
|
||
public string Checksum | ||
{ | ||
get { return FindStringAttribute("checksum"); } | ||
get { return (this as ICarriesAttributes).FindStringAttribute("checksum"); } | ||
} | ||
|
||
string dependencyChecksum; | ||
|
@@ -3616,7 +3636,7 @@ public string Id | |
{ | ||
get | ||
{ | ||
var id = FindStringAttribute("id"); | ||
var id = (this as ICarriesAttributes).FindStringAttribute("id"); | ||
if (id == null) | ||
{ | ||
id = Name + GetHashCode().ToString() + ":0"; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3723,6 +3723,7 @@ protected override Cmd ComputeDesugaring(PrintOptions options) | |
Dictionary<Variable, Expr> substMapBound = new Dictionary<Variable, Expr>(); | ||
List<Variable> /*!*/ | ||
tempVars = new List<Variable>(); | ||
string callId = (this as ICarriesAttributes).FindStringAttribute("id"); | ||
|
||
// proc P(ins) returns (outs) | ||
// requires Pre | ||
|
@@ -3841,6 +3842,7 @@ protected override Cmd ComputeDesugaring(PrintOptions options) | |
AssertCmd /*!*/ | ||
a = new AssertRequiresCmd(this, reqCopy); | ||
Contract.Assert(a != null); | ||
|
||
if (Attributes != null) | ||
{ | ||
// Inherit attributes of call. | ||
|
@@ -3849,6 +3851,11 @@ protected override Cmd ComputeDesugaring(PrintOptions options) | |
a.Attributes = attrCopy; | ||
} | ||
|
||
// Do this after copying the attributes so it doesn't get overwritten | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The method being changed is very large, and uses regions instead of submethods to organize code. I want to ask you apply the boyscout principle of "leaving things behind nicer than they were", and to chop this method up into pieces, and also to move the class There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created #743 to track this, as I think we need to do it very carefully. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Why? Splitting a method up into pieces can be done in a mostly automated way using Rider. Same for moving a class into a separate file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, but reorganizing those methods so that they are well-organized and use better abstractions, rather than just slicing them into pieces, would require more thought. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm paraphrasing an offline discussion I had with @atomb on the above, for the sake of getting some additional eyes on it. @atomb prefers keeping the pacify implementations for various commands in a single method, since that allows having them be vertically adjacent to each other which follows the structure of Figure 3 in this paper, which is what the code is based on. Keeping the structure of the two most similar provides the best way of doing a side-by-side comparison. I would prefer keeping the code for different commands separate. I would even prefer having an abstract I believe that while the paper is structured by putting the concepts at the top-level, and then discussing all commands within each concept, this doesn't mean that's also the right structure for the code. It's easier to discuss just one pair of concept and command at a time, and while the paper doesn't have room to go that slow, in our codebase we can use as many separators, such as files and methods, as we like. Also, the code is more verbose so there is more need to keep the scope of each block to a minimum. I don't think optimizing for comparability between the code and the paper is important. Modern IDEs make it easy to navigate between the code for various commands, so I don't think there's a particular difficulty in making the comparison in any case. |
||
if (callId is not null) { | ||
(a as ICarriesAttributes).CopyIdWithSuffixFrom(tok, req, $"${callId}$requires"); | ||
} | ||
|
||
a.ErrorDataEnhanced = reqCopy.ErrorDataEnhanced; | ||
newBlockBody.Add(a); | ||
} | ||
|
@@ -3860,6 +3867,11 @@ protected override Cmd ComputeDesugaring(PrintOptions options) | |
AssumeCmd /*!*/ | ||
a = new AssumeCmd(req.tok, Substituter.Apply(s, req.Condition)); | ||
Contract.Assert(a != null); | ||
// These probably won't have IDs, but copy if they do. | ||
if (callId is not null) { | ||
(a as ICarriesAttributes).CopyIdWithSuffixFrom(tok, req, $"${callId}$requires_assumed"); | ||
} | ||
|
||
newBlockBody.Add(a); | ||
} | ||
} | ||
|
@@ -4022,6 +4034,10 @@ protected override Cmd ComputeDesugaring(PrintOptions options) | |
|
||
#endregion | ||
|
||
if (callId is not null) { | ||
(assume as ICarriesAttributes).CopyIdWithSuffixFrom(tok, e, $"${callId}$ensures"); | ||
} | ||
|
||
newBlockBody.Add(assume); | ||
} | ||
|
||
|
@@ -4039,6 +4055,9 @@ protected override Cmd ComputeDesugaring(PrintOptions options) | |
cout_exp = new IdentifierExpr(cce.NonNull(couts[i]).tok, cce.NonNull(couts[i])); | ||
Contract.Assert(cout_exp != null); | ||
AssignCmd assign = Cmd.SimpleAssign(param_i.tok, cce.NonNull(this.Outs[i]), cout_exp); | ||
if (callId is not null) { | ||
Attributes = new QKeyValue(param_i.tok, "id", new List<object>(){ $"{callId}$out{i}" }, Attributes); | ||
} | ||
newBlockBody.Add(assign); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
<ProjectReference Include="..\Houdini\Houdini.csproj" /> | ||
<ProjectReference Include="..\Model\Model.csproj" /> | ||
<ProjectReference Include="..\VCGeneration\VCGeneration.csproj" /> | ||
<ProjectReference Include="..\Provers\SMTLib\SMTLib.csproj" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this can be removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The build of Boogie itself works without it, but I discovered it was necessary for Dafny (and presumably other Boogie clients?) to build against a Boogie source tree instead of a pre-built package. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, the alternative is that the Boogie clients such as Dafny also depend on SMTLib, but this seems like a nicer UX. |
||
</ItemGroup> | ||
|
||
</Project> |
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.
If you define these methods as extension methods, then you can also call them without casting to
ICarriesAttributes
first.