Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
ednolan
force-pushed
the
enolan_modernexemplar2
branch
from
a0190c6 to
c902c54
Compare
| {"preset": "msvc-release", "runner": "windows-latest"} | ||
| ] | ||
|
|
||
| build-and-test: |
There was a problem hiding this comment.
This bumps minimum supported version to C++20. We support 17 already and want to build&test that, and I'll be seeing if we can bring it down to 14 or 11 in the future.
Also, this removes old compilers (GCC 12, Clang 17). We should definitely keep some of those if possible.
There was a problem hiding this comment.
You are free to add those after making changes, but I removed them based on testing that the builds with those jobs currently fail. The reasons are that the older compilers in question lack support for <format>; and that cstring_view uses std::type_identity_t which isn't in C++17.
| cmake_minimum_required(VERSION 3.25) | ||
|
|
||
| set(CMAKE_CXX_STANDARD 23) | ||
| cmake_minimum_required(VERSION 3.30...4.3) |
There was a problem hiding this comment.
is there a reason for the upper bound?
There was a problem hiding this comment.
I'm not a CMake expert, but I'm told that this is a best practice having to do with better handling of CMake policy defaults.
|
|
||
| * C++17 | ||
| * CMake 3.25 | ||
| * A C++ compiler that conforms to the C++20 standard or greater |
There was a problem hiding this comment.
why c++20? We are explicitly trying to support c++17.
There was a problem hiding this comment.
See above comment (the implementation currently uses std::type_identity_t).
| static constexpr size_type npos = size_type(-1); | ||
|
|
||
| private: | ||
| static constexpr charT empty_string[1]{}; |
There was a problem hiding this comment.
unexpected change for an "update to latest version". Did the old code break some compiler?
There was a problem hiding this comment.
Yes. This is the compiler error I was working around:
In file included from /src/examples/example.cpp:3:
/src/include/beman/cstring_view/cstring_view.hpp: In constructor 'constexpr beman::basic_cstring_view<charT, traits>::basic_cstring_view()':
/src/include/beman/cstring_view/cstring_view.hpp:115:28: error: 'empty_string' defined 'static' in 'constexpr' function only available with '-std=c++2b' or '-std=gnu++2b'
115 | static const charT empty_string[1]{};
| ^~~~~~~~~~~~
| @@ -1 +0,0 @@ | |||
| #include <beman/cstring_view/cstring_view.hpp> | |||
There was a problem hiding this comment.
Please re-add this file; Lakos rule that each header has a cpp file that includes it that verifies it is standalone.
There was a problem hiding this comment.
We don't need a cpp file to do that test; see https://cmake.org/cmake/help/latest/prop_tgt/VERIFY_INTERFACE_HEADER_SETS.html, which takes care of this in beman.
ednolan
force-pushed
the
enolan_modernexemplar2
branch
from
c902c54 to
88286f5
Compare
- Make cstring_view an INTERFACE-based library - Bring cstring_view the new JSON-based CI framework - Add some missing SPDX license identifiers - Update the README to contain the latest language and a compiler support table - Update the CMake installation logic to the latest version - Slightly adjust the implementation of the default constructor for C++20 compatibility
ednolan
force-pushed
the
enolan_modernexemplar2
branch
from
88286f5 to
80397d5
Compare
2 participants
Uh oh!
There was an error while loading. Please reload this page.