Converti genericamente i byte in numeri interi

Question

Volevo sapere se c'era un modo per renderlo più versatile o più breve/più semplice. Ecco il codice:

// Take only std::byte parameter and return an unsigned integral
constexpr auto bytes_to_uint(std::same_as<std::byte> auto... bytes) -> std::unsigned_integral auto {
    constexpr auto N = sizeof...(bytes);

    // Integral types large enough to hold N bytes
    using types = std::tuple<
        std::uint8_t,
        std::uint16_t,
        std::uint32_t,
        std::uint32_t,
        std::uint64_t,
        std::uint64_t,
        std::uint64_t,
        std::uint64_t
    >;
    
    using result = std::tuple_element_t<N, types>;
    
    return [&]<std::size_t... S>(std::index_sequence<S...>) {
        // Accumulate the part of the number using the bitwise or operator for each bytes
        return ((static_cast<result>(bytes) << CHAR_BIT * (N - S - 1)) | ... );
    }(std::make_index_sequence<N>{});
}

È pensato per essere usato in questo modo:

bytes_to_uint(std::byte{0xaa}, std::byte{0xbb}); // std::uint16_t: 0xaabb

bytes_to_uint(
    std::byte{0x11},
    std::byte{0x22},
    std::byte{0x33},
    std::byte{0x44}
); // std::uint32_t: 0x11223344

Answer 1

La firma della funzione mi sembra difficile da leggere, grazie al vincolo std::same_as<std::byte> auto...e al "tipo di ritorno" finale std::unsigned_integral auto. Potrei piuttosto scrivere qualcosa del genere

