snoyberg / mono-traversable

Type classes for mapping, folding, and traversing monomorphic containers
153 stars 63 forks source link

GHC 8 rejects dangling `INLINE` pragmas #90

Closed hvr closed 8 years ago

hvr commented 8 years ago

While it can be argued whether this should be an error or a warning (@bgamari ping?), the code seems in fact to contain INLINE pragmas which are misplaced... :-)

[2 of 7] Compiling Data.MonoTraversable ( src/Data/MonoTraversable.hs, /tmp/mono-traversable/dist-newstyle/build/mono-traversable-0.10.1/build/Data/MonoTraversable.o )

src/Data/MonoTraversable.hs:489:16: error:
    The INLINE pragma for ‘unsafeHead’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘unsafeHead’ is declared)

src/Data/MonoTraversable.hs:511:16: error:
    The INLINE pragma for ‘omapM_’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘omapM_’ is declared)

src/Data/MonoTraversable.hs:516:16: error:
    The INLINE pragma for ‘unsafeHead’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘unsafeHead’ is declared)

src/Data/MonoTraversable.hs:537:16: error:
    The INLINE pragma for ‘olength’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘olength’ is declared)

src/Data/MonoTraversable.hs:538:16: error:
    The INLINE pragma for ‘omapM_’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘omapM_’ is declared)

src/Data/MonoTraversable.hs:543:16: error:
    The INLINE pragma for ‘unsafeHead’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘unsafeHead’ is declared)

src/Data/MonoTraversable.hs:557:16: error:
    The INLINE pragma for ‘oall’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘oall’ is declared)

src/Data/MonoTraversable.hs:558:16: error:
    The INLINE pragma for ‘oany’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘oany’ is declared)

src/Data/MonoTraversable.hs:561:16: error:
    The INLINE pragma for ‘omapM_’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘omapM_’ is declared)

src/Data/MonoTraversable.hs:564:16: error:
    The INLINE pragma for ‘headEx’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘headEx’ is declared)

src/Data/MonoTraversable.hs:565:16: error:
    The INLINE pragma for ‘lastEx’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘lastEx’ is declared)

src/Data/MonoTraversable.hs:566:16: error:
    The INLINE pragma for ‘unsafeHead’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘unsafeHead’ is declared)

src/Data/MonoTraversable.hs:616:16: error:
    The INLINE pragma for ‘omapM_’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘omapM_’ is declared)

src/Data/MonoTraversable.hs:657:16: error:
    The INLINE pragma for ‘omapM_’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘omapM_’ is declared)

src/Data/MonoTraversable.hs:690:16: error:
    The INLINE pragma for ‘omapM_’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘omapM_’ is declared)

src/Data/MonoTraversable.hs:731:16: error:
    The INLINE pragma for ‘headEx’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘headEx’ is declared)

src/Data/MonoTraversable.hs:732:16: error:
    The INLINE pragma for ‘lastEx’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘lastEx’ is declared)

src/Data/MonoTraversable.hs:733:16: error:
    The INLINE pragma for ‘unsafeHead’ lacks an accompanying binding
      (The INLINE pragma must be given where ‘unsafeHead’ is declared)
bgamari commented 8 years ago

Indeed the severity of this is arguable. If someone feels strongly one way or another please open a ticket so we can discuss it.

gregwebs commented 8 years ago

Thanks for the report! I am fine with this being an error because there is no reason not to fix it. On the other hand, we would get about the same benefit from it being a warning and that would help with not breaking existing code.

hvr commented 8 years ago

@gregwebs fwiw, I just realised that all GHCs dating back to at least GHC 7.0 throw an error for spurious INLINE pragmas. It just turns out that GHCs prior to GHC 8 failed to detect these specific cases of detached INLINEs.

snoyberg commented 8 years ago

Thanks for the report, and this change looks good. I've removed the pragmas listed in your error message. Would you mind testing from HEAD before I release?

bgamari commented 8 years ago

@snoyberg it looks like there are more such INLINEs in Data.Sequences.

snoyberg commented 8 years ago

OK, I actually got set up locally with GHC 8 and got this tested myself. This seems to be working correctly now, and I've released to Hackage. Thanks again for the report!