Skip to content
This repository was archived by the owner on May 10, 2024. It is now read-only.

PARQUET-551:Handle compiler warnings due to disabled DCHECKs in relea… #132

Closed
majetideepak wants to merge 2 commits into from
Closed

PARQUET-551:Handle compiler warnings due to disabled DCHECKs in relea… #132

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6fdd3c4
PARQUET-551:Handle compiler warnings due to disabled DCHECKs in relea…
Jul 5, 2016
3b1212a
addressed comments
Jul 6, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,7 @@ endif ()

message(STATUS "Build Type: ${CMAKE_BUILD_TYPE}")

SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall")
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall -fno-strict-aliasing")

Copy link
Author

@majetideepak majetideepak Jul 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are breaking strict aliasing at
reader-internal.cc:282: uint32_t metadata_len = *reinterpret_cast<uint32_t*>(footer_buffer);

Copy link
Member

@wesm wesm Jul 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Author

@majetideepak majetideepak Jul 6, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. We can use the __restrict attribute if needed.

if (PARQUET_USE_SSE)
SET(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -march=native")
Expand Down
8 changes: 8 additions & 0 deletions src/parquet/util/logging.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ namespace parquet {

#define PARQUET_LOG_INTERNAL(level) parquet::internal::CerrLog(level)
#define PARQUET_LOG(level) PARQUET_LOG_INTERNAL(PARQUET_##level)
#define PARQUET_IGNORE_EXPR(expr) ((void)(expr));

#define PARQUET_CHECK(condition) \
(condition) ? 0 : PARQUET_LOG(FATAL) << "Check failed: " #condition " "
Expand All @@ -44,24 +45,31 @@ namespace parquet {
#define PARQUET_DFATAL PARQUET_WARNING

#define DCHECK(condition) \
PARQUET_IGNORE_EXPR(condition)\
while (false) \
parquet::internal::NullLog()
#define DCHECK_EQ(val1, val2) \
PARQUET_IGNORE_EXPR(val1) \
while (false) \
parquet::internal::NullLog()
#define DCHECK_NE(val1, val2) \
PARQUET_IGNORE_EXPR(val1) \
while (false) \
parquet::internal::NullLog()
#define DCHECK_LE(val1, val2) \
PARQUET_IGNORE_EXPR(val1) \
while (false) \
parquet::internal::NullLog()
#define DCHECK_LT(val1, val2) \
PARQUET_IGNORE_EXPR(val1) \
while (false) \
parquet::internal::NullLog()
#define DCHECK_GE(val1, val2) \
PARQUET_IGNORE_EXPR(val1) \
while (false) \
parquet::internal::NullLog()
#define DCHECK_GT(val1, val2) \
PARQUET_IGNORE_EXPR(val1) \
while (false) \
parquet::internal::NullLog()

Expand Down