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

Add CanonicalProjection #1526

Merged
merged 1 commit into from
Sep 12, 2022
Merged

Conversation

janmasrovira
Copy link
Collaborator

@janmasrovira janmasrovira commented Sep 9, 2022

This pr adds this class:

class CanonicalProjection a b where
  project :: a -> b

It is meant to be used for records types that have an obvious projection into another record. E.g.

instance CanonicalProjection GlobalOptions Internal.Options where
  project g =
    Internal.Options
      { Internal._optShowNameIds = g ^. globalShowNameIds
      }

I have updated the pretty printing functions that expect options to accept anything that has a canonical projection into the options.

The benefit is that the callsites can usually be simplified since you do not need to do an adhoc projection or remember the name of the projection if it exists.

-                  let ppOpts =
-                        Internal.defaultOptions
-                          { Internal._optShowNameIds = globalOpts ^. globalShowNameIds
-                          }
-                  App.renderStdOut (Internal.ppOut ppOpts micro)
+                  App.renderStdOut (Internal.ppOut globalOpts micro)

We have discussed this with @paulcadman and we have a small preference to have this. It'd be nice to know the opinion of @lukaszcz

@lukaszcz
Copy link
Collaborator

lukaszcz commented Sep 9, 2022

I'm in favour of implicit coercions, or things that approximate them!

Unrelated: I think we should have user-declarable implicit coercions in Juvix at some point (e.g. Nat -> Integer). Without implicit coercions in Coq things like MathComp, or any formalisation with sophisticated type structure, would be totally unreadable.

@paulcadman paulcadman merged commit 380ade5 into main Sep 12, 2022
@paulcadman paulcadman deleted the canonical-projection-for-option-records branch September 12, 2022 08:44
@lukaszcz lukaszcz added this to the 0.2.6 milestone Sep 27, 2022
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