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

Remove use of unsafeCoerce in ExprMappable #332

Closed
danmatichuk opened this issue Sep 12, 2022 · 0 comments
Closed

Remove use of unsafeCoerce in ExprMappable #332

danmatichuk opened this issue Sep 12, 2022 · 0 comments
Labels
stale Review - close as 'wontfix' or integrate into other ticket tech debt

Comments

@danmatichuk
Copy link
Collaborator

danmatichuk commented Sep 12, 2022

The function ExprMappable.symExprMappable uses unsafeCoerce as a workaround to define an ad-hoc ExprMappable instance for SymExpr, which is otherwise not possible in the Haskell type system since SymExpr is defined as a type family.
Although this coercion is currently not problematic, future changes to the run-time representation of ToExprMappable would introduce a run-time error if not handled correctly.

There are two solutions to avoid this:

  1. Add the explicit constraint that the sym parameter for mapExpr is an ExprBuilder. This would avoid the need for reasoning about the SymExpr type family and instead allow reasoning about What4.Expr directly, which we can define instances for normally. This has the downside of requiring some refactoring to carry around this constraint in more places, since it is not necessarily available in every place mapExpr is used.

  2. Use unsafeDerive provided by Data.Constraint.Unsafe, which is still unsafe but in a much more constrained way than unsafeCoerce (and generally makes it more clear exactly what is being coerced). This change would be entirely confined to the ExprMappable module (and so would be a straightforward change), but still admits the possibility of allowing run-time errors.

    • In general unsafeDerive can let you prove nonsense type constraints like 1 ~ 2, as well as create contexts where multiple class instances are in scope for the same type.
@thebendavis thebendavis added the stale Review - close as 'wontfix' or integrate into other ticket label May 31, 2024
@thebendavis thebendavis closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Review - close as 'wontfix' or integrate into other ticket tech debt
Projects
None yet
Development

No branches or pull requests

2 participants