Skip to content

Commit

Permalink
feat(build): replace -Wc++-compat' with -x c++'
Browse files Browse the repository at this point in the history
As of this commit, a build with CFLAGS="-x c++" succeeds; ie: all source
files in the tree are both valid C++ source code AND valid C code.

Convert wcpp-compat distcheck builds to actually build with C++, to
prevent future pull requests from breaking C++ compatibility going
forward.

stack-info: PR: aws#578, branch: aws-nslick/stack/25
Signed-off-by: Nicholas Sielicki <[email protected]>
  • Loading branch information
aws-nslick committed Sep 30, 2024
1 parent c7aa42d commit 2682e66
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 7 deletions.
24 changes: 20 additions & 4 deletions .github/workflows/distcheck.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ jobs:
./configure --prefix=/opt/aws-ofi-nccl --with-mpi=/opt/amazon/openmpi \
--with-libfabric=/opt/amazon/efa \
--with-cuda=/usr/local/cuda \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-tests=no \
--enable-platform-aws
Expand All @@ -122,6 +123,9 @@ jobs:
cc:
- gcc
- clang
mode:
- cpp
- c
tracing:
- lttng
- none
Expand All @@ -137,7 +141,7 @@ jobs:
cc: gcc
cc-version: 13

name: u2204/${{ matrix.sdk }}/libfabric@git/${{matrix.cc}}(${{matrix.cc-variant}})/distcheck/
name: u2204/${{ matrix.sdk }}/libfabric@git/${{matrix.cc}}(${{matrix.cc-variant}})/distcheck(${{ matrix.mode }})/
steps:
- uses: actions/checkout@v4
- name: Configure Neuron SDK Repository
Expand Down Expand Up @@ -219,10 +223,12 @@ jobs:
./configure --with-mpi=/opt/amazon/openmpi \
--with-libfabric=/opt/amazon/efa \
--with-cuda=/usr/local/cuda/ \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-tests \
--enable-platform-aws
else
./configure --with-libfabric=/opt/amazon/efa \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-neuron \
--enable-platform-aws
fi
Expand All @@ -249,6 +255,9 @@ jobs:
cc:
- gcc
- clang
mode:
- cpp
- c
sdk:
- cuda
- neuron
Expand All @@ -260,7 +269,7 @@ jobs:
cc: gcc
cc-version: 13

name: u2204/${{ matrix.sdk }}/${{matrix.cc}}(${{matrix.cc-variant}})/unit-tests/
name: u2204/${{ matrix.sdk }}/${{matrix.cc}}(${{matrix.cc-variant}})/unit-tests(${{ matrix.mode }})/
steps:
- uses: actions/checkout@v4
- name: Configure Neuron SDK Repository
Expand Down Expand Up @@ -337,13 +346,15 @@ jobs:
./configure --with-mpi=/opt/amazon/openmpi \
--with-libfabric=/opt/amazon/efa \
--with-cuda=/usr/local/cuda/ \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-tests \
--enable-debug \
--enable-platform-aws
else
./configure --with-libfabric=/opt/amazon/efa \
--enable-neuron \
--enable-debug \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-platform-aws
fi
make -j "$(nproc)"
Expand All @@ -367,7 +378,10 @@ jobs:
sdk:
- cuda
- neuron
name: CodeChecker - ${{ matrix.sdk }}
mode:
- cpp
- c
name: CodeChecker - ${{ matrix.sdk }} ${{ matrix.mode }}
steps:
- uses: actions/checkout@v4
- uses: actions/setup-python@v5
Expand Down Expand Up @@ -423,12 +437,14 @@ jobs:
./configure \
--with-libfabric="/opt/amazon/efa" \
--enable-neuron \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-platform-aws
else
./configure \
--with-libfabric="/opt/amazon/efa" \
--with-mpi="/opt/amazon/openmpi" \
--with-cuda=/usr/local/cuda/ \
--enable-cpp=${{ matrix.mode == 'cpp' && 'yes' || 'no' }} \
--enable-tests \
--enable-platform-aws
fi
Expand All @@ -447,7 +463,7 @@ jobs:
- name: Save CodeChecker HTML output.
uses: actions/upload-artifact@v4
with:
name: CodeChecker Bug Reports for ${{ matrix.sdk }}
name: CodeChecker Bug Reports for ${{ matrix.sdk }} ${{ matrix.mode }}
path: ${{ steps.codechecker.outputs.result-html-dir }}/*.html

- name: CodeChecker Pass Or Fail?
Expand Down
9 changes: 6 additions & 3 deletions configure.ac
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,6 @@ AC_DEFINE_UNQUOTED([OFI_NCCL_TRACE], [${trace}], [Defined to 1 unit test output

dnl standard and normal
picky_compiler_flags="-Wall -Wextra"
dnl C specific warning that warngs when using C features which prevent parsing
dnl as C++. Best-effort, and doesn't cover everything.
picky_compiler_flags="${picky_compiler_flags} -Wc++-compat"
dnl some top-level API functions have unused parameters, but we have to keep
dnl them. TODO: explicitly annotate those functions, rather than setting this
dnl globally.
Expand Down Expand Up @@ -217,6 +214,12 @@ AS_IF([test -d "${srcdir}/.git" -a -z "${enable_werror}"],

AC_SUBST([NCCL_NET_OFI_DISTCHCK_CONFIGURE_FLAGS])

AC_ARG_ENABLE([cpp],
[AS_HELP_STRING([--enable-cpp], [(Developer) Enable building as C++ source. Off by default.])])
AS_IF([test "${enable_cpp}" = "yes"],
[AC_MSG_NOTICE([Building as C++ source.])
CFLAGS="${CFLAGS} -x c++ -std=c++17"])

AC_CONFIG_FILES([Makefile
include/Makefile
3rd-party/Makefile
Expand Down

0 comments on commit 2682e66

Please sign in to comment.