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

Dafny IR does not generate modules in dependency order anymore #5788

Open
dnezam opened this issue Sep 22, 2024 · 2 comments
Open

Dafny IR does not generate modules in dependency order anymore #5788

dnezam opened this issue Sep 22, 2024 · 2 comments
Assignees
Labels
kind: bug Crashes, unsoundness, incorrect output, etc. If possible, add a `part:` label

Comments

@dnezam
Copy link

dnezam commented Sep 22, 2024

Dafny version

4.8.0

Code to produce this issue

No response

Command to run and resulting output

No response

What happened?

Previously, the Dafny IR generated modules in a linear and topologically sorted order which the Dafny to CakeML compiler relied on. However, updating to the current master branch broke this behavior: for example, the default module _module is now mentioned first. Since the Dafny Reference mentions that it will figure out a topological order (emphasis mine)

(4.7. Module Ordering and Dependencies) "In particular, there must be a way to order the modules in a program such that each only refers to things defined before it in the ordering. [...] Dafny will figure out that order for you, assuming you haven’t made any circular references. "

I really would like to utilize it. I confirmed that the offending PR is #5684. If possible, it would be nice if this could be reverted for now to make sure that the CakeML compiler does not drift apart too much. If something downstream of the Rust compiler relies on this, perhaps it could be added there.

What type of operating system are you experiencing the problem on?

Linux

@dnezam dnezam added the kind: bug Crashes, unsoundness, incorrect output, etc. If possible, add a `part:` label label Sep 22, 2024
dnezam added a commit to CakeML/cakeml that referenced this issue Sep 22, 2024
Waiting for resolution of dafny-lang/dafny#5788
before creating a PR.
dnezam added a commit to dnezam/dafny that referenced this issue Sep 25, 2024
…dafny-lang#5684)"

This reverts commit 299e2c8 as a
temporary workaround for the issue described in
dafny-lang#5788
@MikaelMayer
Copy link
Member

Let's find a solution that suits both of us. The problem with the topological order I thought of was that it was not reliable for modules that did not depend on each other, which was causing trouble with code that would patch the output. Indeed, the C# specification said that the order in which we iterate keys in a dictionary is not guaranteed, although the implementation does it perfectly.
That said, there are two things that came up

All of this to say @robin-aws , I think it's safe and we can revert the commit that ensured module ordering to revert it back to topological ordering. What do you think?

@robin-aws
Copy link
Member

Could we restore the topological sort, but make it a stable sort w.r.t. an alphabetical ordering, so we get a deterministic ordering that still respects dependencies?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Crashes, unsoundness, incorrect output, etc. If possible, add a `part:` label
Projects
None yet
Development

No branches or pull requests

3 participants