RFE: -pedantic library build in CI

[Paul Hargrove]

While looking into issue 490, we've noticed that we are not currently testing with -Werror -pedantic applied to the library build. We do so only for the tests (and even there a subset).

This issue is a request to get the GitLab CI jobs dirac-{gcc,clang}_wall to perform pedantic checking of the library code, not just the user-facing headers as is currently done compiling a subset of the tests.

[Paul Hargrove]

I am looking specifically at configuring with CXXFLAGS='-Wall -Werror -pedantic' for g++ and CXXFLAGS='-Wall -Werror -pedantic -DUPCXX_USE_NODISCARD=0' for clang++.

The first thing I hit was a complaint building GASNet's udp-conduit:

/Users/phargrov/upcxx/Bpedantic/bld/GASNet-stable/other/amudp/../../gasnet_toolhelp.h:416:28: error: 'long long' is a C++11 extension [-Werror,-Wc++11-long-long]
    const uint64_t _mask = 0x7f7f7f7f7f7f7f7fULL;
                           ^
/Users/phargrov/upcxx/Bpedantic/bld/GASNet-stable/other/amudp/amudp_ep.cpp:103:29: error: 'long long' is a C++11 extension [-Werror,-Wc++11-long-long]
  AMX_assert(pool->magic == AMUDP_BUFFERPOOL_MAGIC);
                            ^
/Users/phargrov/upcxx/Bpedantic/bld/GASNet-stable/other/amudp/amudp_ep.cpp:90:43: note: expanded from macro 'AMUDP_BUFFERPOOL_MAGIC'
#define AMUDP_BUFFERPOOL_MAGIC ((uint64_t)0x1001feedbac31001ULL)
                                          ^
/Users/phargrov/upcxx/Bpedantic/bld/GASNet-stable/other/amudp/amudp_ep.cpp:134:29: error: 'long long' is a C++11 extension [-Werror,-Wc++11-long-long]
  AMX_assert(pool->magic == AMUDP_BUFFERPOOL_MAGIC);
                            ^
/Users/phargrov/upcxx/Bpedantic/bld/GASNet-stable/other/amudp/amudp_ep.cpp:90:43: note: expanded from macro 'AMUDP_BUFFERPOOL_MAGIC'
#define AMUDP_BUFFERPOOL_MAGIC ((uint64_t)0x1001feedbac31001ULL)
                                          ^
/Users/phargrov/upcxx/Bpedantic/bld/GASNet-stable/other/amudp/amudp_ep.cpp:146:33: error: 'long long' is a C++11 extension [-Werror,-Wc++11-long-long]
      ep->bufferPool[i].magic = AMUDP_BUFFERPOOL_MAGIC;
                                ^
/Users/phargrov/upcxx/Bpedantic/bld/GASNet-stable/other/amudp/amudp_ep.cpp:90:43: note: expanded from macro 'AMUDP_BUFFERPOOL_MAGIC'
#define AMUDP_BUFFERPOOL_MAGIC ((uint64_t)0x1001feedbac31001ULL)
                                          ^
4 errors generated.

