PARQUET-551:Handle compiler warnings due to disabled DCHECKs in relea… #132
Conversation
|
|
||
| SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall") | ||
| SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall -Wno-strict-aliasing") | ||
|
|
There was a problem hiding this comment.
We are breaking strict aliasing at
reader-internal.cc:282: uint32_t metadata_len = *reinterpret_cast<uint32_t*>(footer_buffer);
There was a problem hiding this comment.
Looks like we should be compiling with -fno-strict-aliasing; I think this is considered a best practice in general to avoid odd edge cases, even though it may prevent certain compiler optimizations
There was a problem hiding this comment.
I agree. We can use the __restrict attribute if needed.
|
But this now essentially means that the code passed as |
|
I believe the compiler optimizes the expression away since the result is unused. |
|
Yeah! The compiler will optimize the expression since the result is unused. |
src/parquet/util/logging.h
Outdated
| while (false) \ | ||
| parquet::internal::NullLog() | ||
| #define DCHECK_GT(val1, val2) \ | ||
| ((void)(val1)); \ |
There was a problem hiding this comment.
Maybe put this in a PARQUET_IGNORE_EXPR macro or something to make it more clear?
|
Only thing I may be worried about is if the compiler manages to optimize the calls to |
|
This is a good point. I just debugged the |
|
Nice, then a clear +1 from my side. |
|
+1, thank you |
3 participants
…se builds