-
Notifications
You must be signed in to change notification settings - Fork 455
CDRIVER-5891 extend C standard coverage up to C23 #1890
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Verified by this patch. The C Driver EVG Project settings have been updated to include the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
RHEL_DISTROS = [] | ||
RHEL_DISTROS += ls_distro(name='rhel76', os='rhel', os_type='linux', os_ver='7.6') | ||
RHEL_DISTROS += ls_distro(name='rhel80', os='rhel', os_type='linux', os_ver='8.0') | ||
RHEL_DISTROS += ls_distro(name='rhel84', os='rhel', os_type='linux', os_ver='8.4') | ||
RHEL_DISTROS += ls_distro(name='rhel90', os='rhel', os_type='linux', os_ver='9.0') | ||
RHEL_DISTROS += ls_distro(name='rhel91', os='rhel', os_type='linux', os_ver='9.1') | ||
RHEL_DISTROS += ls_distro(name='rhel92', os='rhel', os_type='linux', os_ver='9.2') | ||
RHEL_DISTROS += ls_distro(name='rhel93', os='rhel', os_type='linux', os_ver='9.3') | ||
RHEL_DISTROS += ls_distro(name='rhel94', os='rhel', os_type='linux', os_ver='9.4') | ||
RHEL_DISTROS += ls_distro(name='rhel95', os='rhel', os_type='linux', os_ver='9.5') | ||
RHEL_DISTROS += ls_distro(name='rhel8.9', os='rhel', os_type='linux', os_ver='8.7') | ||
RHEL_DISTROS += ls_distro(name='rhel92', os='rhel', os_type='linux', os_ver='9.0') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use the variadic splatting operator *
in list/tuple literals (or **
in dict literals) to expand any iterable object into zero or more inline elements:
RHEL_DISTROS = [] | |
RHEL_DISTROS += ls_distro(name='rhel76', os='rhel', os_type='linux', os_ver='7.6') | |
RHEL_DISTROS += ls_distro(name='rhel80', os='rhel', os_type='linux', os_ver='8.0') | |
RHEL_DISTROS += ls_distro(name='rhel84', os='rhel', os_type='linux', os_ver='8.4') | |
RHEL_DISTROS += ls_distro(name='rhel90', os='rhel', os_type='linux', os_ver='9.0') | |
RHEL_DISTROS += ls_distro(name='rhel91', os='rhel', os_type='linux', os_ver='9.1') | |
RHEL_DISTROS += ls_distro(name='rhel92', os='rhel', os_type='linux', os_ver='9.2') | |
RHEL_DISTROS += ls_distro(name='rhel93', os='rhel', os_type='linux', os_ver='9.3') | |
RHEL_DISTROS += ls_distro(name='rhel94', os='rhel', os_type='linux', os_ver='9.4') | |
RHEL_DISTROS += ls_distro(name='rhel95', os='rhel', os_type='linux', os_ver='9.5') | |
RHEL_DISTROS += ls_distro(name='rhel8.9', os='rhel', os_type='linux', os_ver='8.7') | |
RHEL_DISTROS += ls_distro(name='rhel92', os='rhel', os_type='linux', os_ver='9.0') | |
RHEL_DISTROS = [ | |
*ls_distro(name='rhel76', os='rhel', os_type='linux', os_ver='7.6'), | |
*ls_distro(name='rhel80', os='rhel', os_type='linux', os_ver='8.0'), | |
*ls_distro(name='rhel84', os='rhel', os_type='linux', os_ver='8.4'), | |
... | |
] |
if [[ "${CMAKE_GENERATOR:-}" =~ "Visual Studio" ]]; then | ||
# MSBuild needs additional assistance. | ||
# https://devblogs.microsoft.com/cppblog/improved-parallelism-in-msbuild/ | ||
export UseMultiToolTask=1 | ||
export EnforceProcessCountAcrossBuilds=1 | ||
fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should move to Ninja on Windows, eventually. Maybe someday.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I really like the comprehensive std-c*
tasks with the comments documenting the compiler versions.
('rhel94', 'gcc', None, [99, 11, 17, 23]), # GCC 11.4 (max: C2x) | ||
('rhel95', 'gcc', None, [99, 11, 17, 23]), # GCC 11.5 (max: C2x) | ||
|
||
('windows-vsCurrent', 'vs2017x64', None, [99, 11, 17, 'latest']), # Max: C17, clatest (C2x) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change "latest" to "clatest"? The message in compile-std.sh refers to "clatest" (but checks for "latest"):
if [[ "${C_STD_VERSION}" == "latest" ]]; then
[[ "${CMAKE_GENERATOR:-}" =~ "Visual Studio" ]] || {
echo "C_STD_VERSION=clatest is only supported with Visual Studio generators" 1>&2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mixed up the /std:clatest
flag with the C_STD_VERSION=latest
variable. Fixed.
Summary
Resolves CDRIVER-5891.
Extends
std-matrix
Evergreen tasks to support compilation with C99 through C23 using GCC, Clang, and MSVC.Drive-By EVG Config and Script Improvements
To facilitate the expansion of compilation coverage to newer versions of Visual Studio, compile scripts (beyond
std-matrix
tasks) are updated to utilize the CMakeCMAKE_GENERATOR
andCMAKE_GENERATOR_PLATFORM
environment variables to specify the Visual Studio version and target architecture (equivalent to-G
+-A
; similar changes to mongodb/mongo-cxx-driver#1082). The definition ofCC
andCXX
variables are also moved into the config generator to ensure copmiler consistency (no need forif [[ "$CC" == "gcc" ]]
-esque logic in scripts), including specific compiler version requests (e.g.clang-12
). The list of distros indistros.py
was also refactored to use thels_distro
pattern to specify small+large distro pairs (similar changes to mongodb/mongo-cxx-driver#1082).Compiler Support
Reliably deducing compiler support for any given standard is a chore, especially given ongoing partial implementations of the C2x standard (as well as the C17 standard for older compiler versions). For example, support for
static_assert
is already available as early as GCC 9, while other C2x features such as[[nodiscard]]
,__VA_OPT__
, andnullptr
are implemented by GCC 14, with more features such as#embed
provided by GCC 15, and etc. (see major version changelogs for 15.1, 14.1, 13.1, etc.). Similarly,static_assert
and[[nodiscard]]
are supported by Clang 9, with a scattering of C2x features being partially implemented across versions up to Clang 20 (technically even C11 standard support is still in a partial state).Instead of laborously deducing compiler support for language standards or features, detection of C standard support is completely deferred to CMake's built-in feature detection routines (regardless of what flag may be required:
-std=c11
,-std=c1x
,-std=c17
,-std=c2x
, or-std=c23
). When a compiler does not support a given standard or feature, CMake will emit errors similar to the following (e.g. for GCC):The absence of C standards in the matrix for older compiler versions as proposed by this PR was entirely decided by the presence CMake errors similar to the one shown above.
Disabled Warnings
The
public-header-warnings
(+ tests and examples) are added to the C standard task matrix. This motivated adding some new warning flags to be explicitly disabled for Clang with-Weverything
:-Wc++98-compat-pedantic
: we are not interested in continuing to support C++98 in either the C or C++ Driver. (Note: C++11 references the C99 spec.)-Wpre-c2x-compat
: we actively test pre-C2x compatibility and do not need to rely on this warning.bool
despite pre-C2x#define bool _Bool
and inclusion of C99<stdbool.h>
is unnecessarily noisy.-Wunsafe-buffer-usage
: too aggressive, warning on nearly any subscript of a pointer. Impractical to address this warning.C2x Boolean Compatibility
The following code:
and similarly:
produces the following compiler error with GCC 11 and GCC 12 given
-std=c2x
:due to GCC 11 and GCC 12 defining boolean constants as:
instead of as a simple
1
and0
(as in C17 and prior) or as keywords (in GCC 13 and newer with-std=c2x
or-std=c23
). The"("
mentioned in the error message is the first(
in((_Bool)+1u)
.According to https://github.com/gcc-mirror/gcc, this change is due to this commit on Oct 29, 2020, which was motivated by this feature request. The
((_Bool)+1u)
form was proposed by N2393 as an intermediate solution until the compiler has completed implementing the booleans as keywords. It seems<stdbool.h>
for GCC 11 and GCC 12 implement this intermediate solution. These changes were later superceded by this commit on Sep 7, 2022, which remove the macros entirely for-std=c2x
. This seems to correspond to the GCC 13 release and onwards which implemented the keyword-based solution.Because the language does not support contextually redefining
true
andfalse
as "simple" macros wherever the BSON DSL macros may be expanded (that is, we cannot#define
in a#define
), a solution that attempts to preserve the use oftrue
andfalse
as unconditional DSL predicates was considered but rejected. Instead, the specialtrue
andfalse
DSL predicates are renamed to new and equivalent DSL predicates:always
andnever
.Therefore, the code above must now be written instead as:
and similarly: