Require C11

[devreal]

trafficstars

Error out if the compiler does not support C11 and limit the standard to C11 if the compiler accepts a standard flag. This limit prevents us from using features of newer standard versions.

Also removes support for __thread since we now rely on C11 _Thread_local.

Please check my m4 code, not an expert on this.

See https://github.com/open-mpi/ompi/pull/12658#pullrequestreview-2164230547 for notes from the RM discussion and the PR that initiated this.

There will likely be more changes over time replacing pre-C11 features with C11 features where possible but I wanted to keep this manageable.

@jsquyres where should I put a note about this for users?

[devreal]

Mhh, CI fails with this error:

  LEX      keyval_lex.c
  CC       keyval_lex.lo
keyval_lex.c: In function ‘opal_util_keyval_yy_init_buffer’:
keyval_lex.c:1848:48: error: implicit declaration of function ‘fileno’ [-Werror=implicit-function-declaration]
 1848 |         b->yy_is_interactive = file ? (isatty( fileno(file) ) > 0) : 0;
      |                                                ^~~~~~
cc1: some warnings being treated as errors

I tried adding POSIX feature test macros but that doesn't seem to help. There seems to be a fix in flex from 7 years ago: https://github.com/westes/flex/issues/263 Maybe that just hasn't trickled down to the distributions yet... I cannot reproduce this problem on my Mac, unfortunately.

[bosilca]

Add -D _POSIX_C_SOURCE=20240101L" to libopalutilkeyval_la_CFLAGS` in opal/util/keyval/Makefile.am ?

I guess you will need to so the same for all lex/yacc outputs

[devreal]

Not sure why CI fails. All tests seem green but at the end it's marked as failed. Can someone please point me to the error?

[devreal]

Got it. Tests show green even if they failed in Jenkins... The culprit is our tests on RHEL 7.9, which still ships GCC 4.8. GCC added support for _Atomic, _Thread_local, and _Generic in 4.9 [1] and we are currently testing for that as part of our C11 test. We are not using _Atomic by default and _Generic is used in one place, optionally (the displacement/size wrappers for collectives). This PR makes _Thread_local the only option for TLS and removes the detection of __thread. All other features of C11 appear to be available in GCC 4.8, including __STDC_VERSION__ == 201112L.

I see three options:

1. Drop support for RHEL 7.9 and its prehistoric default compiler. According to [2] RHEL 7.9 is on extended support until 2028.
2. Separate out detection of _Atomic, _Generic, and _Thread_local, bring back the fallback to __thread, and require all other features of C11 by checking for __STDC_VERSION__ == 201112L (since GCC 4.7). That will give us access to some of the things C11 introduced without checking for all features explicitly, including _Static_assert, _Noreturn, stdalign.h, and anonymous structs.
3. Drop this PR and bring it back post 2028.

[1] https://gcc.gnu.org/wiki/C11Status [2] https://en.wikipedia.org/wiki/Red_Hat_Enterprise_Linux

[wenduwan]

Drop support for RHEL 7.9 and its prehistoric default compiler.

The extended supported is a paid subscription IIUC so I don't think we can strictly secure the test environment. FYI EFA is dropping support for CentOS 7 and RHEL 7.

[devreal]

That's an important data point, thanks @wenduwan. I put this topic on the list for the next devel call.

[devreal]

It looks like the Mac OS X environment is borked. It fails to find strsep (even though <string.h> is included) and fails on some internal header:

/Applications/Xcode_15.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/signal.h:69:42: error: use of undeclared identifier 'NSIG'
extern __const char *__const sys_signame[NSIG];
                                         ^

[rhc54]

Take a gander at @wenduwan comment in the slack "general" thread - looks like GitHub is making some significant changes to the Mac "actions" environment.

[devreal]

@rhc54 Thanks for the pointer. According to https://github.blog/changelog/2024-08-19-notice-of-upcoming-deprecations-and-breaking-changes-in-github-actions-runners/ the macos-12 are phased out but we're using macos-14-arm64 so I don't think we're affected here.

[hjelmn]

Awesome! +1000

[github-actions[bot]]

Hello! The Git Commit Checker CI bot found a few problems with this PR:

1a86e0b3: Disable silent-rules on CI

• check_cherry_pick: contains a cherry pick message that refers to a commit that exists, but is in an as-yet unmerged pull request: d4410ad42f6e156fb9ae07022bca77bb00a4a400

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

[github-actions[bot]]

Hello! The Git Commit Checker CI bot found a few problems with this PR:

1a86e0b3: Disable silent-rules on CI

• check_cherry_pick: contains a cherry pick message that refers to a commit that exists, but is in an as-yet unmerged pull request: d4410ad42f6e156fb9ae07022bca77bb00a4a400

Please fix these problems and, if necessary, force-push new commits back up to the PR branch. Thanks!

[devreal]

I removed the verbose output and rebased onto current main. All tests pass now. I think this is good to go so please re-review.

[bwbarrett]

@devreal Do you still want to pull these changes into main for the next major release? I think we're all on the same page that we want to do this.

[devreal]

Thanks for the nudge @bwbarrett. I rebased, hope all tests pass this time.

[devreal]

All tests passed. Please rereview and merge if happy with it.

[hppritcha]

could you squash down to fewer commits - maybe one?

[devreal]

Sorry, forgot about that. Squashed down to one commit and kept the details of the changes in the commit message.

[bwbarrett]

I actually disagree with Howard on the squash, but what's done is done.

[devreal]

Thanks @jsquyres!

@janjust The Nvidia CI seems to be running low on disk space.

[devreal]

Thanks :+1: For the record: this is 6.0 only will not be backported to the 5.0 branches.