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

Standard libraries and separate compilation #5981

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

keyboardDrummer
Copy link
Member

@keyboardDrummer keyboardDrummer commented Dec 13, 2024

Fixes dafny-lang/dafny-gradle-plugin#11

What was changed?

  • Add option --translate-standard-library that enables using --standard-libraries in conjunction with separate compilation

How has this been tested?

  • Added comp/separate-compilation/standard-libaries/user.dfy

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

@keyboardDrummer keyboardDrummer enabled auto-merge (squash) December 13, 2024 13:25
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.

Love the option, just quibbling about default behavior and documentation.

I think it's worth mentioning this option in https://github.com/dafny-lang/dafny/blob/master/Source/DafnyStandardLibraries/README.md too, so users are more likely to discover it.

if (options.Backend is LibraryBackend) {
options.Set(CommonOptionBag.TranslateStandardLibrary, false);
}

// For now the standard libraries are still translated from scratch.
// This breaks separate compilation and will be addressed in https://github.com/dafny-lang/dafny/pull/4877
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 you're addressing "this breaks separate compilation" with this change so just that part of the comment can be removed, although I think the rest still applies (although I think we might publish the libraries separately rather than embed them in the runtimes)

if (Options.Backend is LibraryBackend) {
Options.Set(CommonOptionBag.TranslateStandardLibrary, false);
}

// For now the standard libraries are still translated from scratch.
// This breaks separate compilation and will be addressed in https://github.com/dafny-lang/dafny/pull/4877
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@@ -102,6 +102,7 @@ static DafnyCommands() {
CommonOptionBag.NewTypeInferenceDebug,
Method.ReadsClausesOnMethods,
CommonOptionBag.UseStandardLibraries,
CommonOptionBag.TranslateStandardLibrary,
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be a translation option? It doesn't make sense to pass this to resolve or verify

Comment on lines +370 to +371
@"Currently Dafny does not prevent separately built Dafny projects from each including the standard library,
leading to conflicts. When combining such projects, please ensure that only one of them has --translate-standard-library set to true.
Copy link
Member

Choose a reason for hiding this comment

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

The first sentence feels overly defensive to me, but it might be because we see the current state differently. I don't think it's inherently wrong to merge doo files personally, building larger aggregate libraries, as long as you don't use them incorrectly.

public static readonly Option<bool> TranslateStandardLibrary = new("--translate-standard-library", () => true,
@"Currently Dafny does not prevent separately built Dafny projects from each including the standard library,
leading to conflicts. When combining such projects, please ensure that only one of them has --translate-standard-library set to true.
When building a Dafny library (.doo file), this option will automatically be set to false.
Copy link
Member

Choose a reason for hiding this comment

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

This feels like it's going to be confusing and hard to debug, especially since it's a change in behavior. Wouldn't it be better to fail instead, and make users set --translate-standard-library:false explicitly?

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.

Don't pass --standard-libraries in translateDafnyToJava
2 participants