Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmake: add -Wno-unknown-pragmas to CMAKE_CXX_FLAGS #12128

Merged
merged 2 commits into from Nov 23, 2016

Conversation

tchaikov
Copy link
Contributor

user might want to customize it.

Signed-off-by: Kefu Chai kchai@redhat.com

@tchaikov tchaikov changed the title cmake: should not reset CMAKE_CXX_FLAGS cmake: add -Wno-unknown-pragmas to CMAKE_CXX_FLAGS Nov 22, 2016
@adamemerson
Copy link
Contributor

Leitmotifs Glue Taletelling to Music

@@ -112,7 +112,8 @@ else(no_yasm)
endif(CMAKE_SYSTEM_PROCESSOR MATCHES "amd64|x86_64")
endif(no_yasm)

set(CMAKE_CXX_FLAGS "${CMAKE_C_FLAGS} -ftemplate-depth-1024 -Wno-invalid-offsetof")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ftemplate-depth-1024")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-invalid-offsetof -Wno-unknown-pragmas")
Copy link
Contributor

Choose a reason for hiding this comment

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

are there any CMAKE_C_FLAGS set earlier that we still want to include in CMAKE_CXX_FLAGS?

Copy link
Contributor

Choose a reason for hiding this comment

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

@cbodley
I think I was responsible for the last commit of these lines,
But all the previous set warnings are currently passed to both C and C++.
Which might be what it needs anyways. Not having this line does not set any -W-flags for CXX.
So I don't think that the fix on line 115 is correct.

This is what I find grepping for CXX_FLAGS:

./src/CMakeLists.txt:115:set(CMAKE_CXX_FLAGS "${CMAKE_C_FLAGS} -ftemplate-depth-1024 -Wno-invalid-offsetof")
./src/CMakeLists.txt:116:set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wnon-virtual-dtor")
./src/CMakeLists.txt:118:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wstrict-null-sentinel -Woverloaded-virtual")
./src/CMakeLists.txt:126:    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11")
./src/CMakeLists.txt:145:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fdiagnostics-color=${DIAGNOSTICS_COLOR}")
./src/CMakeLists.txt:165:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fprofile-arcs -ftest-coverage -O0")
./src/CMakeLists.txt:173:      set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -I${NSS_INCLUDE_DIR} -I${NSPR_INCLUDE_DIR}")
./src/CMakeLists.txt:227:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -I${XIO_INCLUDE_DIR}")
./src/CMakeLists.txt:232:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -I${RDMA_INCLUDE_DIR}")
./src/CMakeLists.txt:237:  set(CMAKE_CXX_FLAGS "-march=native ${CMAKE_CXX_FLAGS} -I${DPDK_INCLUDE_DIR}")
./src/CMakeLists.txt:248:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-builtin-malloc -fno-builtin-calloc -fno-builtin-realloc -fno-builtin-free")
./src/CMakeLists.txt:252:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-builtin-malloc -fno-builtin-calloc -fno-builtin-realloc -fno-builtin-free")
./src/CMakeLists.txt:256:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-builtin-malloc -fno-builtin-calloc -fno-builtin-realloc -fno-builtin-free")
./src/CMakeLists.txt:352:  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -I${DPDK_INCLUDE_DIR}")

and C_FLAGS gives:

./src/CMakeLists.txt:25:set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wall -Wtype-limits -Wignored-qualifiers -Winit-self")
./src/CMakeLists.txt:26:set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wpointer-arith -Werror=format-security -fno-strict-aliasing -fsigned-char")
./src/CMakeLists.txt:30:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -rdynamic")
./src/CMakeLists.txt:35:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-inconsistent-missing-override")
./src/CMakeLists.txt:36:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-mismatched-tags -Wno-unused-function")
./src/CMakeLists.txt:37:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-unused-local-typedef -Wno-inconsistent-missing-override")
./src/CMakeLists.txt:38:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-unused-private-field -Wno-varargs")
./src/CMakeLists.txt:39:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-gnu-designator -Wno-mismatched-tags")
./src/CMakeLists.txt:40:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wno-missing-braces -Wno-parentheses -Wno-deprecated-register")
./src/CMakeLists.txt:62:      set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=2")
./src/CMakeLists.txt:67:    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fstack-protector-strong")
./src/CMakeLists.txt:115:set(CMAKE_CXX_FLAGS "${CMAKE_C_FLAGS} -ftemplate-depth-1024 -Wno-invalid-offsetof")
./src/CMakeLists.txt:144:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fdiagnostics-color=${DIAGNOSTICS_COLOR}")
./src/CMakeLists.txt:166:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fprofile-arcs -ftest-coverage")
./src/rocksdb/CMakeLists.txt:102:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=address")
./src/rocksdb/CMakeLists.txt:113:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=thread -fPIC")
./src/rocksdb/CMakeLists.txt:123:  set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fsanitize=undefined")
./src/civetweb/CMakeLists.txt:279:set(CMAKE_C_FLAGS_COVERAGE "${CMAKE_C_FLAGS_DEBUG}" CACHE STRING
./src/civetweb/CMakeLists.txt:292:    CMAKE_C_FLAGS_COVERAGE
./src/civetweb/cmake/AddCCompilerFlag.cmake:35:    set(CMAKE_C_FLAGS${VARIANT} "${CMAKE_C_FLAGS${VARIANT}} ${FLAG}" PARENT_SCOPE)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@cbodley and @wjwithagen thanks for your reviews, fixed and repushed.

this silences warnings like:

warning: ignoring #pragma clang diagnostic [-Wunknown-pragmas]

and recent clang (3.9 and 4.0) do not complain at seeing
so we only pass this this param to gcc.

Signed-off-by: Kefu Chai <kchai@redhat.com>
- some options are only meaningful for C++, so move them into
  CMAKE_CXX_FLAGS.
- regroup CMAKE_CXX_FLAGS and CMAKE_C_FLAGS, and remove the duplicated
  options.
- do not reset CMAKE_CXX_FLAGS with ${CMAKE_C_FLAGS}, instead, always
  append ${CMAKE_C_FLAGS} to existing ${CMAKE_CXX_FLAGS}. so we don't
  clobber CMAKE_CXX_FLAGS set by command line.

Signed-off-by: Kefu Chai <kchai@redhat.com>
@tchaikov
Copy link
Contributor Author

changelog

  • extract "add -Wno-unknown-pragmas to CMAKE_C_FLAGS" into its own commit
  • append ${CMAKE_C_FLAGS} to ${CMAKE_CXX_FLAGS}, as all cflags apply to C++.

@liewegas liewegas added this to the kraken milestone Nov 23, 2016
@tchaikov tchaikov merged commit fa5bf9c into ceph:master Nov 23, 2016
@tchaikov tchaikov deleted the wip-cmake branch November 23, 2016 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants