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 Foreign.Marshal.Error.void with a re-export of Data.Functor.void #264

Closed
clyring opened this issue Mar 30, 2024 · 17 comments
Closed
Labels
abandoned Abandoned by proposer (no-show)

Comments

@clyring
Copy link
Member

clyring commented Mar 30, 2024

These two functions have identical behavior and names, but differing types. (This can cause annoying and pointless "ambiguous occurrence" errors when both are in scope.) Data.Functor.void is more general than Foreign.Marshal.Error.void, but the proposed change is nevertheless a breaking one since the more general type of Data.Functor.void can cause type inference to fail in some cases.

But Foreign.Marshal.Error.void has been deprecated since base-4.6.0.0, which shipped with ghc-7.6.1 and was released in September 2012, more than a decade ago. Potentially-affected users have had plenty of time to migrate.

Side note: Foreign.Marshal.Error.void is specified in the Haskell2010 report; see this chapter. Otherwise I might propose to just delete this function instead of generalizing it.

@Bodigrim
Copy link
Collaborator

Given that the function is deprecated for a decade, I don’t see a reason not to delete it. We are way far away from Haskell Report to bother about it.

@Bodigrim
Copy link
Collaborator

@hasufell
Copy link
Member

We are way far away from Haskell Report to bother about it.

I would like a base that conforms to the spec or an update to the spec.

Do we know why the deprecation was introduced?

I don't see base in the 7.6 tree https://gitlab.haskell.org/ghc/ghc/-/tree/ghc-7.6/libraries

@tomjaguarpaw
Copy link
Member

I don't have a strong opinion between the three options (no change, replace with general version, remove it).

@Bodigrim
Copy link
Collaborator

Do we know why the deprecation was introduced?

I don't see base in the 7.6 tree https://gitlab.haskell.org/ghc/ghc/-/tree/ghc-7.6/libraries

It was deprecated by @simonmar in https://gitlab.haskell.org/ghc/ghc/-/commit/99490045a60eca41e79c2158d839a65cf454de2b, Apr 2012, but there is no explanation or discussion attached.

@tomjaguarpaw
Copy link
Member

Given the deprecation message I guess the reason was that the more general version of void should be preferred!

{-# DEPRECATED void "use Control.Monad.void instead" #-}

@mixphix
Copy link
Collaborator

mixphix commented Apr 22, 2024

I think its removal is safe by now. I think if Haskell should stick to a Report, it should be a modern one, but such a Report has not yet been written.

@Bodigrim
Copy link
Collaborator

I'm in favor of removal.

@Bodigrim
Copy link
Collaborator

Bodigrim commented Apr 25, 2024

@parsonsmatt @angerman @velveteer any more opinions on the preferred course of actions here?

@parsonsmatt
Copy link

I'm in favor of removing it or replacing with a re-export of Data.Functor.void.

@clyring
Copy link
Member Author

clyring commented Apr 26, 2024

Another point to consider: A fair number of packages have import Foreign hiding ( void ) which will trigger a -Wdodgy-imports warning if void is completely removed from Foreign.Marshal.Error. That's not a big deal: there are several acceptable ways of dealing with this warning, including silencing it. But it does probably create busy-work for the relevant maintainers, which could be avoided by replacing Foreign.Marshal.Error.void with a re-export of Data.Functor.void.

But I won't sleep any less soundly if the committee still prefers to delete the function instead.

@hasufell
Copy link
Member

@clyring we don't consider warnings breaking changes: #262

Werror users get what they asked for.

@Bodigrim
Copy link
Collaborator

I think we all agree that Foreign.Marshal.Error is not a right place for void :: Functor f => f a -> f (), it's just that CLC members may have different appetite when it comes to breakage.

As noted in the proposal, replacing void :: IO a -> IO () with void :: Functor f => f a -> f () is also a breaking change, because it can potentially make type checking ambiguous. So it would require an impact assessment, same as just removing it. So I guess it's a question which impact assessment (and patches) @clyring is motivated enough to perform.

If we go for re-export, let's deprecate it, so that we do not close the door to the eventual removal of the function.

@Bodigrim Bodigrim added the awaits-impact-assessment Awaits an impact assessment (e. g., using https://github.com/Bodigrim/clc-stackage) label Apr 29, 2024
@Bodigrim
Copy link
Collaborator

@clyring can we make more progress on this?

@Bodigrim
Copy link
Collaborator

@clyring just a gentle reminder of your proposal.

@Bodigrim
Copy link
Collaborator

@clyring unless there is some progress by the end of August, I'll have to close the proposal as abandoned.

@Bodigrim
Copy link
Collaborator

Bodigrim commented Sep 1, 2024

Closing as abandoned, feel free to reopen when there are resources to continue.

@Bodigrim Bodigrim closed this as completed Sep 1, 2024
@Bodigrim Bodigrim added abandoned Abandoned by proposer (no-show) and removed awaits-impact-assessment Awaits an impact assessment (e. g., using https://github.com/Bodigrim/clc-stackage) labels Sep 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned Abandoned by proposer (no-show)
Projects
None yet
Development

No branches or pull requests

6 participants