ekmett / profunctors

Haskell 98 Profunctors
http://hackage.haskell.org/package/profunctors
Other
70 stars 43 forks source link

Explicitly mark modules as Safe #94

Closed phadej closed 3 years ago

phadej commented 3 years ago

This depends on https://github.com/ekmett/tagged/pull/51 being released

Lower bounds are bumped to releases which have explicit Safe Haskell annotations. (I also tested them locally).

phadej commented 3 years ago

The GHC-8.10 job demonstrates that -Winferred-safe-imports works slightly too well in GHC-8.10 :(

RyanGlScott commented 3 years ago

Since this depends on ekmett/tagged#51, should I cut a new release of tagged now? Or is this something that can wait?

phadej commented 3 years ago

I mada https://github.com/ekmett/tagged/pull/52, and with that I'd welcome tagged-0.8.6.1 to Hackage!

RyanGlScott commented 3 years ago

FYI: Since this PR is marked as a draft, I'm unable to merge it. When you feel that this ready to land, unmark it as a draft and ping me.

One question: I noticed that Data.Profunctor.Unsafe is currently not given any kind of Safe Haskell pragma. Should it be marked explicitly as Unsafe?

phadej commented 3 years ago

One question: I noticed that Data.Profunctor.Unsafe is currently not given any kind of Safe Haskell pragma. Should it be marked explicitly as Unsafe?

Yes. I now wonder why -Wmissing-safe-haskell-mode doesn't say anything about it. (I'll look what I really wrote in GHC).

phadej commented 3 years ago

That was wrong.

Data.Profunctor.Unsafe is given Trustworthy by... @ekmett already before 2015 https://github.com/ekmett/profunctors/commit/1c3508e5

phadej commented 3 years ago

Sorry for chatty comment spam.

(In genreal chaning from Trustworthy to explicitly Unsafe would be breaking change.)

However, Data.Profunctor.Unsafe doesn't have anything unsafe anymore, or rather, it has Coercible stuff, which is unsafe in base, but IMO it's trustworthy.

Recent issue in GHC: https://gitlab.haskell.org/ghc/ghc/-/issues/19129

phadej commented 3 years ago

I checked lower bounds. Please (re)review.

RyanGlScott commented 3 years ago

However, Data.Profunctor.Unsafe doesn't have anything unsafe anymore, or rather, it has Coercible stuff, which is unsafe in base, but IMO it's trustworthy.

Good point. I think the "unsafe" part of Data.Profunctor.Unsafe came from the days when profunctors supported GHCs without Coercible, and as a result, some of the operations had to be implemented with unsafeCoerce. In light of this, perhaps the "unsafe" part should be revisited. But that doesn't have to happen now.

RyanGlScott commented 3 years ago

I've uploaded profunctors-5.6.1 to Hackage with these changes.