Modify compiler flags to suppress variable length array error#2052
Conversation
Added -Wno-error=vla to COMMON_CXXFLAGS and COMMON_TESTING_FLAGS to prevent fatal warnings for variable-length arrays.
|
In the future I think we should move to |
|
I've created #2053 to track that. TBH I'd assumed extracting from argc/argv would be a bit of a pain, but it doesn't look too bad thankfully. |
aroffringa
left a comment
There was a problem hiding this comment.
Flag changes look fine. I added one suggestion for the cast for which you use NOLINT.
| // Changing these to a static_cast breaks the tests at least | ||
| tr.tx_buf = (uint64_t) tx_buf; // NOLINT(readability/casting) | ||
| tr.rx_buf = (uint64_t) rx_buf; // NOLINT(readability/casting) |
There was a problem hiding this comment.
I don't think a static_cast should even compile here; the C cast will behave like a reinterpret_cast in this case (but is more permissive). I think best would be to do an explicit reinterpret_cast:
| // Changing these to a static_cast breaks the tests at least | |
| tr.tx_buf = (uint64_t) tx_buf; // NOLINT(readability/casting) | |
| tr.rx_buf = (uint64_t) rx_buf; // NOLINT(readability/casting) | |
| tr.tx_buf = reinterpret_cast<uint64_t>(tx_buf); | |
| tr.rx_buf = reinterpret_cast<uint64_t>(rx_buf); |
There was a problem hiding this comment.
My memory is clearly rubbish:
- There aren't any tests
- It did indeed fail to compile
I tried your reinterpret_cast locally and it does at least compile, but there aren't any tests so I'll have to take it on faith...
@FloEdelmann if you've still got a test setup for this, please feel free to give it a go and let us know, but we'll merge as is.
There was a problem hiding this comment.
Applied, but just re-opened pending any possible testing.
Not yet tested with SPI DMX input. Co-authored-by: André Offringa <offringa@gmail.com>
f684f3e
into
OpenLightingProject:master
Added -Wno-error=vla to COMMON_CXXFLAGS and COMMON_TESTING_FLAGS to prevent fatal warnings for variable-length arrays.
Plus other minor tidying.