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

fix: File contains no code even when there is code in prefixed modules #1840

Merged
merged 6 commits into from
Feb 21, 2022

Conversation

MikaelMayer
Copy link
Member

@MikaelMayer MikaelMayer commented Feb 18, 2022

Fix #1838

This PR changes Dafny.atg so that it now reads (added part in bold)

if (membersDefaultClass.Count == 0 && defaultModule.Includes.Count == 0 && defaultModule.TopLevelDecls.Count == 0
&& (defaultModule.PrefixNamedModules == null || defaultModule.PrefixNamedModules.Count == 0)) {

That way, it does not trigger the warning "File contains no code" if there are some prefixed modules inside the main module.
The original files that were testing this warning were actually testing when we accidentally put a comment that comments the entire file, and another file that does not actually contain code, so this PR is consistent with them.

RELEASE_NOTES.md Outdated
@@ -1,7 +1,7 @@
# Upcoming

- fix: No output when compiling to JavaScript on Windows (https://github.com/dafny-lang/dafny/pull/1824)

- fix: No warning "File contains no code" if module contains a submodule (https://github.com/dafny-lang/dafny/pull/1840)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be in the release notes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Isn't this the file RELEASE_NOTES.md ? Note how in 1 commit, I was able to predict the pull request number :-)

Copy link
Member

Choose a reason for hiding this comment

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

I meant that maybe this should not be mentioned in the release notes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh I got your comment. Yes I think it can reasonably go there.

Copy link
Member Author

Choose a reason for hiding this comment

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

We can always decide to remove it later, right @fabiomadge ?

Copy link
Member

Choose a reason for hiding this comment

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

Sure

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think making the call now is useful. The point of preparing the notes is, that we don't need to think about them at release time.
In this case I think that this is user facing and has a place in the notes. I wouldn't object to later aggregating it under something like:

##Resolution
...

@MikaelMayer MikaelMayer changed the title Fixes #1838 fix: File contains no code even when there is code in prefixed modules Feb 18, 2022
@MikaelMayer MikaelMayer self-assigned this Feb 18, 2022
RELEASE_NOTES.md Outdated Show resolved Hide resolved
@MikaelMayer MikaelMayer enabled auto-merge (squash) February 21, 2022 13:25
@MikaelMayer MikaelMayer merged commit 663e22f into master Feb 21, 2022
@MikaelMayer MikaelMayer deleted the fix-1838-file-no-code branch February 21, 2022 14:12
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.

Inconsistent submodule behavior
3 participants