[MLIR][Python] Update Nanobind Warnings List for clang-cl on Windows #166828
Conversation
Created using spr 1.3.7 [skip ci]
Created using spr 1.3.7
boomanaiden154
requested review from
jpienaar and
makslevental
and removed request for
jpienaar
|
@llvm/pr-subscribers-mlir Author: Aiden Grossman (boomanaiden154) ChangesWe recently moved over to compiling with clang-cl on Windows. This ended Full diff: https://github.com/llvm/llvm-project/pull/166828.diff 1 Files Affected:
diff --git a/mlir/cmake/modules/AddMLIRPython.cmake b/mlir/cmake/modules/AddMLIRPython.cmake
index fa6aec8a603a9..8196e2a2a3321 100644
--- a/mlir/cmake/modules/AddMLIRPython.cmake
+++ b/mlir/cmake/modules/AddMLIRPython.cmake
@@ -791,7 +791,6 @@ function(add_mlir_python_extension libname extname)
get_property(NB_LIBRARY_TARGET_NAME TARGET ${libname} PROPERTY LINK_LIBRARIES)
target_compile_options(${NB_LIBRARY_TARGET_NAME}
PRIVATE
- -Wall -Wextra -Wpedantic
-Wno-c++98-compat-extra-semi
-Wno-cast-qual
-Wno-covered-switch-default
@@ -799,11 +798,11 @@ function(add_mlir_python_extension libname extname)
-Wno-nested-anon-types
-Wno-unused-parameter
-Wno-zero-length-array
+ -Wno-missing-field-initializers
${eh_rtti_enable})
target_compile_options(${libname}
PRIVATE
- -Wall -Wextra -Wpedantic
-Wno-c++98-compat-extra-semi
-Wno-cast-qual
-Wno-covered-switch-default
@@ -811,6 +810,7 @@ function(add_mlir_python_extension libname extname)
-Wno-nested-anon-types
-Wno-unused-parameter
-Wno-zero-length-array
+ -Wno-missing-field-initializers
${eh_rtti_enable})
endif()
|
We recently moved over to compiling with clang-cl on Windows. This ended up causing a large increase in warnings, particularly due to how warnings are handled in nanobind. cd91d0f initially set -Wall -Wextra and -Wpedantic while fixing another issue, which is probably not what we want to do on third-party code. We also need to disable -Wmissing-field-initializers to get things clean in this configuration. Pull Request: llvm#166828
We recently moved over to compiling with clang-cl on Windows. This ended up causing a large increase in warnings, particularly due to how warnings are handled in nanobind. cd91d0f initially set -Wall -Wextra and -Wpedantic while fixing another issue, which is probably not what we want to do on third-party code. We also need to disable -Wmissing-field-initializers to get things clean in this configuration. Pull Request: llvm#166828
makslevental
left a comment
makslevental
left a comment
There was a problem hiding this comment.
This thing is a perennial PITA. I'm stamping to unblock but can you can also try NB_SUPPRESS_WARNINGS.
|
PS also this wjakob/nanobind#994 🙂 |
Yeah, looks like it. Very interesting to see a project that is pretty against disabling warnings, but also against accepting patches to fix warnings that do come up. I'll land this as is and see if I can get |
Created using spr 1.3.7 [skip ci]
boomanaiden154
changed the base branch from
users/boomanaiden154/main.mlirpython-update-nanobind-warnings-list-for-clang-cl-on-windows
to
main
boomanaiden154
deleted the
users/boomanaiden154/mlirpython-update-nanobind-warnings-list-for-clang-cl-on-windows
branch
…on Windows We recently moved over to compiling with clang-cl on Windows. This ended up causing a large increase in warnings, particularly due to how warnings are handled in nanobind. cd91d0f initially set -Wall -Wextra and -Wpedantic while fixing another issue, which is probably not what we want to do on third-party code. We also need to disable -Wmissing-field-initializers to get things clean in this configuration. Reviewers: makslevental, jpienaar, rkayaith Reviewed By: makslevental Pull Request: llvm/llvm-project#166828
|
Thanks! |
|
It looks like the option in wjakob/nanobind#868 does not actually fix the issue. We still get warnings when building the nanobind objects that compose the static lib, which it doesn't seem like that patch actually addresses. So I think we'll need this logic to manually set additional compile options for now. |
We recently moved over to compiling with clang-cl on Windows. This ended
up causing a large increase in warnings, particularly due to how
warnings are handled in nanobind. cd91d0f
initially set -Wall -Wextra and -Wpedantic while fixing another issue,
which is probably not what we want to do on third-party code. We also
need to disable -Wmissing-field-initializers to get things clean in this
configuration.