Skip to content

treewide: define default C&C++ versions for compilers #255979

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.

Sign up for GitHub

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

Draft
tobim wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

treewide: define default C&C++ versions for compilers #255979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions pkgs/development/compilers/gcc/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -413,6 +413,8 @@ ${""} done
passthru = {
inherit langC langCC langObjC langObjCpp langAda langFortran langGo langD version;
isGNU = true;
defaultCStandard = if atLeast11 then 17 else if atleast5 then 11 else 89;
defaultCxxStandard = if atLeast11 then 17 else if atleast6 then 14 else 98;
} // lib.optionalAttrs (!atLeast12) {
hardeningUnsupportedFlags = lib.optionals is48 [ "stackprotector" ] ++ [ "fortify3" ];
};
Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/10/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 11;
Copy link
Member

@AndersonTorres AndersonTorres Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be treated as int?

Copy link
Contributor Author

@tobim tobim Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean?
In case you are thinking of gnu11 vs c++11: That almost never makes a difference. Allowing arithmetic on this value is what makes it useful.

Copy link
Member

@AndersonTorres AndersonTorres Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Understandable.
The problem to me is because e.g C20 comes after C99 however 20<99.

Copy link
Contributor Author

@tobim tobim Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a very astute observation. It seems I'll need to change to interface to a defaultCxxStandardless = YY: ... function. Bummer.

Copy link

@ghost ghost Sep 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is another reason why it's better to detect "is the compiler going to use a standard equal to or later than XYZ" like autoconf does -- no need to encode the entire list of standards in nix, along with their partial order.

defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/11/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 17;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/12/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 17;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/13/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 17;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/14/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 17;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/15/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 17;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/16/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 17;
defaultCxxStandard = 17;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/5/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 11;
defaultCxxStandard = 98;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/6/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 11;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/7/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 11;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/8/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 11;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/compilers/llvm/9/clang/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ let
passthru = {
inherit libllvm;
isClang = true;
defaultCStandard = 11;
defaultCxxStandard = 14;
hardeningUnsupportedFlags = [ "fortify3" ];
};

Expand Down
10 changes: 3 additions & 7 deletions pkgs/development/libraries/grpc/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,10 @@ stdenv.mkDerivation rec {
# problematic, because the compatibility types in abseil will have different
# interface definitions than the ones used for building abseil itself.
# [1] https://github.com/grpc/grpc/blob/v1.57.0/CMakeLists.txt#L239-L243
++ (let
defaultCxxIsOlderThan17 =
(stdenv.cc.isClang && lib.versionAtLeast stdenv.cc.cc.version "16.0")
|| (stdenv.cc.isGNU && lib.versionAtLeast stdenv.cc.cc.version "11.0");
in lib.optionals (stdenv.hostPlatform.isDarwin && defaultCxxIsOlderThan17)
[
# TODO: Patching that line out would be a better solution.
++ lib.optionals (stdenv.hostPlatform.isDarwin && stdenv.cc.cc.defaultCxxStandard >= 17) [
"-DCMAKE_CXX_STANDARD=17"
]);
];

# CMake creates a build directory by default, this conflicts with the
# basel BUILD file on case-insensitive filesystems.
Expand Down
2 changes: 2 additions & 0 deletions pkgs/development/libraries/protobuf/generic-v3-cmake.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ let
"-Dprotobuf_ABSL_PROVIDER=package"
] ++ lib.optionals (!stdenv.targetPlatform.isStatic) [
"-Dprotobuf_BUILD_SHARED_LIBS=ON"
] ++ lib.optionals (stdenv.cc.cc.defaultCxxStandard < 14) [
"-DCMAKE_CXX_STANDARD=14"
Comment on lines +83 to +84
Copy link
Member

@AndersonTorres AndersonTorres Sep 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
] ++ lib.optionals (stdenv.cc.cc.defaultCxxStandard < 14) [
"-DCMAKE_CXX_STANDARD=14"
] ++ lib.optionals (stdenv.cc.cc.defaultCxxStandard < 14) [
"-DCMAKE_CXX_STANDARD=14"

What??

How can stdenv.cc.cc.defaultCxxStandard be less than 14 but -DCMAKE_CXX_STANDARD be 14?

Copy link
Contributor Author

@tobim tobim Sep 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea here is to pass -DCMAKE_CXX_STANDARD only if the compiler in use defaults to an older language version. C++14 is the minimum required to build protobuf.

Copy link
Member

@AndersonTorres AndersonTorres Sep 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I understood. The default C++ standard is not the same as the supported C++ standard.

However it looks too superfluous.
"If the default C++ standard of this compiler is not 14, then force-set the C++ standard to 14".
Why not force-set the C++ standard to 14 and ignore the compiler's default?

Copy link
Contributor Author

@tobim tobim Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the default is newer than 14 some of the dependencies are not API compatible (cough, abseil, cough), so that approach doesn't work.

Copy link
Contributor

@happysalada happysalada Sep 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should write a comment to explain that so that the next guy who looks as this doesn't go through the same process.
something like

# protobuf needs at least c++14 to be compiled
# if the default is higher don't override it as abseil for example won't be api compatible

]
# Tests fail to build on 32-bit platforms; fixed in 3.22
# https://github.com/protocolbuffers/protobuf/issues/10418
Expand Down