[Openmp-commits] [PATCH] D65867: [RFC] [OpenMP] Turn on -Wall compiler warnings by default

Jonathan Peyton via Phabricator via Openmp-commits openmp-commits at lists.llvm.org
Tue Aug 13 09:00:36 PDT 2019


jlpeyton added inline comments.


================
Comment at: openmp/cmake/config-ix.cmake:4-15
+check_c_compiler_flag(-Wall OPENMP_HAVE_WALL_FLAG)
 check_c_compiler_flag(-Werror OPENMP_HAVE_WERROR_FLAG)
 
+# Additional warnings that are not enabled by -Wall.
+check_cxx_compiler_flag(-Wcast-qual OPENMP_HAVE_WCAST_QUAL_FLAG)
+check_c_compiler_flag(-Wformat-pedantic OPENMP_HAVE_WFORMAT_PEDANTIC_FLAG)
+check_c_compiler_flag(-Wsign-compare OPENMP_HAVE_WSIGN_COMPARE_FLAG)
----------------
Hahnfeld wrote:
> jlpeyton wrote:
> > Should everything be check_cxx_compiler_flag()?
> This was also my first feeling, but my second thought was that we might have C files again in the future. For example,  some very simple tools with OMPT might not need C++ and we'll want all warnings for them. However, that's a weak argument for now without a real use and I can change it to `check_cxx_compiler_flag` if you think that's more consistent.
After some thought, I think we should use `check_cxx_compiler_flag()` and here is my reasoning:
The trend with the openmp project has been turning C code/files/anything into C++, sometimes nominally, but C++ nonetheless.  I think it is reasonable to assume language agnostic options (e.g., -Wall) are supported by both the C and corresponding C++ compiler.  This assumption is seen in the `append_if(HAVE_FLAG "-Wflag" C_FLAGS CXX_FLAGS)` code in HandleOpenMPOptions.cmake.  So any future C file should be able to use the flags without issue.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65867/new/

https://reviews.llvm.org/D65867





More information about the Openmp-commits mailing list