Avoid Lift ByteArray breaking with RebindableSyntax

[TeofilC]

haskell/text#534 fixes a bug in the Lift Text instance namely the way numeric values were lifted meant that in the presence of RebindableSyntax, which might override the meaning of numeric literals, the code would break.

The Lift ByteArray instance in base¹ is vulnerable to the same issue.

https://gitlab.haskell.org/ghc/ghc/-/merge_requests/13270 fixes this but leads to a change in behaviour. If a user were to inspect the output of the lift call they would see a largely equivalent but distinct AST.

It's highly unlikely that this would break anyone's code. ByteArray was only recently added to base, and most users of lift do not inspect its output.

Here is the relevant part of the diff for context:

  lift (ByteArray b) =
-    [| addrToByteArray $(lift len)
+   -- we cannot use Lift Int here because of #25267 and #25272
+   [| addrToByteArray $(pure . LitE . IntPrimL $ fromIntegral len)
                       $(pure . LitE . BytesPrimL $ Bytes ptr 0 (fromIntegral len))
    |]

Before this we got something like addrToByteArray 10 ... now we get addrToByteArray 10# ... (and the type of the non-exported function addrToByteArray changed to accommodate this)

Footnotes

1. The Lift ByteArray instance used to live in template-haskell but had to be moved to base when all the wired-in definitions from template-haskell were moved to ghc-internal. An orphan instance in template-haskell would have no longer been guaranteed to be in scope for users and could have led to breakage. See: this comment ↩

[Bodigrim]

How did it happen that instance Lift ByteArray was moved from template-haskell into base? Clearly this is a change of API (previoiusly base did not export an instance and now it does), but I don't recall a CLC proposal to do so.

(I'm not being formal for no reason here: dealing with TH AST all over the base is a maintenance burden, and this proposal is just a herald of what to come)

[tomjaguarpaw]

How did it happen that instance Lift ByteArray was moved from template-haskell into base?

It happened here: https://gitlab.haskell.org/ghc/ghc/-/commit/228dcae6a0efbe5289add5330c46f581780dd96c

I guess the point is that if Lift goes into ghc-internal then we have to put instance Lift ByteArray next to the definition of ByteArray (unless we want an orphan instance).

[TeofilC]

The lack of a CLC proposal was an oversight from me. As you can see it was quite a big MR and I overlooked that this could do with a proposal. I can write one up if you'd like. I'll definitely keep it in mind for the future in any case.

I'm not being formal for no reason here: dealing with TH AST all over the base is a maintenance burden, and this proposal is just a herald of what to come

Thats a good point. I think ByteArray is a bit of an exception though. Most instances are just derived ones.

[Bodigrim]

The lack of a CLC proposal was an oversight from me. As you can see it was quite a big MR and I overlooked that this could do with a proposal. I can write one up if you'd like.

Yes, please. Also make sure that there are changelog entries and @since annotations.

[TeofilC]

The lack of a CLC proposal was an oversight from me. As you can see it was quite a big MR and I overlooked that this could do with a proposal. I can write one up if you'd like.

Yes, please. Also make sure that there are changelog entries and @since annotations.

See: #287

[TeofilC]

I'm pausing this for now as I'm no longer sure if this is the best fix to the underlying issue. I'll revisit this in a few months