[ 
https://issues.apache.org/jira/browse/ARROW-1615?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16181884#comment-16181884
 ] 

Wes McKinney commented on ARROW-1615:
-------------------------------------

I agree that this should be better documented so that developers know what 
standard of warning cleanliness is being enforced in Travis CI. We should 
indicate the set of additional flags that each kind of compiler (gcc, clang, 
etc.) should pass as part of development. 

I am not sure we should require _all_ builds on all platforms to compile with 
these flags, as it may result in unnecessary failures due to compiler warnings 
(although if you do not build with {{-Werror}} then maybe the compiler warnings 
are OK).

> CXX flags for development more permissive than Travis CI builds
> ---------------------------------------------------------------
>
>                 Key: ARROW-1615
>                 URL: https://issues.apache.org/jira/browse/ARROW-1615
>             Project: Apache Arrow
>          Issue Type: Bug
>            Reporter: Rene Sugar
>             Fix For: 0.8.0
>
>         Attachments: ARROW-TravisCI-CLang-CXXflags.patch
>
>
> arrow/cpp/cmake_modules/SetupCxxFlags.cmake uses more permissive CXX flags 
> than the Travis CI integration build.
> A project that builds without warnings or errors on a development machine can 
> break during a Travis build.
> Those using CLang and GNU may not see the same warnings before a check-in.
> Travis CI:
> -Wall -std=c11 -Wconversion -Wno-sign-conversion -Werror -std=gnu+11
> Arrow:
> -Wall -std=c++11 -std=gnu++11
> Parquet-CPP:
> -Wall -std=gnu++11
> With only CLang installed, g++ is a wrapper for CLang.
> To make it less likely that a Clang build will break when checked-in, CLang 
> has a -Weverything flag from which specific warnings can be disabled and 
> enabled.
> http://amattn.com/p/better_apps_clang_weverything_or_wall_is_a_lie.html
> https://gcc.gnu.org/wiki/ClangDiagnosticsComparison
> https://github.com/Barro/compiler-warnings
> However, the warning that caused this particular build failure is specific to 
> GNU gcc.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=40752
> https://stackoverflow.com/questions/12641387/is-gccs-wconversion-incompatible-with-the-use-of-compound-assignment-etc
> CLang has a "-Weverything" flag. Individual warnings were disabled until the 
> current Arrow source code compiled without errors.
> Individual warnings can be enabled when using the CLang compiler to see if 
> there is anything that needs to be fixed.
> ```
> elseif((CMAKE_CXX_COMPILER_ID STREQUAL "Clang") OR (CMAKE_CXX_COMPILER_ID 
> STREQUAL "AppleClang"))
>   # NOTE: -Wconversion does not return the same warnings on CLang and GNU 
> (build can still fail even when enabled)
>   set(CXX_COMMON_FLAGS "-Weverything -Wno-c++98-compat 
> -Wno-c++98-compat-pedantic -Wno-deprecated -Wno-weak-vtables -Wno-padded 
> -Wno-unused-parameter -Wno-undef -Wno-documentation-deprecated-sync 
> -Wno-reserved-id-macro -Wno-shadow -Wno-switch-enum -Wno-documentation 
> -Wno-exit-time-destructors -Wno-global-constructors 
> -Wno-weak-template-vtables -Wno-undefined-reinterpret-cast 
> -Wno-implicit-fallthrough -Wno-old-style-cast -Wno-unreachable-code-return 
> -Wno-float-equal -Wno-missing-prototypes -Wno-double-promotion 
> -Wno-non-virtual-dtor -Wno-unused-macros -Wno-covered-switch-default 
> -Wno-unreachable-code-break -Wno-extra-semi -Wno-range-loop-analysis 
> -Wno-shift-sign-overflow -Wno-used-but-marked-unused 
> -Wno-missing-variable-declarations -Wno-gnu-zero-variadic-macro-arguments 
> -Wconversion -Wno-sign-conversion -Wc++11-narrowing -Wnarrowing -Werror 
> -std=c++11")
> else()
>   # Travis CI builds use these flags
>   set(CXX_COMMON_FLAGS "-Wall -Wconversion -Wno-sign-conversion -Werror 
> -std=c++11")
> endif()
> ```
> ```
> In file included from 
> /home/travis/build/apache/arrow/cpp/src/arrow/buffer.h:29:0,
> from /home/travis/build/apache/arrow/cpp/src/arrow/buffer.cc:18:
> /home/travis/build/apache/arrow/cpp/src/arrow/util/bit-util.h: In member 
> function ‘void arrow::internal::BitmapWriter::Set()’:
> /home/travis/build/apache/arrow/cpp/src/arrow/util/bit-util.h:108:30: error: 
> conversion to ‘uint8_t
> {aka unsigned char}’ from ‘int’ may alter its value [-Werror=conversion]
> void Set() { current_byte_ |= (1 << bit_offset_); }
> ^
> /home/travis/build/apache/arrow/cpp/src/arrow/util/bit-util.h: In member 
> function ‘void arrow::internal::BitmapWriter::Clear()’:
> /home/travis/build/apache/arrow/cpp/src/arrow/util/bit-util.h:110:32: error: 
> conversion to ‘uint8_t {aka unsigned char}
> ’ from ‘int’ may alter its value [-Werror=conversion]
> void Clear()
> { current_byte_ &= ~(1 << bit_offset_); }
> ^
> cc1plus: all warnings being treated as errors
> [28/137] Building CXX object CMakeFiles/arrow_objlib.dir/src/arrow/array.cc.o
> FAILED: CMakeFiles/arrow_objlib.dir/src/arrow/array.cc.o 
> /usr/bin/ccache /usr/lib/ccache/g+-4.9 -DARROW_EXTRA_ERROR_CONTEXT 
> -DARROW_NO_DEPRECATED_API -DARROW_WITH_BROTLI -DARROW_WITH_LZ4 
> -DARROW_WITH_SNAPPY -DARROW_WITH_ZLIB -DARROW_WITH_ZSTD -isystem 
> /home/travis/build/apache/arrow/cpp-toolchain/include -isystem 
> googletest_ep-prefix/src/googletest_ep/include -isystem 
> gbenchmark_ep/src/gbenchmark_ep-install/include -isystem 
> /home/travis/build/apache/arrow/cpp/thirdparty/hadoop/include 
> -I/home/travis/build/apache/arrow/cpp/src -ggdb -O0 -Wall -std=c11 -msse3 
> -Wconversion -Wno-sign-conversion -Werror -g -fPIC -std=gnu+11 -MD -MT 
> CMakeFiles/arrow_objlib.dir/src/arrow/array.cc.o -MF 
> CMakeFiles/arrow_objlib.dir/src/arrow/array.cc.o.d -o 
> CMakeFiles/arrow_objlib.dir/src/arrow/array.cc.o -c 
> /home/travis/build/apache/arrow/cpp/src/arrow/array.cc
> In file included from 
> /home/travis/build/apache/arrow/cpp/src/arrow/buffer.h:29:0,
> from /home/travis/build/apache/arrow/cpp/src/arrow/array.h:27,
> from /home/travis/build/apache/arrow/cpp/src/arrow/array.cc:18:
> /home/travis/build/apache/arrow/cpp/src/arrow/util/bit-util.h: In member 
> function ‘void arrow::internal::BitmapWriter::Set()’:
> /home/travis/build/apache/arrow/cpp/src/arrow/util/bit-util.h:108:30: error: 
> conversion to ‘uint8_t {aka unsigned char}’ from ‘int’ may alter its value 
> [-Werror=conversion]
> void Set() { current_byte_ |= (1 << bit_offset_); }
> ^
> /home/travis/build/apache/arrow/cpp/src/arrow/util/bit-util.h: In member 
> function ‘void arrow::internal::BitmapWriter::Clear()’:
> /home/travis/build/apache/arrow/cpp/src/arrow/util/bit-util.h:110:32: error: 
> conversion to ‘uint8_t {aka unsigned char}’ from ‘int’ may alter its value 
> [-Werror=conversion]
> void Clear() { current_byte_ &= ~(1 << bit_offset_); }
> ^
> cc1plus: all warnings being treated as errors
> ```



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

Reply via email to