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

crucible-mir: Remove vestigial ?mirCS and ?mirLib implicit params, as well as Monoid instances for Collection/CollectionState/RustModule #1259

Open
RyanGlScott opened this issue Sep 24, 2024 · 0 comments
Labels
crucible MIR Issues relating to Rust/MIR support technical debt

Comments

@RyanGlScott
Copy link
Contributor

In #1253, we propose to make crucible-mir respect the mir-json schema version. As part of implementing this, it would be convenient to store the version number in the parsed Collection, as this could be useful to report to the user in various situations. However, there is an obstacle that stands in our way of doing so: Collection has a Monoid instance, and it's very unclear how to implement mempty and mappend for version numbers. We certainly don't want to conjure up a version number out of thin air in mempty, and it's unclear how we would combine two different version numbers.

Why does Collection have a Monoid instance anyway? It ultimately dates back to an old approach that crux-mir used for translating Collections in commit 12df289. In that approach, one would call translateMIR multiple times and combine the results using mappend. However, that approach was removed in commit b0708a0. After that commit, the only time we ever use the Monoid instance for Collection is to call mappend (mempty :: Collection) x. This could just as well be simplified to x, thereby avoiding the need for the Monoid instance in the first place.

Similarly, we could remove:

  • The Monoid instances for CollectionState and RustModule (which depend on the Monoid Collection instance, but are similarly unused)
  • The ?mirCS and ?mirLib implicit parameters (which are only ever instantiated with mempty).
@RyanGlScott RyanGlScott added crucible technical debt MIR Issues relating to Rust/MIR support labels Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crucible MIR Issues relating to Rust/MIR support technical debt
Projects
None yet
Development

No branches or pull requests

1 participant