Implemented TH splices for validated ByteString literals #712

vdukhovni · 2025-08-16T06:53:37Z

thLiteral    :: Quote m => String -> Code m ByteString
thHexLiteral :: Quote m => String -> Code m ByteString

The former rejects inputs with non-octet code points above 0xFF. The latter rejects odd-length inputs or inputs with characters other than non-hexadecimal digits.

vdukhovni · 2025-08-16T09:13:18Z

@Bodigrim I don't understand what's going on with CI. Any help appreciated.

Bodigrim · 2025-08-16T09:31:04Z

@vdukhovni no idea tbh. Maybe something changed under the hood, either in runner images or in the haskell action.

vdukhovni · 2025-08-16T11:57:46Z

I have very little experience tuning GitHub CI. Any chance someone can help?

Bodigrim · 2025-08-16T18:14:22Z

It seems it was an intermittent failure with https://github.com/haskell-actions/setup. I don't think you need to touch CI setup in this PR.

vdukhovni · 2025-08-17T00:28:54Z

Thanks, indeed most of the problems appear to have been transient. I reverted the CI changes, and the only failure so far is with OpenBSD, which reports:
⚠️ Not enough compute credits to prioritize tasks!

Otherwise, no issues. So I think I'm done, unless you'd prefer to name the two functions differently. The names thLiteral and thHexLiteral were a best effort choice at the time, but one can probably make a case for other choices if these don't appeal.

vdukhovni · 2025-08-21T03:10:26Z

Review request: @hsyl20 @Bodigrim @clyring

hsyl20

LGTM

I would prefer more explicit names: something like literalFromAscii (or literalFromChar8) and literalFromHex

Data/ByteString/Internal/Type.hs

vdukhovni · 2025-08-21T10:43:03Z

LGTM

I would prefer more explicit names: something like literalFromAscii (or literalFromChar8) and literalFromHex

Many thanks for the prompt review! I'm about to push a fixup for all the nits, and what remains then is to reach consensus on the splice names. Of the above I prefer literalFromChar8 over literalFromAscii and have no objections to literalFromHex. I take it you don't see any benefit from including a th prefix to make it clear these are splices rather than directly usable functions?

hsyl20 · 2025-08-21T12:36:47Z

I take it you don't see any benefit from including a th prefix to make it clear these are splices rather than directly usable functions?

Yes the type and literal already convey that imo.

Data/ByteString/Internal/Type.hs

vdukhovni · 2025-08-24T14:53:17Z

@hsyl20, @Bodigrim Many thanks for the reviews, much appreciated. If at some point you find some more review cycles, I've revived, rebased and improved #569, so reviews there would also be great.

vdukhovni · 2025-09-09T14:47:01Z

@hsyl20 @clyring @Bodigrim I believe this is done. Please let me know if anything is missing.

clyring · 2025-09-14T02:35:18Z

Data/ByteString/Internal/Type.hs

-import Data.Bits                ((.&.))
+import Data.Bits                ((.|.), (.&.), complement, shiftL)
 import Data.Char                (ord)
