enable more warnings

[ghost]

-Wall -Wextra is the bare minimum amount. There are many good warnings not included there.

* dc26ad1e origin/enable-all-the-warnings feat(build): enable -Wnull-dereference
| 
| We get this for free, likely fixed in #627 or with some of the
| -fanalyzer fixes. Enable -Wnull-dereference whenever -Werror is enabled.
|
|  configure.ac | 2 ++
|  1 file changed, 2 insertions(+)
|

  
* 7ffd15fb feat(build): enable -Wcast-align
| 
| Without this, gcc complains:
| > cast from 'char *' to 'struct nccl_ofi_freelist_elem_t *' increases
| > required alignment from 1 to 8
| 
| Fix problematic areas in container_of and freelist, then enable
| -Wcast-align whenever -Werror is enabled.
| 
| Related-commit: 4e649131
|
|  configure.ac                     | 2 ++
|  include/nccl_ofi_config_bottom.h | 4 ++--
|  include/nccl_ofi_freelist.h      | 4 ++--
|  src/nccl_ofi_freelist.c          | 6 +++---
|  4 files changed, 9 insertions(+), 7 deletions(-)
|

  
* 4a23bc27 feat(build): enable -Wdouble-promotion
| 
| Fix cases where floats were implicitly being converted to doubles. You
| cannot pass a float to our logging macro, so explicitly cast those
| usages to doubles and use the right format specifier.
| 
| Enable -Wdouble-promotion when -Werror is enabled.
| 
|  configure.ac                        |  2 ++
|  include/nccl-headers/nvidia/tuner.h |  2 +-
|  src/nccl_ofi_net.c                  |  4 ++--
|  src/platform-aws.c                  |  7 +++----
|  src/tuner/nccl_ofi_tuner.c          | 14 +++++++-------
|  tests/unit/show_tuner_decisions.c   |  8 ++++----
|  6 files changed, 19 insertions(+), 18 deletions(-)
| 


* 84323fee feat(build): enable -Wimplicit-fallthrough
| 
| Add missing case statements and enable -Wimplicit-fallthrough.
| 
|  include/nccl_ofi_mr.h          |  3 +++
|  src/nccl_ofi_api.c             |  4 ++++
|  src/nccl_ofi_cuda.c            | 41 ++++++++++++++++++-----------------
|  src/nccl_ofi_rdma.c            | 47 ++++++++++++++++++++++++++++++++++++++++
|  src/nccl_ofi_sendrecv.c        |  2 ++
|  tests/functional/test-common.h |  3 +++
|  tests/unit/test-common.h       |  3 +++
|  7 files changed, 83 insertions(+), 20 deletions(-)
|

  
* 0f5eab8b feat(build): enable stricter -Wformat=2
| 
| Enable -Wformat=2, which is stricter than Wall's -Wformat:
| > -Wformat=2
| >> Enable -Wformat plus additional format checks. Currently equivalent to
| >> -Wformat -Wformat-nonliteral -Wformat-security -Wformat-y2k.
| 
| Fix a warning that this exposed: this literal in topo.c can be made
| completely const and should be.
| 
|  configure.ac                   | 2 ++
|  src/nccl_ofi_topo.c            | 2 +-
|  tests/functional/test-common.h | 3 +++
|  tests/unit/test-common.h       | 3 +++
|  4 files changed, 9 insertions(+), 1 deletion(-)
|


* 79079522 feat(build): remove all shadowing, enable -Wshadow
| 
| a global `provider_filter` was made redundant with 159bfed1 and this is
| now passed to every user directly, but the provider_filter string that
| now exists in nccl_net_ofi_create_plugin was shadowing the global one
| that was exported everywhere in nccl_ofi.h. Delete the global one.
| 
| rdma had two cases of declaring a new variable, shadowing the old
| variable, pointing to the same thing. Delete the second useless
| declaration. sendrecv had a case where it was shadowing a "req" variable
| in an inner scope, but pointing to a different req. Just rename the
| variable in the inner scope to be more clear.
| 
| mr.c needed to remove a useless include, which removes the shadow on
| system_page_size. This unfortunately exposed an issue in the header for
| mr.h; config.h was included late, it was transiently relying on an
| inclusion for the definition of offsetof and, because we do not build
| with C11, stdbool. The conditional inclusion of fi_domain.h was not
| correct in all cases. So add <stddef.h> there, always include
| fi_domain.h, and fix the config.h misordering.
| 
| Rename some things in tests to avoid shadowing.
| 
| With these fixed, enable -Wshadow whenever -Werror is enabled.
| 
|  configure.ac                             | 15 +++++++++++++--
|  include/nccl_ofi.h                       |  4 ----
|  include/nccl_ofi_mr.h                    |  7 ++++---
|  src/nccl_ofi_mr.c                        |  1 -
|  src/nccl_ofi_net.c                       |  2 --
|  src/nccl_ofi_rdma.c                      |  2 --
|  src/nccl_ofi_sendrecv.c                  | 11 ++++++-----
|  tests/functional/nccl_connection.c       | 16 ++++++++--------
|  tests/functional/nccl_message_transfer.c | 16 ++++++++--------
|  tests/functional/ring.c                  | 16 ++++++++--------
|  tests/functional/test-common.h           | 15 ++++++++-------
|  tests/unit/mr.c                          | 23 +++++++++++------------
|  12 files changed, 66 insertions(+), 62 deletions(-)
|  

Commit is based atop #632, which is itself based atop #627. This is a draft PR and should not be reviewed before both of those land. When they land, this PR should automatically remove those commits, leaving only the top 6 commits.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[ghost]

aws:bot:retest