This was only seen on macOS, which is not part of the current scope (dirac-{gcc,clang}_wall are Linux), and probably easily resolved w/ -std=c++11. ```

[Paul Hargrove]

The next issue I hit (still only seen on macOS, not Linux) is a complaint from logic in utils/config/upcxx/gasnet_macros.sh:

conftest.cpp:6:54: error: empty macro arguments are a C99 feature [-Werror,-Wc99-extensions]
  _t1rMKHV81Bsp9aU1A_+GASNETT_NEVER_INLINE(/*fnname*/,/*declarator*/)+_t2z0dVmWCWoS2H2Ro_
                                                     ^
conftest.cpp:6:69: error: empty macro arguments are a C99 feature [-Werror,-Wc99-extensions]
  _t1rMKHV81Bsp9aU1A_+GASNETT_NEVER_INLINE(/*fnname*/,/*declarator*/)+_t2z0dVmWCWoS2H2Ro_
                                                                    ^
2 errors generated.

Again, -std=c++11 seems to resolve this.

[Paul Hargrove]

The good news is that excluding the two items (easily resolved using -std=c++11 if attempting pedantic with Xcode), the only pedantic warnings (promoted to errors) seen are the ones in issue 490 (for which we have pull request 382 in testing right now)

[Paul Hargrove]

As hoped, -std=c++11 gives a clean -pendantic build on macOS.
So, I am proposing to apply the following change to the gitlab-ci.yml after issue 490 has been resolved (to avoid the appearance of regression on develop):

commit 2e6e93457b0eb83c56bf1e04a08ea95a02a60a64
Author: Paul H. Hargrove <[e-mail]>
Date:   Tue Sep 21 17:55:58 2021 -0700

    Expand -pedantic coverage to include library builds

diff --git a/gitlab-ci.yml b/gitlab-ci.yml
index 8ba755f..261b91a 100644
--- a/gitlab-ci.yml
+++ b/gitlab-ci.yml
@@ -534,7 +534,7 @@ dirac-clang_wall:
     - export CC=clang CXX=clang++
     - export CI_RUN_TESTS=0
     - export CI_NO_TESTS="${CI_NO_TESTS:+$CI_NO_TESTS,}nodiscard,issue"
-    - export CI_EXTRAFLAGS="$CI_EXTRAFLAGS -pedantic -Wall -DUPCXX_USE_NODISCARD=0"
+    - export CXXFLAGS="-pedantic -Wall -DUPCXX_USE_NODISCARD=0"

 # Compile-only tester with extended compiler checks,
 # but a reduced set of tests (since many are not -Wall-clean).
@@ -545,7 +545,7 @@ dirac-gcc_wall:
     - export CC=gcc CXX=g++
     - export CI_RUN_TESTS=0
     - export CI_NO_TESTS="${CI_NO_TESTS:+$CI_NO_TESTS,}nodiscard,issue"
-    - export CI_EXTRAFLAGS="$CI_EXTRAFLAGS -pedantic -Wall"
+    - export CXXFLAGS="-pedantic -Wall"

 # Compile-only tester with LTO and pedantic warnings enabled
 # NOTE: only SMP because amudp and ammpi libs lead to harmless ODR warnings
@@ -592,6 +592,9 @@ macos_current-gcc:
 # Compile-only tester with extended compiler checks,
 # but a reduced set of tests (since many are not -Wall-clean).
 # Note: `-DUPCXX_USE_NODISCARD=0` prevents complaints that `[[nodiscard]]` requires C++17.
+# Note: `-std=c++11` prevents complaints about `long long` (in udp-conduit) and
+#       empty macro arguments (in a probe in gasnet_macros.sh), both due to the
+#       default C++98 in effect when those are compiled.
 macos_current-xcode_wall:
   extends: [ .generic, .new_xcode, .macos_current ]
   variables:
@@ -599,7 +602,7 @@ macos_current-xcode_wall:
   before_script:
     - export CI_RUN_TESTS=0
     - export CI_NO_TESTS="${CI_NO_TESTS:+$CI_NO_TESTS,}nodiscard,issue"
-    - export CI_EXTRAFLAGS="$CI_EXTRAFLAGS -pedantic -Wall -DUPCXX_USE_NODISCARD=0"
+    - export CXXFLAGS="-std=c++11 -pedantic -Wall -DUPCXX_USE_NODISCARD=0"

 ################################################################################
 ##

[Paul Hargrove]

The changes above have been committed, but they fail to add -Werror to the library compile. So, these three jobs may generate new warnings, but not the desired failure. This issue remains open pending additional work to address that.

[Dan Bonachea]

Mass roll-over of open issues to next release milestone

[Paul Hargrove]

Resolved (as @Dan Bonachea proposed in issue 516) in 1de5b26ff of the upcxx-ci repo.