-
Notifications
You must be signed in to change notification settings - Fork 269
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
Add tooltip on assertions that shows which functions are hidden #5929
Merged
keyboardDrummer
merged 17 commits into
dafny-lang:master
from
keyboardDrummer:whatIsHiddenHint
Nov 28, 2024
Merged
Changes from 2 commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
2a92810
Use custom Boogie
keyboardDrummer ea4cbb0
Add tooltip on assertions for what is hidden when verifying
keyboardDrummer 8dedb49
Add tests and a fix
keyboardDrummer a6e2f8f
Merge branch 'master' into whatIsHiddenHint
keyboardDrummer 3bf7ab4
Improvements
keyboardDrummer 6293a27
Revert bad change
keyboardDrummer 2b63912
Fix test
keyboardDrummer f68416e
Code review
keyboardDrummer b2daf87
Remove Boogie submodule
keyboardDrummer 45a6b9d
Update Boogie version
keyboardDrummer 9e0feff
Merge branch 'master' into whatIsHiddenHint
keyboardDrummer ea82f96
Merge branch 'whatIsHiddenHint' of github.com:keyboardDrummer/dafny i…
keyboardDrummer 6e0dbfc
Submodule attempt
keyboardDrummer 6f2abe4
Trigger CI
keyboardDrummer eecb794
Merge branch 'master' into whatIsHiddenHint
keyboardDrummer 5c074df
Trigger CI
keyboardDrummer d8eb6cd
Merge branch 'whatIsHiddenHint' of github.com:keyboardDrummer/dafny i…
keyboardDrummer File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
[submodule "boogie"] | ||
path = boogie | ||
url = [email protected]:keyboardDrummer/boogie.git |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -356,15 +356,27 @@ private async Task VerifyUnverifiedSymbol(bool onlyPrepareVerificationForGutterT | |||
return result; | ||||
}); | ||||
if (updated || randomSeed != null) { | ||||
updates.OnNext(new CanVerifyPartsIdentified(canVerify, | ||||
tasksPerVerifiable[canVerify].ToList())); | ||||
updates.OnNext(new CanVerifyPartsIdentified(canVerify, tasks)); | ||||
} | ||||
|
||||
// When multiple calls to VerifyUnverifiedSymbol are made, the order in which they pass this await matches the call order. | ||||
await ticket; | ||||
|
||||
if (!onlyPrepareVerificationForGutterTests) { | ||||
foreach (var tokenTasks in tasks.GroupBy(t => | ||||
BoogieGenerator.ToDafnyToken(true, t.Token)). | ||||
OrderBy(g => g.Key)) { | ||||
var functions = tokenTasks.SelectMany(t => t.Split.HiddenFunctions.Select(f => f.tok). | ||||
OfType<FromDafnyNode>().Select(n => n.Node). | ||||
OfType<Function>()).Distinct(); | ||||
var hiddenFunctions = string.Join(", ", functions.Select(f => f.FullDafnyName)); | ||||
if (!string.IsNullOrEmpty(hiddenFunctions)) { | ||||
Reporter.Info(MessageSource.Verifier, tokenTasks.Key, $"hidden functions: {hiddenFunctions}"); | ||||
} | ||||
} | ||||
|
||||
foreach (var task in tasks.Where(taskFilter)) { | ||||
|
||||
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.
Suggested change
|
||||
var seededTask = randomSeed == null ? task : task.FromSeed(randomSeed.Value); | ||||
VerifyTask(canVerify, seededTask); | ||||
} | ||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
17 changes: 16 additions & 1 deletion
17
Source/IntegrationTests/TestFiles/LitTests/LitTest/ast/reveal/revealFunctions.dfy.expect
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,20 @@ | ||
revealFunctions.dfy(90,19): Info: auto-accumulator tail recursive | ||
revealFunctions.dfy(90,19): Info: decreases x | ||
revealFunctions.dfy(15,12): Info: hidden functions: P | ||
revealFunctions.dfy(22,12): Info: hidden functions: P | ||
revealFunctions.dfy(49,21): Info: hidden functions: OpaqueFunc | ||
revealFunctions.dfy(57,11): Info: hidden functions: Natty | ||
revealFunctions.dfy(60,11): Info: hidden functions: Natty | ||
revealFunctions.dfy(79,4): Info: hidden functions: Obj | ||
revealFunctions.dfy(81,18): Info: hidden functions: Obj | ||
revealFunctions.dfy(117,14): Info: hidden functions: Outer | ||
revealFunctions.dfy(118,14): Info: hidden functions: Inner | ||
revealFunctions.dfy(121,14): Info: hidden functions: Outer | ||
revealFunctions.dfy(15,12): Error: assertion might not hold | ||
revealFunctions.dfy(22,12): Error: assertion might not hold | ||
revealFunctions.dfy(49,21): Error: assertion might not hold | ||
revealFunctions.dfy(117,14): Error: assertion might not hold | ||
revealFunctions.dfy(118,14): Error: assertion might not hold | ||
revealFunctions.dfy(121,14): Error: assertion might not hold | ||
|
||
Dafny program verifier finished with 22 verified, 3 errors | ||
Dafny program verifier finished with 23 verified, 6 errors |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Wrapping tokens to filter functions is not ideal. There are multi competing token wrapping strategies that will end up failing this test in the future. For example, refinement tokens will wrap your FromDafnyNode token making them not pass this test. There are other wrapped tokens types, like auto-generated tokens, nested tokens, override tokens for traits, etc.
I have always advocated for replacing tokens with a single meta-data field on which we would add some flags as needed.
I'm not expecting you to do this refactoring, so in the meantime, please use something like
FromDafnyNode.Has()
that would recursively traverse the tokens to find one of its kind.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.
Here the wrapping happens right before handing the token to Boogie, so Dafny won't do any further wrapper, and Boogie might do some wrapping on tokens but only use that for nodes that it creates, so not on the nodes that I'm interested in. I don't think there's a potential issue here. If there is one, how can we test for it?
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.
Ok, good to know that the risk is mitigated. I think that we still have the risk that later, someone will wrap your token after your own wrapping and their test cases will be orthogonal to yours, so both won't be tested together.
If you want to make sure your wrapping is the last one before sending the token to boogie, I'd suggest: changing the name to something like
DafnyMetaData
so that future development will become compelled to add metadata here rather than adding a new wrapping token.