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

feat: Standard library support + DafnyStdLibs.Wrappers #4678

Merged
merged 56 commits into from
Oct 20, 2023

Conversation

robin-aws
Copy link
Member

@robin-aws robin-aws commented Oct 18, 2023

Description

This change adds support for "standard libraries" that are packaged together with the Dafny distribution, at least for now. They are automatically available for import with the option --standard-libraries, which will likely become true by default in Dafny 5. To avoid Dafny's footprint from getting larger and larger over time, some portion of them may be moved into separately distributed packages at a later date. They will remain optional and the Dafny language itself should not depend on them.

A later change will likely add pre-translated source to the runtimes as well, and a translation option such as --include-standard-libraries similar to --include-runtime to rely on this copy of them instead of translating them into every component.

Implementation notes:

  • The primary implementation approach, which is mostly not user-visible, is to pre-build one or more .doo files and embed them in DafnyPipeline.dll, just as we already do with the various runtime files such as DafnyRuntime.dll. They are added as input DafnyFiles, using URIs of the form dllresource://DafnyPipeline/<name>.doo, so they are not re-verified but are in scope for compilation.
  • I've fixed/improved a few things around .doo files, such as loosening the library checks for some options, and no longer checking options that only affect compilation (it may make sense to leverage these files for compilation compatibility checking in the future but that's not currently in scope).
  • The current limitations of the standard library support are documented in Source/DafnyStandardLibraries/CONTRIBUTING.md, and should make it clear which pending libraries are still partially blocked.

How has this been tested?

  • The general standard library support is tested with the integration tests under stdlibs and metatests/StdLibsOffByDefaultInTests.dfy
  • The Wrappers library is tested via WrappersExamples.dfy

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

@robin-aws robin-aws changed the title feat: Standard library support (verification only), DafnyStdLib.Wrappers feat: Standard library support (verification only), DafnyStdLib.Wrappers (WIP) Oct 18, 2023
robin-aws and others added 7 commits October 18, 2023 08:55
Fixes #446

### Description
Do not let documentSymbol API return a faulty line when parsing a symbol
name has an error

### How has this been tested?
- Added language server test

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
### Description
Attempt at making `QuickEditsInLargeFile` pass more reliably on OSX

### How has this been tested?
It's a change intended to affect the existing test
`QuickEditsInLargeFile`

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
Move Littish tests from `Test` to `Source/IntegrationTests/Test`. This
way there's no symlink needed which causes Rider to behave annoyingly

