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

Replace polymorphic proxy workaround with monomorphic Proxy type #281

Merged
merged 5 commits into from
Mar 14, 2022
Merged

Replace polymorphic proxy workaround with monomorphic Proxy type #281

merged 5 commits into from
Mar 14, 2022

Conversation

JordanMartinez
Copy link
Contributor

Description of the change

Fixes #231 by changing forall proxy. proxy SomeKind -> .. to just Proxy SomeKind.

Also removes Proxy2 and Proxy3 types since Proxy is enough now.


Checklist:

  • Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

@JordanMartinez JordanMartinez added type: breaking change A change that requires a major version bump. purs-0.15 A reminder to address this issue or merge this PR before we release PureScript v0.15.0 labels Feb 15, 2022
@JordanMartinez JordanMartinez changed the title Use single Proxy Replace polymorphic proxy workaround with monomorphic Proxy type Feb 15, 2022
@garyb
Copy link
Member

garyb commented Feb 15, 2022

Is there a downside to leaving proxy polymorphic?

@JordanMartinez
Copy link
Contributor Author

I'm not sure. I suppose one could do something like

data MyProxyType (sym :: Symbol) = MyProxyType OtherInfo

reflectSymbol (MyProxyType "foo" :: MyProxyType "bar")

But if this is hard-coded to Proxy, I don't think it would cause problems either.

@thomashoneyman
Copy link
Member

thomashoneyman commented Feb 16, 2022

Is there a downside to leaving proxy polymorphic?

What about the opposite — is there an advantage to leaving it polymorphic? As far as I remember it was a stopgap to reduce breakage but the plan was to remove it.

The main thing that comes to mind is that leaving it polymorphic is a little less clear than an explicit Proxy argument (to me), including in type errors. As a consequence we have things like Proxy2 or Proxy3, which really aren’t necessary anymore (right?).

@garyb
Copy link
Member

garyb commented Feb 16, 2022

The hypothetical advantage is you could use types other than proxy that carry the symbol... like, say you have a generic Id type that uses a symbol for discriminating between different entities:

newtype Id (tagSymbol) = Id NonEmptyString

type ItemId = Id "Item"
type OrderId = Id "Order"

You could just pass your ItemId or OrderId to places that currently expect proxy sym:

printIdType   tag. IsSymbol tag  Id tag  String
printIdType = reflectSymbol

But it's not really that big a deal either, it should always be possible to instantiate a Proxy with the appropriate type anyway:

printIdType   tag. IsSymbol tag  Id tag  String
printIdType _ = reflectSymbol (Proxy  _ tag)

At the worst it would require adding some type signatures. Also "constructing" a Proxy is just a value dereference at runtime, the same as passing the Id _ value here, so if we think it's better for understandability purposes to make it monomorphic then I think that's fine.

I do actually have some types like Id here, but when I checked I found I'm always using Proxy anyway due to the way things worked out. 😄

@JordanMartinez JordanMartinez merged commit b3e64dd into purescript:master Mar 14, 2022
@JordanMartinez JordanMartinez deleted the use-single-proxy branch March 14, 2022 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
purs-0.15 A reminder to address this issue or merge this PR before we release PureScript v0.15.0 type: breaking change A change that requires a major version bump.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update type signature for reflectSymbol for PureScript 0.15
3 participants