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

Add auditor support for the :concurrent attribute #3660

Merged
merged 7 commits into from
Mar 13, 2023

Conversation

atomb
Copy link
Member

@atomb atomb commented Mar 1, 2023

Adds support to dafny audit for reporting instances of the {:concurrent} attribute, intended to mark code that should be, but can't be proven to be, safe for use in concurrent settings.

Fixes #3463

By submitting this pull request, I confirm that my contribution is made under the terms of the MIT license.

@atomb atomb requested a review from robin-aws March 1, 2023 22:27
Copy link
Member

@robin-aws robin-aws left a comment

Choose a reason for hiding this comment

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

We should add this to the reference manual too at a minimum (and hopefully guide users to that entry for details). Perhaps we could even point to dafny-lang/rfcs#6 as the necessary change to support proving this in many cases?


### Member `ConcurrentMethod`

* Declaration has `{:concurrent}` attribute. MUST manually review and test in a concurrent setting.
Copy link
Member

Choose a reason for hiding this comment

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

Does the auditor have a way of guiding the user to further documentation about a particular class of assumption and mitigation? I feel like this one deserves a bit of text about how Dafny can't (yet) analyze this for you etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

It might make sense to add some text including a link to documentation, for all of the assumption types. I'll look into that.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the reference manual to describe all of the forms of assumption that the auditor reports, including a link to rfcs#6 in the bullet for {:concurrent}. The generated report currently doesn't include a link to that section of the manual, however.

Copy link
Member

Choose a reason for hiding this comment

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

I think that's good enough for now at least, so a user can search the manual for that attribute and get a hit.

@@ -0,0 +1 @@
The `dafny audit` command now reports instances of the `{:concurrent}` attribute, intended to flag code that is intended, but can't be proven, to be safe for use in a concurrent setting.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `dafny audit` command now reports instances of the `{:concurrent}` attribute, intended to flag code that is intended, but can't be proven, to be safe for use in a concurrent setting.
The `dafny audit` command now reports instances of the `{:concurrent}` attribute, intended to flag code that is intended to be safe for use in a concurrent setting.

"but can't be proven" implies there is could that could be proven safe, but Dafny can't do that yet.

Copy link
Member

@robin-aws robin-aws left a comment

Choose a reason for hiding this comment

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

Thanks for the addition to the reference manual - just a quick edit to that content and this LGTM!

docs/DafnyRef/UserGuide.md Outdated Show resolved Hide resolved

### Member `ConcurrentMethod`

* Declaration has `{:concurrent}` attribute. MUST manually review and test in a concurrent setting.
Copy link
Member

Choose a reason for hiding this comment

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

I think that's good enough for now at least, so a user can search the manual for that attribute and get a hit.

@atomb atomb requested a review from robin-aws March 8, 2023 21:04
@atomb atomb enabled auto-merge (squash) March 8, 2023 21:52
@atomb atomb merged commit 6a45d61 into dafny-lang:master Mar 13, 2023
@atomb atomb deleted the auditor-concurrent branch January 4, 2024 17:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

{:concurrent} attribute support in auditor
2 participants