Skip to content

Use C++20 for programs that generate the HTML lists #445

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

jwakely
Copy link
Member

@jwakely jwakely commented Nov 4, 2024

As of version 19.1.0, Clang's libc++ doesn't provide chrono::clock_cast or chrono::parse so maybe it's too soon to make this change.

@jwakely jwakely marked this pull request as draft November 4, 2024 13:01
@jwakely jwakely force-pushed the c++20 branch 9 times, most recently from e841fea to 26cc0b0 Compare November 4, 2024 14:32
@jwakely
Copy link
Member Author

jwakely commented Nov 4, 2024

As of version 19.1.0, Clang's libc++ doesn't provide chrono::clock_cast or chrono::parse so maybe it's too soon to make this change.

And the ubuntu-latest runner-image for GitHub Actions is ubuntu-22.04 which only has clang-15, which isn't good enough. There is an ubuntu-24.04 image with a newer clang, but that image is still in beta.

And the GCC version in ubuntu-latest is insufficient to build the C++20 code too.

@jwakely jwakely force-pushed the c++20 branch 2 times, most recently from 560b76a to 7872e53 Compare November 4, 2024 14:41

auto parse_date(std::istream & temp) -> gregorian::date {
auto parse_date(std::istream & temp) -> std::chrono::year_month_day {
#if __cpp_lib_chrono >= 201803L
Copy link
Member Author

Choose a reason for hiding this comment

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

chrono::parse requires GCC 14, but there's a workaround.

else {
auto mtime = fs::last_write_time(filename);
#if __cpp_lib_chrono >= 201803L
t = clock_cast<system_clock>(mtime);
Copy link
Member Author

Choose a reason for hiding this comment

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

chrono::clock_cast isn't supported by libc++, but there's a workaround.

@jwakely
Copy link
Member Author

jwakely commented Nov 4, 2024

There are several places in the code that should be converted to use views::filter

@jwakely jwakely force-pushed the c++20 branch 2 times, most recently from f917514 to a80f36e Compare November 4, 2024 20:04
@jwakely
Copy link
Member Author

jwakely commented Nov 4, 2024

There are several places in the code that should be converted to use views::filter

e.g.

   // This construction calls out for filter-iterators
//   std::multiset<lwg::issue, order_by_first_tag> pending_issues;
   std::vector<lwg::issue> pending_issues;
   for (auto const & elem : issues ) {
      if (predicate(elem)) {
         pending_issues.emplace_back(elem);
      }
   }

   sort(begin(pending_issues), end(pending_issues), order_by_section{section_db});

But I'm not sure if would actually be faster/simpler/better to insert into a multiset from a filter_view than the current code.

@jwakely
Copy link
Member Author

jwakely commented Nov 4, 2024

There are several places in the code that should be converted to use views::filter

e.g.

   // This construction calls out for filter-iterators
//   std::multiset<lwg::issue, order_by_first_tag> pending_issues;
   std::vector<lwg::issue> pending_issues;
   for (auto const & elem : issues ) {
      if (predicate(elem)) {
         pending_issues.emplace_back(elem);
      }
   }

   sort(begin(pending_issues), end(pending_issues), order_by_section{section_db});

But I'm not sure if would actually be faster/simpler/better to insert into a multiset from a filter_view than the current code.

Maybe for C++23 one day, just because everything above can be replaced by:

auto pending_issues = issues | std::views::filter(predicate) | std::ranges::to<std::multiset<lwg::issue, order_by_section>>(order_by_section{section_db});

@jwakely jwakely force-pushed the c++20 branch 2 times, most recently from 0cc9ced to 9d18565 Compare November 4, 2024 21:07
@Dani-Hub
Copy link
Member

Dani-Hub commented Nov 21, 2024

Some weeks ago I considered a similar change locally (Only switching to C++20 without anything else) to see how mingw can handle these changes. I now tested your branch successfully on my system and it seems to work well. I appreciate this course of action.

@jwakely
Copy link
Member Author

jwakely commented May 29, 2025

I've rebased this PR. It would be nice if the github actions workflows didn't need to use CXX=g++-14 for C++20 support, but the default version in the ubuntu-latest image is g++-13. If we merge this, then we should remember to change it back to g++ when ubuntu-latest starts to refer to ubuntu-26.04 in a year's time.

jwakely added 7 commits May 29, 2025 20:15
Use g++-14 for GitHub workflows, as ubuntu-latest is ubuntu-24.04 which
uses g++-13 by default.
Use workarounds for missing chrono::parse and chrono::clock_cast.
Instead of sorting several times in a row using different orderings, we
can define a single projection that defines the desired final order.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants