Skip to content

Commit

Permalink
turn on -Werror=extra in Bazel
Browse files Browse the repository at this point in the history
Summary:
These are mostly helpful warnings but we explicitly disable two of
them that are problematic in our codebase.

We also remove -Werror=type-limits and -Werror=unused-but-set-variable
since they are both included as part of -Wextra.

Test Plan: Rely on CI.

Reviewers: alband

Subscribers:

Tasks:

Tags:

Pull Request resolved: pytorch#79327

Approved by: https://github.com/malfet
  • Loading branch information
Michael Andreas Dagitses authored and pytorchmergebot committed Jun 13, 2022
1 parent 3db1092 commit 34b0285
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 4 deletions.
26 changes: 24 additions & 2 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@ build --per_file_copt=^external/@-w
# cache
# * it allows for selective overrides on individual targets since the
# macro-level opts will come earlier than target level overrides
build --per_file_copt='^//.*\.(cpp|cc)$'@-Werror=type-limits
build --per_file_copt='^//.*\.(cpp|cc)$'@-Werror=unused-but-set-variable

build --per_file_copt='^//.*\.(cpp|cc)$'@-Werror=all
# The following warnings come from -Wall. We downgrade them from error
Expand All @@ -74,6 +72,30 @@ build --per_file_copt='^//.*\.(cpp|cc)$'@-Wno-sign-compare
# We intentionally use #pragma unroll, which is compiler specific.
build --per_file_copt='^//.*\.(cpp|cc)$'@-Wno-error=unknown-pragmas

build --per_file_copt='^//.*\.(cpp|cc)$'@-Werror=extra
# The following warnings come from -Wextra. We downgrade them from error
# to warnings here.
#
# unused-parameter-compare has a tremendous amount of violations in the
# codebase. It will be a lot of work to fix them, just disable it for
# now.
build --per_file_copt='^//.*\.(cpp|cc)$'@-Wno-unused-parameter
# missing-field-parameters has both a large number of violations in
# the codebase, but it also is used pervasively in the Python C
# API. There are a couple of catches though:
# * we use multiple versions of the Python API and hence have
# potentially multiple different versions of each relevant
# struct. They may have different numbers of fields. It will be
# unwieldy to support multiple versions in the same source file.
# * Python itself for many of these structs recommends only
# initializing a subset of the fields. We should respect the API
# usage conventions of our dependencies.
#
# Hence, we just disable this warning altogether. We may want to clean
# up some of the clear-cut cases that could be risky, but we still
# likely want to have this disabled for the most part.
build --per_file_copt='^//.*\.(cpp|cc)$'@-Wno-missing-field-initializers

build --per_file_copt='//:aten/src/ATen/RegisterCompositeExplicitAutograd\.cpp$'@-Wno-error=unused-function
build --per_file_copt='//:aten/src/ATen/RegisterCompositeImplicitAutograd\.cpp$'@-Wno-error=unused-function
build --per_file_copt='//:aten/src/ATen/RegisterMkldnnCPU\.cpp$'@-Wno-error=unused-function
Expand Down
26 changes: 24 additions & 2 deletions tools/rules/cu.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@ load("@rules_cuda//cuda:defs.bzl", "cuda_library")
NVCC_COPTS = [
"--expt-relaxed-constexpr",
"--expt-extended-lambda",
"--compiler-options=-Werror=type-limits",
"--compiler-options=-Werror=unused-but-set-variable",

"--compiler-options=-Werror=all",
# The following warnings come from -Wall. We downgrade them from
Expand All @@ -16,6 +14,30 @@ NVCC_COPTS = [
"--compiler-options=-Wno-sign-compare",
# We intentionally use #pragma unroll, which is compiler specific.
"--compiler-options=-Wno-error=unknown-pragmas",

"--compiler-options=-Werror=extra",
# The following warnings come from -Wextra. We downgrade them from
# error to warnings here.
#
# unused-parameter-compare has a tremendous amount of violations
# in the codebase. It will be a lot of work to fix them, just
# disable it for now.
"--compiler-options=-Wno-unused-parameter",
# missing-field-parameters has both a large number of violations
# in the codebase, but it also is used pervasively in the Python C
# API. There are a couple of catches though:
# * we use multiple versions of the Python API and hence have
# potentially multiple different versions of each relevant
# struct. They may have different numbers of fields. It will be
# unwieldy to support multiple versions in the same source file.
# * Python itself for many of these structs recommends only
# initializing a subset of the fields. We should respect the API
# usage conventions of our dependencies.
#
# Hence, we just disable this warning altogether. We may want to
# clean up some of the clear-cut cases that could be risky, but we
# still likely want to have this disabled for the most part.
"-Wno-missing-field-initializers",
]

def cu_library(name, srcs, copts = [], **kwargs):
Expand Down

0 comments on commit 34b0285

Please sign in to comment.