<small>By submitting this pull request, I confirm that my contribution
is made under the terms of the [MIT
license](https://github.com/dafny-lang/dafny/blob/master/LICENSE.txt).</small>
@@ -212,6 +221,30 @@ public class ManifestData {
return false;
}

// Checks that the library option ==> the local option.
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 it's better to use /// comments. IDE will then also move the comments if you move the definition.

but we should be very resistant to raising the limit in the checked in copy.

**Brittleness of code using the standard libraries**: This one is tougher.
There is tension between two different use cases:
Copy link
Member

@keyboardDrummer keyboardDrummer Oct 20, 2023

Choose a reason for hiding this comment

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

Should imports have the ability to customize opaqueness ? @MikaelMayer

Copy link
Member Author

Choose a reason for hiding this comment

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

Have chatted with @jtristan about this a bit too. Cut #4697 to follow up with ideas.

Copy link
Member

Choose a reason for hiding this comment

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

We can deal with this at the library side by refining modules and expose more or less data, and also define careful export sets.
That way, customizing opaqueness is done at the import level.

I also wish users should also be able to refine a library module while exposing everything, and then in their refining module they could hide everything they don't want.

be applied. When a FC-value is returned by a method there is no place to call such a conversion function: the return value of the method must be captured by either `:=` or `:-`.
So some new syntax will be needed --- what this will be is under design and discussion.

The workaround, however, is straitforward: just capture the results using `:=` and make the conversion directly. Here the is example above restated using a method.
Copy link
Member

Choose a reason for hiding this comment

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

give it to me STRAIT

Copy link
Member Author

Choose a reason for hiding this comment

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

🎸


The Wrappers module holds some
simple datatypes to support common patterns, such as optional values or the result of operations that can fail.
These are additionally useful with Dafny's abrupt-termination-on-failure `:-` operator.
Copy link
Member

Choose a reason for hiding this comment

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

I'm not a native English speaker but: additionally -> particularly?

- `Outcome<E>` - which is either `Pass` with no informatino or `Fail` with an error value of type `E` (often a `string`)
- `Result<R,E>` - which is either `Success` with a value of type `R` or `Failure` with error value of type `E`

These are common programming idioms. The main complication comes when they are mixed in the same program.
Copy link
Member

@keyboardDrummer keyboardDrummer Oct 20, 2023

Choose a reason for hiding this comment

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

Consider leaving out line 16. Line 5 already says these datatypes help with common patterns, and the second sentence on line 16 does not constructive to me.


In this library module Dafny defines three such types:
- `Option<R>` - which is either `Some` with a value of type `T` or a `None` with no information
- `Outcome<E>` - which is either `Pass` with no informatino or `Fail` with an error value of type `E` (often a `string`)
Copy link
Member

Choose a reason for hiding this comment

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

informatino

```

You could just capture the result of `Find` using `:=` in a `Option<int>` variable and inspect it. But if the `None` condition is
generally a rare error, it is easy to forget to always check that each operation was successful. Instead, the `:-` changes the
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 I prefer this text not to have line breaks, so the viewer can take care of the appropriate breaking.

Copy link
Member

Choose a reason for hiding this comment

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

Looks like this when I'm reading it
image

Copy link
Member Author

@robin-aws robin-aws Oct 20, 2023

Choose a reason for hiding this comment

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

My general preference is loosely adhering to https://sembr.org/ personally. I didn't want to edit this too heavily so we can see a clean diff against the original, though: https://github.com/dafny-lang/libraries/blob/master/src/dafny/Wrappers.md

Copy link
Member

Choose a reason for hiding this comment

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

I don't get it. Why wouldn't you want regular text to look the same when writing as when reading? If we're soft-wrapping it with no explicit line breaks when reading, why not also write it like that?

See the [reference manual](https://dafny.org/latest/DafnyRef/DafnyRef#sec-update-with-failure-statement)
for more on the similarities and differences with exceptions.

### Outcome
Copy link
Member

@keyboardDrummer keyboardDrummer Oct 20, 2023

Choose a reason for hiding this comment

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

Is this used a lot? C#'s anonymous records Option<(int x, string y)> seem nicer

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 actually disagree with the statement that this is the most common use case. It's more important that returning an Outcome instead of a Result means you can say :- CheckSomeThings(x) as opposed to var _ :- CheckSomeThings(x), where in the latter case it's too easy to do var _ := CheckSomeThings(x) which has no effect!

Rewriting.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ended up just flipping the order of use cases.


A `Result` carries both a success value and failure information, with two separate, specifiable types. Its use is then very similar to `Option`.

### Combining different FC-types in expressions
Copy link
Member

Choose a reason for hiding this comment

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

While the acronym FC has been defined before (once, and was not used afterwards), I would not use it in the title here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agreed about the titles, although "FC" is used a handful of times in the content.

Copy link
Member

@keyboardDrummer keyboardDrummer left a comment

Choose a reason for hiding this comment

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

Amazing! Love it

enforcing a tight upper-bound on the resources needed to verify each batch of assertions.
For simplicitly, this project just sets a direct `--resource-limit`,
rather than relying on the second-pass approach of the `dafny-reportgenerator`.
I have set this limit aggressively low in the hopes that we can maintain it
Copy link
Contributor

Choose a reason for hiding this comment

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

I would use a passive voice there.

@robin-aws robin-aws changed the title feat: Standard library support (verification only), DafnyStdLib.Wrappers (WIP) feat: Standard library support (verification only), DafnyStdLib.Wrappers Oct 20, 2023
@robin-aws robin-aws changed the title feat: Standard library support (verification only), DafnyStdLib.Wrappers feat: Standard library support (verification only), DafnyStdLibs.Wrappers Oct 20, 2023
@robin-aws robin-aws changed the title feat: Standard library support (verification only), DafnyStdLibs.Wrappers feat: Standard library support (pre-verification only), DafnyStdLibs.Wrappers Oct 20, 2023
@robin-aws robin-aws changed the title feat: Standard library support (pre-verification only), DafnyStdLibs.Wrappers feat: Standard library support + DafnyStdLibs.Wrappers Oct 20, 2023
@robin-aws robin-aws enabled auto-merge (squash) October 20, 2023 17:07
@robin-aws robin-aws enabled auto-merge (squash) October 20, 2023 17:08
@robin-aws robin-aws merged commit 38416f8 into master Oct 20, 2023
20 checks passed
@robin-aws robin-aws deleted the standard-library-support-verification-only branch October 20, 2023 22:03
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.

4 participants