reinterpret_cast from integer to pointer

[brtaylor]

I'm trying to compile for Teensy3.x with a makefile using a newer C++ compiler and the Wire library. I'm running into the error 'reinterpret_cast from integer to pointer' (previously it would have just been a warning). Specifically, it's complaining about SIM_SCGC4 on this line:

constexpr TwoWire::I2C_Hardware_t TwoWire::i2c0_hardware = {
	SIM_SCGC4, SIM_SCGC4_I2C0,
#if defined(__MKL26Z64__) || defined(__MK20DX128__) || defined(__MK20DX256__)
	18, 17, 255, 255, 255,
	2, 2, 0, 0, 0,
	19, 16, 255, 255, 255,
	2, 2, 0, 0, 0,
#elif defined(__MK64FX512__) || defined(__MK66FX1M0__)
	18, 17, 34, 8, 48,
	2, 2, 5, 7, 2,
	19, 16, 33, 7, 47,
	2, 2, 5, 7, 2,
#endif
	IRQ_I2C0
};

https://github.com/PaulStoffregen/W...3d5971b6fe4828d08c857df9/WireKinetis.cpp#L839

SIM_SCGC4 is defined in kinetis.h:

#define SIM_SCGC4		(*(volatile uint32_t *)0x40048034) // System Clock Gating Control Register 4

https://github.com/PaulStoffregen/c...c84e62b5c0e9243128416/teensy3/kinetis.h#L1193

My understanding is that reinterpret_cast is not allowed in constexpr since it may result in undefined behavior (in general, not in this case). I'm not sure what the "right" fix is in this case to make the compiler happy and avoid other potential issues. I suppose the easiest would be to remove constexpr in TwoWire::I2C_Hardware_t TwoWire::i2c0_hardware, but between this and the MAKE_CONST macro (https://github.com/PaulStoffregen/W...3d5971b6fe4828d08c857df9/WireKinetis.cpp#L907), it seems like a lot of work went into making everything evaluate at compile time, and I'd hate to lose that efficiency. Any thoughts?

Thanks!
Brian

 

[PaulStoffregen]

Which version of gcc is this?

Or in other words, if I want to (quickly & easily) reproduce this problem, what's the exact file I download & install?

 

[brtaylor]

I was trying with 10.2.1. As a test, I tried with 5.3.1 and did not encounter that error. Then testing with 6.2.1 and I got the reinterpret_cast error again.

 

[luni]

I stumbled over the same issue in one of my libraries a couple of months ago. It looks like the construct was never "legal" but gcc ignored the error in older versions. There are a lot of discussions at Stackoverflow about this. Some say that this restriction is just too restrictive. Here an example showing the weirdness of the problem:

#define KINETIS_I2C0		(*(KINETIS_I2C_t *)0x40066000)

//constexpr uintptr_t i2c0_addr = uintptr_t(&KINETIS_I2C0); // <- problematic line 
  constexpr uintptr_t i2c0_addr = uintptr_t(0x40066000);    // this works of course

void setup(){
    ((KINETIS_I2C_t*)i2c0_addr)->A2 = 0;  // use it
}

void loop(){ 
}

The second definition of i2c0_addr does exactly the same as the commented out version. To fix this you'd need to change the register defines from variables to numbers but I don't think this is worth it.
BTW: The T4.x code uses another pattern and complies without warnings...

In my classes I switched from constexpr pointers to static const which works. I never verified but I'm quite sure that the compiler is smart enough to interpret static const as compiletime constant.

Pattern:

class test
{
public:
    test();

protected:
    static const KINETIS_I2C_t* i2c0_addr;
};

const KINETIS_I2C_t* test::i2c0_addr = &KINETIS_I2C0;   // in some cpp file

 

[brtaylor]

Here's a very simple program that can compile directly under Linux, instead of needing to cross compile, that illustrates the issue:

#include <cstdint>

#define SIM_SCGC4		(*(volatile uint32_t *)0x40048034) // System Clock Gating Control Register 4

class TwoWire {
public:
	typedef struct {
		volatile uint32_t &clock_gate_register;
	} I2C_Hardware_t;
	static const I2C_Hardware_t i2c0_hardware;

};

// constexpr TwoWire::I2C_Hardware_t TwoWire::i2c0_hardware = {SIM_SCGC4}; // reinterpret cast error
const TwoWire::I2C_Hardware_t TwoWire::i2c0_hardware = {SIM_SCGC4};        // no error

int main() {}

Changing constexpr to const in the definition of i2c0_hardware fixes the issue; however, I'm not sure what the implications are of making that change.