The issue here is that we do not cleanly reject shifts beyond the width of an integer; instead this might be rejected by an assertion in SafeInt when building in debug mode. We should reject this cleanly for all build types at runtime via an exception.

All these cases are expected to fail:

print uint8(0)<<8;
print uint16(0)<<16;
print uint32(0)<<32;
print uint64(0)<<64;

print int8(0)<<8;
print int16(0)<<16;
print int32(0)<<32;
print int64(0)<<64;

Even though there is deliberate handling for undefined shifts in SafeInt I filed dcleblanc/SafeInt#63 to see whether they could be convinced to switch to a throwing versions.

at runtime via an exception.

If the shift is by a const, it would be nice to get a compiler error at build time. Nice to have, but thought I'd comment :-)

I'm still wondering if << promoting to uint64 could be a feature, but that might just open a can of worms.

I'm still wondering if << promoting to uint64 could be a feature, but that might just open a can of worms.

Yeah, for one there are no automatic coercions from uint64 to smaller integer types so this quickly can become noisy. The other issue is that even with an uint64 we could still shift by at most 64, but we have no integer type with that range and would need a runtime check regardless.

Upstream has a PR open to move unsafe bitfshifts from assertion to exception. If this lands soon a bump would fix our issue (would still be good to add some tests); if it does not we could hook into the existing mechanism, see here.

I haven't audited which other places invoke SAFEINT_ASSERT, we might want to instrument it to throw as well (might require defining SAFEINT_REMOVE_NOTHROW.

Do we know if SafeInt is going to merge this?

I pinged upstream on whether they could merge their fix. I also tried to customize SAFEINT_ASSERT like documented so we throw instead of abort, but this applies everywhere and is a pretty blunt tool, see #1897.