constexpr auto bytes_to_uint(std::initializer_list<std::byte> bytes) {

...Ah, ma allora non potresti usarlo bytes.size()come espressione costante; Vedo. Quindi penserei di scrivere un set di sovraccarico, come questo:

constexpr std::uint8_t bytes_to_uint(std::byte a) {
    return a;
}
constexpr std::uint16_t bytes_to_uint(std::byte a, std::byte b) {
    return (a << 8) | b;
}
constexpr std::uint32_t bytes_to_uint(std::byte a, std::byte b, std::byte c, std::byte d) {
    return (a << 24) | (b << 16) | (c << 8) | d;
}

Ma immagino che questo sia disordinato perché hai bisogno di 16 diversi sovraccarichi. Non puoi nemmeno usare gli argomenti delle funzioni predefinite, perché vuoi bytes_to_uint(a,b,c)essere uguale a bytes_to_uint(0,a,b,c)e non bytes_to_uint(a,b,c,0). Certo che potresti ancora scrivere

#define B std::byte
constexpr std::uint8_t bytes_to_uint(B a)
  { return bytes_to_uint(0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,a); }
constexpr std::uint16_t bytes_to_uint(B a, B b)
  { return bytes_to_uint(0,0,0,0,0,0,0,0,0,0,0,0,0,0,a,b); }
constexpr std::uint32_t bytes_to_uint(B a, B b, B c)
  { return bytes_to_uint(0,0,0,0,0,0,0,0,0,0,0,0,0,a,b,c); }
constexpr std::uint32_t bytes_to_uint(B a, B b, B c, B d)
  { return bytes_to_uint(0,0,0,0,0,0,0,0,0,0,0,0,a,b,c,d); }
constexpr std::uint64_t bytes_to_uint(B a, B b, B c, B d, B e)
  { return bytes_to_uint(0,0,0,0,0,0,0,0,0,0,0,a,b,c,d,e); }
[...22 more lines...]
#undef B

ma scommetto che non vuoi farlo. Va bene, continuiamo con il modello che hai fatto.

---

using result = std::tuple_element_t<N, types>;

Preferirei vedere questo typedef dipendente utilizzare CamelCase(come un parametro di modello) o suffixedwith_type(come un typedef di un membro STL). Chiamarlo lo resultfa sembrare troppo simile a una variabile e rende difficile scegliere l'unico posto in cui lo usi.

Invece di spendere 13 righe e una <tuple>dipendenza, preferirei semplicemente farlo

using ResultType = std::conditional_t<
    (N == 1), std::uint8_t, std::conditional_t<
    (N == 2), std::uint16_t, std::conditional_t<
    (N <= 4), std::uint32_t, std::uint64_t>>>;

Il che mi ricorda che hai bisogno di qualcosa del genere

static_assert(N <= 16);

per impedirti di provare a gestire un elenco di argomenti di 17 byte o più.

E non me ne sono nemmeno accorto fino a quando non l'ho provato in Godbolt, ma qui hai un bug di uno! Volevi dire tuple_element_t<N-1, types>. Ricorda che l'indicizzazione inizia sempre da zero (ovunque tranne <regex>).

---

Se non ti piace conditional_t, un'altra opzione è usare i normali vecchi ifs. Scomponi il calcolo mentre è ancora parametrizzato su ResultType, quindi usa una if-elsecatena per decidere il tipo giusto da collegare per ResultTypedopo:

auto do_it = [&]<class ResultType, std::size_t... S>(ResultType, std::index_sequence<S...>) {
    return ((static_cast<ResultType>(bytes) << CHAR_BIT * (N - S - 1)) | ... );
};
if constexpr (N == 1) {
    return do_it(std::uint8_t{}, std::make_index_sequence<N>{});
} else if constexpr (N == 2) {
    return do_it(std::uint16_t{}, std::make_index_sequence<N>{});
} else if constexpr (N <= 4) {
    return do_it(std::uint32_t{}, std::make_index_sequence<N>{});
} else if constexpr (N <= 8) {
    return do_it(std::uint64_t{}, std::make_index_sequence<N>{});
}

---

Ancora meglio, consenti al compilatore di fare i calcoli alla sua larghezza di bit preferita (che è 64 bit su tutte le piattaforme desktop che mi interessano) e quindi troncalo alla fine. Questo produce un codegen simile e si legge ancora meglio:

std::uint64_t result = [&]<std::size_t... S>( std::index_sequence<S...>) {
    return ((static_cast<std::uint64_t>(bytes) << CHAR_BIT * (N - S - 1)) | ... );
}(std::make_index_sequence<N>{});
if constexpr (N == 1) {
    return std::uint8_t(result);
} else if constexpr (N == 2) {
    return std::uint16_t(result);
} else if constexpr (N <= 4) {
    return std::uint32_t(result);
} else {
    return std::uint64_t(result);
}

---

Hai un altro bug quando N == 1(oltre al bug off-by-one). Quando N == 1, l'espressione fold non ha alcuna |operazione, quindi è solo uno uint8_tspostamento di zero. Quell'espressione di spostamento ha tipo int. Che non è un tipo integrale senza segno. Quindi il tuo vincolo di tipo di ritorno fallisce!

Questo è solo un altro motivo per fare tutti i calcoli prima in uint64_t, e poi abbassare a uint8_tdestra prima di tornare, come mostrato nel mio ultimo esempio sopra.

Scrivere qualsiasi test case avrebbe rilevato sia questo bug che il bug off-by-one. I casi di test sono sempre importanti! Soprattutto quando hai intenzione di pubblicare il codice per la revisione pubblica. (O per la revisione da parte dei colleghi, se è per questo.)

---

Infine, consiglio le parentesi per chiarire la precedenza di x << CHAR_BIT * y. Nel contesto è ovvio quale ti aspettavi fosse la precedenza; ma come lettore, non sono sicuro che tu abbia ragione. Metti le parentesi in modo che non ci debba pensare nemmeno per un secondo.

Tuttavia, in questo contesto, questo è un punto molto secondario, perché chiaramente non ti aspetti che nessuno legga l'espressione ((static_cast<result>(bytes) << CHAR_BIT * (N - S - 1)) | ... ). È una riga di codice "fidati di me".

È anche sciocco fingere che CHAR_BITsia rilevante qui. Questo codice esplode in modo spettacolare se CHAR_BITè qualcosa di diverso da 8. Quindi scrivi e basta 8; e se sei obbligato a lavorare in un riferimento a CHAR_BIT, fallo scrivendo

static_assert(CHAR_BIT == 8);

nella parte superiore della funzione.