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

Warn when using non doo file as a library #5313

Merged
merged 26 commits into from
Apr 11, 2024

Conversation

keyboardDrummer
Copy link
Member

Changes

  • Warn when passing non-doo files to --library
  • Lots of code was made asynchronous. These locations are performing IO, so it's good that they're async now.

How has this been tested?

  • Updated existing CLI tests

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) April 10, 2024 10:02
@@ -0,0 +1,2 @@
- Warn when passing something other than a doo file to `--library`
- Allow using project files in the same places as `.dfy` files. They behave as if they are a Dafny source file that includes all the sources and library files defined in the project file
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
- Allow using project files in the same places as `.dfy` files. They behave as if they are a Dafny source file that includes all the sources and library files defined in the project file

(or rather move to #5297 :)

}

result.Add(DafnyFile.CreateAndValidate(errorReporter, fileSystem, Options, DafnyMain.StandardLibrariesDooUri, Project.StartingToken));
var file = await DafnyFile.CreateAndValidate(errorReporter, fileSystem, Options, DafnyMain.StandardLibrariesDooUri, Project.StartingToken);
if (file != null) {
Copy link
Member

Choose a reason for hiding this comment

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

This is a bit dangerous though, isn't it? If there's some packaging error and the main stdlib doo file is missing, we'll silently ignore it. Better to throw a description internal exception I think.

Copy link
Member Author

Choose a reason for hiding this comment

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

Oof, I don't recall why this was needed. Well, fastest way to remember is to remove it again ^,^

Copy link
Member Author

@keyboardDrummer keyboardDrummer Apr 10, 2024

Choose a reason for hiding this comment

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

Without it, the tests fail:

  Failed stdlibs/StandardLibraries_Errors.dfy [52 s]
  Error Message:
   System.AggregateException : One or more errors occurred. (Command returned non-zero exit code (1): DiffCommand TestFiles/LitTests/LitTest/stdlibs/StandardLibraries_Errors.dfy.expect /home/runner/work/dafny/dafny/dafny/Source/IntegrationTests/bin/Debug/net6.0/TestFiles/LitTests/LitTest/stdlibs/Output/StandardLibraries_Errors.dfy.tmp
Output:
AssertEqualWithDiff() Failure
Diff (changing expected into actual):
 
 Dafny program verifier finished with 1 verified, 0 errors
 
 Dafny program verifier finished with 1 verified, 0 errors
 StandardLibraries_Errors.dfy(20,20): Error: module Std does not exist (position 0 in path Std.Wrappers)
 1 resolution/type errors detected in StandardLibraries_Errors.dfy
 CLI: Error: cannot load DafnyStandardLibraries-notarget.doo: --unicode-char is set locally to False, but the library was built with True
 CLI: Error: cannot load DafnyStandardLibraries.doo: --unicode-char is set locally to False, but the library was built with True
+Encountered internal compilation exception: Object reference not set to an instance of an object.
 Please use the '--check' and/or '--print' option as doo files cannot be formatted in place.
 Please use the '--check' and/or '--print' option as doo files cannot be formatted in place.

Seems there is no custom check to prevent --unicode-char false and --standard-libraries from being used together. This is discovered and reported in the call to DafnyFile.CreateAndValidate, after which it still returns null and should not crash because of that.

Copy link
Member

Choose a reason for hiding this comment

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

Ah that's fair, if the file is missing it should fail more loudly. I forgot that CreateAndValidate only returns null if the file wasn't valid.

Comment on lines 191 to 194
$"The file '{Options.GetPrintPath(unverifiedLibrary)}' was passed to --library. " +
$"Verification previously done for that file might not be sufficient, " +
$"because it might have used options incompatible with the current ones. " +
$"Use a .doo file to enable Dafny to check that compatible options were used");
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 file '{Options.GetPrintPath(unverifiedLibrary)}' was passed to --library. " +
$"Verification previously done for that file might not be sufficient, " +
$"because it might have used options incompatible with the current ones. " +
$"Use a .doo file to enable Dafny to check that compatible options were used");
```suggestion
$"The file '{Options.GetPrintPath(unverifiedLibrary)}' was passed to --library. " +
$"Verification for that file might have used options incompatible with the current ones, or might have been skipped entirely. " +
$"Use a .doo file to enable Dafny to check that compatible options were used.");

robin-aws
robin-aws previously approved these changes Apr 10, 2024
@keyboardDrummer keyboardDrummer merged commit 4e586b5 into dafny-lang:master Apr 11, 2024
20 checks passed
@keyboardDrummer keyboardDrummer deleted the warnNonDooLibrary branch April 11, 2024 16:24
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.

3 participants