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

Make runTLS{Client,Server}StartTLS general #264

Merged
merged 2 commits into from May 11, 2016
Merged

Conversation

abbradar
Copy link
Contributor

@abbradar abbradar commented May 5, 2016

This makes it possible to use general runTLSClientStartTLS without losing monadic state, and also generalizes runTLSServerStartTLS. Consists of several parts:

  1. Make top function in ApplicationStartTLS return a -- this allows us to return and re-introduce wrapped monadic state with liftBaseOp.
  2. Also make it parametrized by a monad. This is to not lose monadic state from the nested startTLS function. Another possible solution would be to use impredicative types to allow returning value from it (replace (AppData -> IO ()) -> IO () with forall a. (AppData -> IO a) -> IO a and use ordinary liftBaseOp). Unfortunately, GHC doesn't work well with this.
  3. Finally, generalize both runTLS*StartTLS functions.

Also this allows one to return value from runTLSClientStartTLS (not strictly needed for this patch) and drops an extra constraint from runTLSClient.

I'm not sure if this needs to be done some other way w.r.t. backwards compatibility.

EDIT: Also notice that (1) is also not strictly required as we already have (2) for passing state. If needed, I can split this into a different PR.

@snoyberg
Copy link
Owner

Changing the existing type synonym breaks backwards compatibility, so I'd rather just define a new type synonym. Other than that, changes look backwards compatible.

@abbradar
Copy link
Contributor Author

Fixed!

@snoyberg snoyberg merged commit 46b13a1 into snoyberg:master May 11, 2016
@snoyberg
Copy link
Owner

Thanks!

snoyberg added a commit that referenced this pull request May 11, 2016
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.

None yet

2 participants