+import Data.Foldable            (foldr')


The unqualified foldr' briefly confused me. (Actually, why are these quote-generators defined in D.B.Internal.Type instead of the exposed Data.ByteString?)

clyring · 2025-09-14T02:41:34Z

Data/ByteString/Internal/Type.hs

+-- > ehloCmd :: ByteString
+-- > ehloCmd = $$(literalFromChar8 "EHLO")
+--
+literalFromChar8 :: String -> THLift ByteString


I respect the idea of using a type synonym here, but (with recent GHCs) it expands to a higher-rank type, which is different from the type we would otherwise have given it.

The alternative to just repeat the whole function with a different type signature, or use CPP.
Is that necessary? I've not run into any issues with using the splice in GHC 9.12. Is there a problem with 9.14?

Take a look at how template-haskell-lift shims around this: https://gitlab.haskell.org/ghc/template-haskell-lift/-/blob/main/src/Language/Haskell/TH/Lift.hs?ref_type=heads#L40

It's currently unreleased, but I'll try to publish this library in the next few weeks.

It means we can just write Qoute m => ... -> Code m a and it will turn into the correct thing for all versions of GHC without higher-rank types, etc.

I've adopted that approach.

clyring · 2025-09-14T02:45:05Z

Data/ByteString/Internal/Type.hs

+literalFromChar8 "" = [||empty||]
+literalFromChar8 s = case foldr' op (Octets 0 []) s of
+    Octets n ws -> liftTyped (unsafePackLenBytes n ws)
+    Hichar i w  -> liftCode $ fail $ "non-octet character '\\" ++


@TeofilC Would this liftCode $ fail $ ... stuff require any adjustments to your template-haskell-lift plans?

Thanks for the headsup. This should be fine.

clyring · 2025-09-14T03:00:14Z

Data/ByteString/Internal/Type.hs

+    op :: Char -> S2W -> S2W
+    op (fromIntegral . fromEnum -> !(w :: Word)) acc
+        | w <= 0xff = case acc of
+            Octets i ws -> Octets (i + 1) (fromIntegral w : ws)


(i+1) can overflow.

That would require that the input string length be greater than maxBound @Int, and the consequence is just an incorrect report of the problem offset. I suspect this does not warrant work-arounds.

thLiteral :: Quote m => String -> Code m ByteString thHexLiteral :: Quote m => String -> Code m ByteString The former rejects inputs with non-octet code points above 0xFF. The latter rejects odd-length inputs or inputs with characters other than non-hexadecimal digits.

Henry Sylvain review fixups

Andrew Lelechenko review fixes

…literals tweak TH type signatures

vdukhovni force-pushed the th-splices branch 2 times, most recently from 70f1a6a to b53c921 Compare August 16, 2025 08:26

vdukhovni force-pushed the th-splices branch from b53c921 to a6b5c82 Compare August 17, 2025 00:13

hsyl20 approved these changes Aug 21, 2025

View reviewed changes

Data/ByteString/Internal/Type.hs Outdated Show resolved Hide resolved

Data/ByteString/Internal/Type.hs Outdated Show resolved Hide resolved

Data/ByteString/Internal/Type.hs Outdated Show resolved Hide resolved

vdukhovni force-pushed the th-splices branch 2 times, most recently from c5906d9 to 28a0cb6 Compare August 21, 2025 14:09

Bodigrim reviewed Aug 21, 2025

View reviewed changes

Data/ByteString/Internal/Type.hs Show resolved Hide resolved

Data/ByteString/Internal/Type.hs Show resolved Hide resolved

Bodigrim reviewed Aug 22, 2025

View reviewed changes

Data/ByteString/Internal/Type.hs Show resolved Hide resolved

Data/ByteString/Internal/Type.hs Outdated Show resolved Hide resolved

Data/ByteString/Internal/Type.hs Outdated Show resolved Hide resolved

vdukhovni force-pushed the th-splices branch 2 times, most recently from 83cf073 to 2f5671a Compare August 23, 2025 03:11

Bodigrim approved these changes Aug 24, 2025

View reviewed changes

Bodigrim requested a review from clyring August 24, 2025 10:28

clyring reviewed Sep 14, 2025

View reviewed changes

hs-viktor added 3 commits September 14, 2025 20:16

fixup! Implemented TH splices for validated ByteString literals

86d94ba

Henry Sylvain review fixups

fixup! fixup! Implemented TH splices for validated ByteString literals

d4d62f0

Andrew Lelechenko review fixes

vdukhovni force-pushed the th-splices branch from 3626e62 to 8b3189d Compare September 14, 2025 10:19

fixup! fixup! fixup! Implemented TH splices for validated ByteString …

d8c2d02

…literals tweak TH type signatures

vdukhovni force-pushed the th-splices branch from 8b3189d to d8c2d02 Compare September 14, 2025 10:53

Implemented TH splices for validated ByteString literals #712

Are you sure you want to change the base?

Implemented TH splices for validated ByteString literals #712

Uh oh!

Conversation

vdukhovni commented Aug 16, 2025

Uh oh!

vdukhovni commented Aug 16, 2025

Uh oh!

Bodigrim commented Aug 16, 2025

Uh oh!

vdukhovni commented Aug 16, 2025

Uh oh!

Bodigrim commented Aug 16, 2025

Uh oh!

vdukhovni commented Aug 17, 2025

Uh oh!

vdukhovni commented Aug 21, 2025

Uh oh!

hsyl20 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vdukhovni commented Aug 21, 2025

Uh oh!

hsyl20 commented Aug 21, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

vdukhovni commented Aug 24, 2025

Uh oh!

vdukhovni commented Sep 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!