Skip to content

CI job name "🐍 3 • GCC 10 • C++2a • x64" (changed from C++20, which is misleading) #4074

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

Closed
wants to merge 3 commits into from

Conversation

rwgk
Copy link
Collaborator

@rwgk rwgk commented Jul 16, 2022

Description

Distinguish cmake_cxx_standard & std_passed_to_compiler, to make it more obvious what is actually being tested, and why PYBIND11_CPP20 is not defined.

Suggested changelog entry:

rwgk added 3 commits July 16, 2022 11:15
…ore obvious what is actually being tested, and why PYBIND11_CPP20 is not defined.
@rwgk rwgk changed the title Debug gcc std=20 vs PYBIND11_CPP17 mismatch CI job name "🐍 3 • GCC 10 • C++2a • x64" (changed from C++20, which is misleading) Jul 18, 2022
@rwgk rwgk marked this pull request as ready for review July 18, 2022 15:53
@rwgk rwgk requested a review from henryiii as a code owner July 18, 2022 15:53
@rwgk rwgk requested a review from Skylion007 July 18, 2022 15:53
@henryiii
Copy link
Collaborator

This complicates the CI config for very little benefit. And shouldn't we define PYBIND11_CPP20 for c+++2a? And maybe we should just bump the GCC version so that we get c++20 instead?

@rwgk
Copy link
Collaborator Author

rwgk commented Jul 18, 2022

This complicates the CI config for very little benefit.

I'm open to exploring alternatives, but the current reporting is misleading, which is bad because there is the danger that we slip into wrong conclusions.

And shouldn't we define PYBIND11_CPP20 for c+++2a?

That will get us into the weeds here, with plenty potential pitfalls:

# if __cplusplus >= 202002L

And maybe we should just bump the GCC version so that we get c++20 instead?

I kind of like that, but do we really want to take the step of giving up on GCC 10 just to avoid fixing the reporting (with +3 trivial lines)?

Another simple alternative is to test c++17 (instead of c++2a). That would be obvious & clean. I don't think we're gaining much exercising "experimental" C++20 features of an outdated compiler. What do you think?

@henryiii
Copy link
Collaborator

henryiii commented Jul 18, 2022

I wouldn't consider it "giving up", it's just that we don't test every version of GCC already, and GCC 8-10 works fine in 11-17, and it doesn't fully support 20 yet, so we could instead make this a test of whatever version was the first to support 20 (GCC 11?). I don't think we are gaining much with GCC 7+10 vs. 7+11 unless 10 is important for some special reason.

Testing the first version of a compiler to support C++20 on C++20 mode is a very useful datapoint, though. (similarly with C++17, which I'm guessing was GCC 7? Hopefully not 10? :) )

@rwgk
Copy link
Collaborator Author

rwgk commented Jul 18, 2022

Below are all the GCC versions we are testing with at the moment. Currently the job for which I want to tweak the reporting (with C++ Info: 10.4.0 C++17 below, reported as C++20, but actually using -std=c++2a) is the only one using GCC 10.

What do you want to do exactly?

I'd prefer to:

  • not have misleading reporting because that could make us slip into wrong conclusions
  • not lose GCC 10 completely

What I don't know: Why does cmake automatically map from 20 to 2a? Can we rely on cmake to "know" what the GCC it finds actually supports? What other ways do we have to build the list of

GCC version to first fully support

  • C++11
  • C++14
  • C++17
  • C++20

which is what it sounds to me you are suggesting?

$ grep -i 'C++ Info: .*gcc' *.txt | cut -d' ' -f2- | cut -d_ -f1 | sort -t' ' -k3 -n | uniq
C++ Info: 4.8.5 20150623 (Red Hat 4.8.5-44) C++11
C++ Info: 7.5.0 C++11
C++ Info: 8.3.0 C++11
C++ Info: 8.5.0 20210514 (Red Hat 8.5.0-10) C++11
C++ Info: 9.4.0 C++11
C++ Info: 9.4.0 C++17
C++ Info: 10.4.0 C++17
C++ Info: 11.2.1 20220127 (Red Hat 11.2.1-9) C++11
C++ Info: 12.1.0 C++11

@rwgk
Copy link
Collaborator Author

rwgk commented Jul 18, 2022

https://gcc.gnu.org/projects/cxx-status.html seems to have all the information we need, although with a lot of extra details.

Taking the max-by-eye in the "Available in GCC?" columns I'm arriving at:

  • C++11: GCC 4.8.1
  • C++14: GCC 7
  • C++17: GCC 8
  • C++20: GCC 12

But I also noticed this:

To enable C++20 support, add the command-line parameter -std=c++20 (use -std=c++2a in GCC 9 and earlier) to your g++ command line.

This suggests -std=c++20 should work with GCC 10. Could it be that cmake is interfering in an unfortunate way? Is there a way to disable the interference?

@rwgk
Copy link
Collaborator Author

rwgk commented Jul 18, 2022

Using docker pull gcc:10, it turns out -std=c++20 vs -std=c++2a does not make a difference to the value of __cplusplus:

# ./what__cplusplus_all.sh
+ cat what__cplusplus.cc
#include <iostream>

int main() { std::cout << __cplusplus << std::endl; }
+ g++ --version
g++ (GCC) 10.4.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

+ g++ what__cplusplus.cc -std=c++11
+ ./a.out
201103
+ g++ what__cplusplus.cc -std=c++14
+ ./a.out
201402
+ g++ what__cplusplus.cc -std=c++17
+ ./a.out
201703
+ g++ what__cplusplus.cc -std=c++2a
+ ./a.out
201709
+ g++ what__cplusplus.cc -std=c++20
+ ./a.out
201709
+ g++ what__cplusplus.cc -std=c++23
g++: error: unrecognized command-line option '-std=c++23'; did you mean '-std=c++03'?
+ g++ what__cplusplus.cc
+ ./a.out
201402

@rwgk
Copy link
Collaborator Author

rwgk commented Jul 18, 2022

Raw data covering GCC 7 through 12:

::::::::::::::
gcc7.txt
::::::::::::::
+ cat
+ g++ --version
g++ (GCC) 7.5.0
Copyright (C) 2017 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

+ g++ what__cplusplus.cc -std=c++11
+ ./a.out
201103
+ g++ what__cplusplus.cc -std=c++14
+ ./a.out
201402
+ g++ what__cplusplus.cc -std=c++17
+ ./a.out
201703
+ g++ what__cplusplus.cc -std=c++2a
g++: error: unrecognized command line option '-std=c++2a'; did you mean '-std=c++03'?
+ g++ what__cplusplus.cc -std=c++20
g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++03'?
+ g++ what__cplusplus.cc -std=c++23
g++: error: unrecognized command line option '-std=c++23'; did you mean '-std=c++03'?
+ g++ what__cplusplus.cc
+ ./a.out
201402
::::::::::::::
gcc8.txt
::::::::::::::
+ cat
+ g++ --version
g++ (GCC) 8.5.0
Copyright (C) 2018 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

+ g++ what__cplusplus.cc -std=c++11
+ ./a.out
201103
+ g++ what__cplusplus.cc -std=c++14
+ ./a.out
201402
+ g++ what__cplusplus.cc -std=c++17
+ ./a.out
201703
+ g++ what__cplusplus.cc -std=c++2a
+ ./a.out
201709
+ g++ what__cplusplus.cc -std=c++20
g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?
+ g++ what__cplusplus.cc -std=c++23
g++: error: unrecognized command line option '-std=c++23'; did you mean '-std=c++03'?
+ g++ what__cplusplus.cc
+ ./a.out
201402
::::::::::::::
gcc9.txt
::::::::::::::
+ cat
+ g++ --version
g++ (GCC) 9.5.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

+ g++ what__cplusplus.cc -std=c++11
+ ./a.out
201103
+ g++ what__cplusplus.cc -std=c++14
+ ./a.out
201402
+ g++ what__cplusplus.cc -std=c++17
+ ./a.out
201703
+ g++ what__cplusplus.cc -std=c++2a
+ ./a.out
201709
+ g++ what__cplusplus.cc -std=c++20
g++: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?
+ g++ what__cplusplus.cc -std=c++23
g++: error: unrecognized command line option '-std=c++23'; did you mean '-std=c++03'?
+ g++ what__cplusplus.cc
+ ./a.out
201402
::::::::::::::
gcc10.txt
::::::::::::::
+ cat
+ g++ --version
g++ (GCC) 10.4.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

+ g++ what__cplusplus.cc -std=c++11
+ ./a.out
201103
+ g++ what__cplusplus.cc -std=c++14
+ ./a.out
201402
+ g++ what__cplusplus.cc -std=c++17
+ ./a.out
201703
+ g++ what__cplusplus.cc -std=c++2a
+ ./a.out
201709
+ g++ what__cplusplus.cc -std=c++20
+ ./a.out
201709
+ g++ what__cplusplus.cc -std=c++23
g++: error: unrecognized command-line option '-std=c++23'; did you mean '-std=c++03'?
+ g++ what__cplusplus.cc
+ ./a.out
201402
::::::::::::::
gcc11.txt
::::::::::::::
+ cat
+ g++ --version
g++ (GCC) 11.3.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

+ g++ what__cplusplus.cc -std=c++11
+ ./a.out
201103
+ g++ what__cplusplus.cc -std=c++14
+ ./a.out
201402
+ g++ what__cplusplus.cc -std=c++17
+ ./a.out
201703
+ g++ what__cplusplus.cc -std=c++2a
+ ./a.out
202002
+ g++ what__cplusplus.cc -std=c++20
+ ./a.out
202002
+ g++ what__cplusplus.cc -std=c++23
+ ./a.out
202100
+ g++ what__cplusplus.cc
+ ./a.out
201703
::::::::::::::
gcc12.txt
::::::::::::::
+ cat
+ g++ --version
g++ (GCC) 12.1.0
Copyright (C) 2022 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

+ g++ what__cplusplus.cc -std=c++11
+ ./a.out
201103
+ g++ what__cplusplus.cc -std=c++14
+ ./a.out
201402
+ g++ what__cplusplus.cc -std=c++17
+ ./a.out
201703
+ g++ what__cplusplus.cc -std=c++2a
+ ./a.out
202002
+ g++ what__cplusplus.cc -std=c++20
+ ./a.out
202002
+ g++ what__cplusplus.cc -std=c++23
+ ./a.out
202100
+ g++ what__cplusplus.cc
+ ./a.out
201703

@rwgk
Copy link
Collaborator Author

rwgk commented Jul 18, 2022

I think the below would make sense. I'll try that out in another PR.

The idea is:

  • Latest "fully supported" is what most people will want to use.
  • We also want one C++11 & one C++14.
                                   Curr all  Proposed gcc
g++ (GCC) 7.5.0  -std=c++11 201103 *         *
                 -std=c++14 201402
                 -std=c++17 201703           *
g++ (GCC) 8.5.0  -std=c++11 201103 *
                 -std=c++14 201402           *
                 -std=c++17 201703           *
g++ (GCC) 9.5.0  -std=c++11 201103 *
                 -std=c++14 201402
                 -std=c++17 201703 *
g++ (GCC) 10.4.0 -std=c++11 201103
                 -std=c++14 201402
                 -std=c++17 201703           *
                 -std=c++2a 201709 ODD
g++ (GCC) 11.3.0 -std=c++11 201103 *
                 -std=c++14 201402
                 -std=c++17 201703
                 -std=c++20 202002           *
g++ (GCC) 12.1.0 -std=c++11 201103 *
                 -std=c++14 201402
                 -std=c++17 201703
                 -std=c++20 202002           *

rwgk added a commit to rwgk/pybind11 that referenced this pull request Jul 18, 2022
@rwgk
Copy link
Collaborator Author

rwgk commented Jul 19, 2022

Based on @henryiii's comments I'm closing this band-aid level PR in favor of PR #4083, which systematically expands our GCC and clang coverage, especially for modern compilers.

@rwgk rwgk closed this Jul 19, 2022
rwgk added a commit that referenced this pull request Jul 21, 2022
* More systematic gcc coverage, based on #4074 (comment)

* Fix complete fail.

* Resolve GCC 11 & 12 "redundant move in return statement" warnings.

* Also add clang 11, 12, 13 (to gather info for warning suppressions).

* Add & use `PYBIND11_DETECTED_CLANG_WITH_MISLEADING_CALL_STD_MOVE_EXPLICITLY_WARNING`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants