Skip to content

Enable -Wall and -Wextra warnings on the _cl module - #930

Open
alexfikl wants to merge 6 commits into
inducer:mainfrom
alexfikl:add-walls
Open

alexfikl wants to merge 6 commits into
inducer:mainfrom
alexfikl:add-walls

Conversation

@alexfikl

@alexfikl alexfikl commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread CMakeLists.txt
Comment on lines +187 to +190
target_include_directories(_cl SYSTEM PRIVATE
${CL_INC_DIR}
${NANOBIND_INCLUDE_DIR}
${Python_NumPy_INCLUDE_DIRS})

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is mainly needed so that gcc/clang don't complain about warnings in these headers. I think NB_SUPRESS_WARNINGS already does most of this, but added it with CL and NumPy just in case.

Comment thread CMakeLists.txt
Comment on lines +198 to +217
if(CMAKE_CXX_COMPILER_ID MATCHES "GNU|Clang")
target_compile_options(_cl PRIVATE
-Wall
-Wextra
# NOTE: VLAs are used by PYOPENCL_STACK_CONTAINER for dubious reasons
-Wpedantic -Wno-vla
-Wshadow
-Wold-style-cast
-Wcast-align
-Wconversion -Wsign-conversion
-Wnon-virtual-dtor
-Woverloaded-virtual
-Wformat=2
-Wimplicit-fallthrough
-Wdouble-promotion
-Wnull-dereference
# NOTE: mostly calls to deprecated CL functions that we support
-Wno-deprecated-declarations
)
endif()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Loads of random warnings here. The -Wconversion ones are a bit noisy, but overall they should be ok?

Comment thread CMakeLists.txt
Comment on lines +202 to +203
# NOTE: VLAs are used by PYOPENCL_STACK_CONTAINER for dubious reasons
-Wpedantic -Wno-vla

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

That VLA in there is pretty old. Still useful? Even the Linux kernel removed VLAs 😁

Comment thread src/wrap_cl.hpp
{ \
PYOPENCL_PRINT_CALL_TRACE_INFO(#NAME, TRACE_INFO); \
cl_int status_code; \
cl_int _cl_status_code; \

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Some of these were shadowing, but most of them were not. I added a _cl_ prefix to everybody just for consistency.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant