Skip to content

Add VarInt support#263

Merged
fboemer merged 1 commit intomainfrom
fboemer/varint
Jan 14, 2026
Merged

Add VarInt support#263
fboemer merged 1 commit intomainfrom
fboemer/varint

Conversation

@fboemer
Copy link
Copy Markdown
Contributor

@fboemer fboemer commented Jan 14, 2026

Adds VarInt support

@fboemer fboemer marked this pull request as ready for review January 14, 2026 19:50
Comment thread Sources/PrivateInformationRetrieval/IndexPir/VarInt.swift Outdated
Comment thread Sources/PrivateInformationRetrieval/IndexPir/VarInt.swift Outdated
@fboemer fboemer force-pushed the fboemer/varint branch 3 times, most recently from 44ee4e6 to e0e903f Compare January 14, 2026 20:19
@fboemer fboemer requested a review from karulont January 14, 2026 20:20
Copy link
Copy Markdown
Contributor

@karulont karulont left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try with changing the intermediate type from UInt64 to T in the decoding?

Comment thread Sources/PrivateInformationRetrieval/IndexPir/VarInt.swift
Comment thread Sources/PrivateInformationRetrieval/IndexPir/VarInt.swift
Comment thread Sources/PrivateInformationRetrieval/IndexPir/VarInt.swift
@fboemer
Copy link
Copy Markdown
Contributor Author

fboemer commented Jan 14, 2026

Did you try with changing the intermediate type from UInt64 to T in the decoding?

We'd need some extra logic to check for overflow shifting & T == UInt8 (typeSpecificOverflow was failing otherwerise)
This would work, but it's more complicated.
So I think it's better to leave as is.

    /// Decode a varint from a byte array
    /// Returns (decoded value, number of bytes consumed)
    static func decode<T: FixedWidthInteger & UnsignedInteger>(_ bytes: some Collection<UInt8>) throws
        -> (decoded: T, bytesConsumed: Int)
    {
        precondition(T.bitWidth >= 7)
        var result: T = 0
        var shift = 0
        var index = 0

        while index < bytes.count {
            let byte = bytes[bytes.index(bytes.startIndex, offsetBy: index)]
            index += 1

            // Check for overflow (varint can be at most 10 bytes for 64-bit)
            if shift >= T.bitWidth {
                throw VarIntError.overflow
            }

            // Check if adding these bits would overflow
            let bitsToAdd = T(byte & 0x7F)
            
            // Check if shifting would cause overflow
            if shift > 0 && (T.bitWidth - shift) < 7 {
                // If we're trying to shift beyond the bit width, check if higher bits are set
                if bitsToAdd >> (T.bitWidth - shift) > 0 {
                    throw VarIntError.overflow
                }
            }
            
            // Add the 7 data bits
            let shiftedBits = bitsToAdd << shift
            
            // Check for overflow when adding to result
            if T.max - result < shiftedBits {
                throw VarIntError.overflow
            }
            
            result |= shiftedBits

            // If continuation bit is not set, we're done
            if (byte & 0x80) == 0 {
                return (result, index)
            }

            shift += 7
        }

        // Ran out of bytes while continuation bit was set
        throw VarIntError.truncated
    }

@karulont
Copy link
Copy Markdown
Contributor

Did you try with changing the intermediate type from UInt64 to T in the decoding?

We'd need some extra logic to check for overflow shifting & T == UInt8 (typeSpecificOverflow was failing otherwerise) This would work, but it's more complicated. So I think it's better to leave as is.

Yeah, fine to leave at is. Just a note: that it will not work for types larger than UInt64.

@fboemer
Copy link
Copy Markdown
Contributor Author

fboemer commented Jan 14, 2026

Did you try with changing the intermediate type from UInt64 to T in the decoding?

We'd need some extra logic to check for overflow shifting & T == UInt8 (typeSpecificOverflow was failing otherwerise) This would work, but it's more complicated. So I think it's better to leave as is.

Yeah, fine to leave at is. Just a note: that it will not work for types larger than UInt64.

Good point, I'll add a precondition check for this

@fboemer fboemer merged commit 3a0555c into main Jan 14, 2026
18 checks passed
@fboemer fboemer deleted the fboemer/varint branch January 14, 2026 21:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants