Making template instantiation fail

[Jorgen Grahn]

It occurred to me to ask an actual question, rather than involving
myself in the usual arguments about swearing and so on ...

The other day I wrote this one:

/* Consume a 16-bit integer from a sequence of
* uint8_t or similar, big-endian.
*/
template <class Iter>
unsigned eat16(Iter& i)
{
unsigned n = *(i++);
n = n << 8 + *(i++);
return n;
}

I can accept if it fails silently when I feed in a
vector<unsigned>, list<unsigned long> iterator etc.

But it would also fail silently if I feed it a char*, or
a vector<char>::iterator -- and that's not acceptable to me.

Question: what's the canonical C11 way to make the template fail if
'i' doesn't iterate over a bunch of unsigned things? Seems like the
kind of thing people would do all the time.

I played with static_assert(), std::is_unsigned and decltype(*i), but
couldn't get it to work within reasonable time.

Should I (and this just occurred to me) use
std::iterator_traits<Iter>::value_type rather than decltype(*i)?

This seems to work:

/* Consume a 16-bit integer from a sequence of
* uint8_t or similar, big-endian.
*/
template <class Iter>
unsigned eat16(Iter& i)
{
typedef std::iterator_traits<Iter> traits;
static_assert(std::is_unsigned<typename traits::value_type>::value,
"unsigned, dammit!");

unsigned n = *(i++);
n <<= 8;
n += + *(i++);
return n;
}

I'm not used to doing these kinds of things. Usually I leave it up to
the caller to make sure the template argument is sane -- but this is
in a context where I use Boost, and I'm less sure of the types
involved than I normally am.

/Jorgen

--
// Jorgen Grahn <grahn@ Oo o. . .
\X/ snipabacken.se> O o .

[Paavo Helde]

If you just want to detect invalid usage, static_assert should be fine,
and the above should be kosher as far as the incoming iterator type is
defined correctly. OTOH, going over decltype should work also with
iterators which do not have proper traits defined. This should probably
look something like:

static_assert(std::is_unsigned<typename
std::remove_reference<decltype(*i)>::type>::value,
"unsigned, dammit!");

[Jorgen Grahn]

Thanks -- that looks like my first attempt, minus some bugs which
might explain why mine didn't work properly ...

[Alf P. Steinbach]

On 2/10/2016 9:03 PM, Jorgen Grahn wrote:
> It occurred to me to ask an actual question, rather than involving
> myself in the usual arguments about swearing and so on ...
>
> The other day I wrote this one:
>
> /* Consume a 16-bit integer from a sequence of
> * uint8_t or similar, big-endian.
> */
> template <class Iter>
> unsigned eat16(Iter& i)
> {
> unsigned n = *(i++);
> n = n << 8 + *(i++);
> return n;
> }
>
> I can accept if it fails silently when I feed in a
> vector<unsigned>, list<unsigned long> iterator etc.
>
> But it would also fail silently if I feed it a char*, or
> a vector<char>::iterator -- and that's not acceptable to me.

Yeah, for portable code you need to check CHAR_BIT (size of byte).

But except for the possibility of 16-bit char, with a strategic little
cast in there it will work nicely also for type `char`.

> Question: what's the canonical C11 way to make the template fail if
> 'i' doesn't iterate over a bunch of unsigned things? Seems like the
> kind of thing people would do all the time.

Depends on whether you want the template to be ignored where it's
unsuitable, or to yield an error.

For example, an `operator T` for a class should perhaps best be ignored
it T isn't suitable: we don't want willy-nilly unintended and
undesirable conversion chains to be invoked. And one way to make it
ignored is to use SFINAE (Substitution Failure Is Not An Error). The
standard library provides `enable_if` and since C++14 `enable_if_t` to
do SFINA-based checking in not overly complex ways.

Where you instead want an error, there are two main possibilities:

• `static_assert`, or some other construct that will fail at compile
time, or

• make it a non-template.

These are not mutually exclusive.

E.g., modulo notation I'd do it like this (includes a little bug-fix):

#include <limits.h> // CHAR_BIT

#define STATIC_ASSERT( e ) static_assert( e, #e )

using Byte = unsigned char;
int const bits_per_byte = CHAR_BIT;

auto eat16( Byte const*& p )
-> unsigned
{
STATIC_ASSERT( bits_per_byte == 8 );
unsigned n = Byte( *p++ );
n = (n << 8) + Byte( *p++ );
return n;
}

#include <stdio.h>

auto main() -> int
{
Byte const big_endian[] = {12345/256, 12356%256, 0, 42};
Byte const* p = big_endian;
printf( "%u", eat16( p ) );
printf( " %u\n", eat16( p ) );

}


> I played with static_assert(), std::is_unsigned and decltype(*i), but
> couldn't get it to work within reasonable time.
>
> Should I (and this just occurred to me) use
> std::iterator_traits<Iter>::value_type rather than decltype(*i)?

Yes. Yes! Yes!

At least for me it removes some doubt about whether the correct type is
referred to.

Well, it might be a very subjective associative thing, because I've been
burned a few times by `decltype`, where the fix was traits.

> This seems to work:
>
> /* Consume a 16-bit integer from a sequence of
> * uint8_t or similar, big-endian.
> */
> template <class Iter>
> unsigned eat16(Iter& i)
> {
> typedef std::iterator_traits<Iter> traits;

A C++11 `using` declaration is more clear to me :).

> static_assert(std::is_unsigned<typename traits::value_type>::value,
> "unsigned, dammit!");

To reduce the notational overhead, since C++17 there is the helper
template variable `is_unsigned_v`. However, I prefer to just define a
helper template function `is_unsigned_`. Also, I generally use a macro
wrapper for `static_assert` rather then the single-argument C++17 form.

I.e., modulo a macro prefix,

STATIC_ASSERT( is_unsigned_<typename traits::value_type>() );

> unsigned n = *(i++);
> n <<= 8;
> n += + *(i++);
> return n;
> }
>
> I'm not used to doing these kinds of things. Usually I leave it up to
> the caller to make sure the template argument is sane -- but this is
> in a context where I use Boost, and I'm less sure of the types
> involved than I normally am.

Well, again (maybe you just skipped to the end here :) ) I suggest
simply making this a non-template, and using `static_assert` for the
8-bits-per-byte assumption.

But for the template version, I'd check that the value type is an 8-bit
type, nothing more:

STATIC_ASSERT( bits_per_byte == 8 );
STATIC_ASSERT( sizeof( typename traits::value_type ) == 1 );

And then I'd cast each of the two values to `Byte`, so it would work
also with `char`, or indeed `signed char`. The last one isn't so
formally guaranteed, because on a non-two's complement machine there is
an invalid or redundant bit pattern for `signed char`. But AFAIK there
are no extant such machines, and absolutely none with C++ compiler.

I.e., my preferred solution would be

auto eat16( Byte const*& p )
-> unsigned
{
STATIC_ASSERT( bits_per_byte == 8 );
unsigned n = *p++;
n = (n << 8) + *p++;
return n;
}

But if a template is really required, if one must deal with additional
byte-sized types (`char` and `unsigned char` are the only built-in type
possibilities, but it might be an `enum`), then

template< class It >
auto eat16( It& it )
-> unsigned
{
using Traits = std::iterator_traits<It>;
STATIC_ASSERT( bits_per_byte == 8 );
STATIC_ASSERT( sizeof( typename Traits::value_type ) == 1 );

unsigned n = Byte( *it++ );
n = (n << 8) + Byte( *it++ );
return n;
}


Cheers & hth.,

- Alf

[Alf P. Steinbach]

On 2/11/2016 7:37 AM, Alf P. Steinbach wrote:
> On 2/10/2016 9:03 PM, Jorgen Grahn wrote:
>> It occurred to me to ask an actual question, rather than involving
>> myself in the usual arguments about swearing and so on ...
>>

Oh, sorry about the redundants casts in the non-template function. I
edited the wrong file and it propagated unnoticed all the way to Usenet.

Cheers!,

- Alf

[Jorgen Grahn]

On Thu, 2016-02-11, Alf P. Steinbach wrote:
> On 2/10/2016 9:03 PM, Jorgen Grahn wrote:
>> It occurred to me to ask an actual question, rather than involving
>> myself in the usual arguments about swearing and so on ...
>>
>> The other day I wrote this one:
>>
>> /* Consume a 16-bit integer from a sequence of
>> * uint8_t or similar, big-endian.
>> */
>> template <class Iter>
>> unsigned eat16(Iter& i)
>> {
>> unsigned n = *(i++);
>> n = n << 8 + *(i++);
>> return n;
>> }
>>
>> I can accept if it fails silently when I feed in a
>> vector<unsigned>, list<unsigned long> iterator etc.
>>
>> But it would also fail silently if I feed it a char*, or
>> a vector<char>::iterator -- and that's not acceptable to me.
>
> Yeah, for portable code you need to check CHAR_BIT (size of byte).
>
> But except for the possibility of 16-bit char, with a strategic little
> cast in there it will work nicely also for type `char`.

I was going to write "no thanks; I don't want that. I want to see the
stream of octets from the socket as unsigned things" ... but as it
turns out, this part of Boost.Asio really wants to give me data in
terms of char -- there's a std::streambuf<<char> somewhere in the
background. Seems like a silly limitation, but never mind; Asio does
a lot of other things which are useful to me.

So I ended up adding a static_cast, and lifting the "unsigned" restriction.

...

>> typedef std::iterator_traits<Iter> traits;
>
> A C++11 `using` declaration is more clear to me :).

I'm slowly starting to use those, but so far it's only a tool to avoid
getting comments like the one above :-)