Casting issue?

I don’t get it. Why does the code below output 255 rather than 4294967295 or 0xffffffff ?

class RGBA
  property r : UInt8
  property g : UInt8
  property b : UInt8
  property a : UInt8
  
  def initialize(@r : UInt8, @g : UInt8, @b : UInt8, @a : UInt8)
  end
  
  def to_hex() : UInt32
    ((@r << 24) | (@g << 16) | (@b << 8) | @a).to_u32()
  end
end

puts RGBA.new(255,255,255,255).to_hex()

I think it boils down to the implementation of #<<, specifically:

  • If count is greater than the number of bits of this integer, returns 0

So since you’re shifting by 24 on a UInt8 255_u8 << 24 # => 0.

EDIT: Seems there is a #unsafe_shl method, but doesn’t seem to be public. Is there a reason why?

Thanks !

Changing the code to this now works.

res : UInt32 = ((@r.to_u32 << 24) | (@g.to_u32 << 16) | (@b.to_u32 << 8) | @a).to_u32

That said, expressiveness does take a hit and that makes me miss the simpler days of C (if only it had classes).

Color32 ColorSDL_to_32(SDL_Color c) {
  return c.a | (c.b << 8) | (c.g << 16) | (c.r << 24);
}

IIUC, this only works because C implicitly promotes the operands of bitwise operations to int.

Crystal doesn’t do that. If you shift a UInt8, the operation performs on 8-bits and you get back a UInt8.

If you want different semantics, you need to explicitly cast to the desired type.

I think that makes a lot more sense. C’s behaviour is surprising and overall, less powerful.

IIUC, this only works because C implicitly promotes the operands of bitwise operations to int.

In C, left bit shifting – I just looked this up – is done in place and shifting by more bits than there are in the left operand is undefined behaviour. I had always assumed that this would be automatically managed by the compiler (working, say, on a duplicate stack entry of appropriate size when right expression exceeded bit count of left operand). One more foot gun.

C’s behaviour is surprising and overall, less powerful.

Let 's not go there.

1 Like

In C it depends on the implementation whether shifts are arithmetic or logic. Doesn’t matter for left shift, but for right shift of signed integer types it’s very relevant.

I ran into this sort of thing a LOT while making YunoSynth. The removal of the undefined behavior (even if most compilers seem to do the “promote to a larger int” thing) is, in my opinion, a good thing. But the resulting Crystal syntax was a bit of a pain to look at and read with all the explicit casts back and forth between numeric types in order to keep the exact same behavior as the original C code. In many ways it reminded me of when I implemented a YM2610 ADPCM-A codec in Go, where I ran into the same thing. Give and take, I guess, especially with more semi-low-level code

protected def opCalc(phase : UInt32, env : UInt32, pm : Int32) : Int32
    x : UInt32 = ((env.to_u64! << 3) +
                  @sinTab[(((phase & INV_FREQ_MASK) +
                            (pm.to_u64! < 15)) << (FREQ_SH)) & SIN_MASK]).to_u32!

    if x >= TL_TAB_LEN
        0
      else
        @tlTab.get!(x)
      end
end
delta = ((1 + (step.to_i32! << 1).abs) * voice.stepSize.to_i32!) >> 1
addr = (regs[0x85].to_u32! << 16) | (regs[0x84].to_u32! << 8) | @low.get!(ch)
loopPos = (regs[0x05].to_u32! << 16) | (regs[0x04].to_u32! << 8)

At least the behavior is well defined here ^_^

FWIW you could probably change the type restrictions of env and pm to UInt64 to avoid the explicit casts. The method can still be called with arguments of the current types due to autocasting.
Not sure if this API makes sense from the perspective of the domain model, though.

1 Like

opCalc is an internal method that does some of the FM synthesis calculations and is called a lot. About 30.5 million times for a 1-minute PC-88 song.

Both the example player for YunoSynth and the old Crystal version of Benben compile using -Dno_number_autocast to be more strict. Other downstream code may also use this flag, so I can’t really rely on automatic upcasting. In fact, a least one person mentioned to me that it didn’t work with the -Dno_number_autocast flag, which is why I made the change. So I either put the casts in opCalc, or in the methods that call it.

The code with the addr and loopPos are part of the SegaPCM emulation core. regs contains the emulated registers, which have to remain 8-bit unsigned integers. I suppose I could do some unsafe casting here, but that kinda defeats the purpose of safe code.

The delta bit is part of the QSound emulator, specifically the ADPCM decoding. step is an Int8 and, to match the hardware correctly, expands 8-bit ROM data to 16-bit, and then back to 8-bit, so stuff like that also has to remain.

None of this is really an issue, just an example of how the to_u32! or whatever gets to be an eyesore in some cases ^_^