Move to C++20 #16077

omoerbeek · 2025-09-04T08:22:13Z

Short description

Currently running test package builds: all fine (see https://github.com/omoerbeek/pdns/actions/runs/17457993427)

Checklist

I have:

read the CONTRIBUTING.md document
compiled this code
tested this code
included documentation (including possible behaviour changes)
documented the code
added or modified regression test(s)
added or modified unit test(s)

A few compare calls needed to be made explicit to avoid on trixie clang 19:

In file included from /usr/include/boost/test/test_tools.hpp:45,
                 from /usr/include/boost/test/unit_test.hpp:18,
                 from ../pdns/test-dnsrecordcontent.cc:9:
../pdns/test-dnsrecordcontent.cc: In member function ‘void test_dnsrecordcontent::test_equality::test_method()’:
../pdns/test-dnsrecordcontent.cc:20:19: warning: C++20 says that these are ambiguous, even though the second is reversed:
   20 |   BOOST_CHECK(a1==a2);
      |                   ^~
In file included from ../pdns/test-dnsrecordcontent.cc:10:
../pdns/dnsrecords.hh:88:8: note: candidate 1: ‘virtual bool ARecordContent::operator==(const DNSRecordContent&) const’
   88 |   bool operator==(const DNSRecordContent& rhs) const override
      |        ^~~~~~~~
../pdns/dnsrecords.hh:88:8: note: candidate 2: ‘virtual bool ARecordContent::operator==(const DNSRecordContent&) const’ (reversed)

One set of warnings remain on trixie using clang 19:

In file included from ../dnsdist-web.cc:34:
In file included from ../dnsdist.hh:42:
In file included from ../dnsdist-doh-common.hh:34:
../tcpiohandler.hh:179:17: warning: 'atomic_load_explicit<TLSCtx>' is deprecated: use 'std::atomic<std::shared_ptr<T>>' instead [-Wdeprecated-declarations]
  179 |     return std::atomic_load_explicit(&d_ctx, std::memory_order_acquire);
      |                 ^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/c++/14/bits/shared_ptr_atomic.h:131:5: note: 'atomic_load_explicit<TLSCtx>' has been explicitly marked deprecated here
  131 |     _GLIBCXX20_DEPRECATED_SUGGEST("std::atomic<std::shared_ptr<T>>")
      |     ^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/x86_64-linux-gnu/c++/14/bits/c++config.h:132:45: note: expanded from macro '_GLIBCXX20_DEPRECATED_SUGGEST'
  132 | # define _GLIBCXX20_DEPRECATED_SUGGEST(ALT) _GLIBCXX_DEPRECATED_SUGGEST(ALT)
      |                                             ^
/usr/lib/gcc/x86_64-linux-gnu/14/../../../../include/x86_64-linux-gnu/c++/14/bits/c++config.h:100:19: note: expanded from macro '_GLIBCXX_DEPRECATED_SUGGEST'
  100 |   __attribute__ ((__deprecated__ ("use '" ALT "' instead")))
      |                   ^

…en though the second is reversed Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

…an incomplete class Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

miodvallat · 2025-09-04T08:45:06Z

Do we really want to move to C++20 at this point? Do you see any benefit in switching to that standard?

(also be warned that if this happens, I will introduce and use spaceship operators in the code)

coveralls · 2025-09-04T08:51:11Z

Pull Request Test Coverage Report for Build 17551617099

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

20 of 20 (100.0%) changed or added relevant lines in 5 files are covered.
99 unchanged lines in 11 files lost coverage.
Overall coverage increased (+0.01%) to 66.015%

Files with Coverage Reduction	New Missed Lines	%
modules/gpgsqlbackend/gpgsqlbackend.cc	1	88.8%
pdns/misc.cc	1	62.51%
pdns/validate.cc	1	68.5%
pdns/rcpgenerator.cc	2	90.53%
modules/gpgsqlbackend/spgsql.cc	3	68.18%
pdns/recursordist/test-syncres_cc2.cc	3	89.18%
pdns/dnsdistdist/dnsdist-lbpolicies.cc	4	72.57%
pdns/recursordist/syncres.cc	4	80.97%
pdns/packethandler.cc	8	72.25%
pdns/recursordist/test-syncres_cc1.cc	8	90.19%

Totals
Change from base Build 17546445389:	0.01%
Covered Lines:	128368
Relevant Lines:	165838

💛 - Coveralls

omoerbeek · 2025-09-04T08:55:04Z

Do we really want to move to C++20 at this point? Do you see any benefit in switching to that standard?

(also be warned that if this happens, I will introduce and use spaceship operators in the code)

Nothing dramatic, a few things nice to have, see https://en.cppreference.com/w/cpp/20.html
I like the little things, e.g.: string::ends_with()/starts_with(), map::contains().
Ranges is another thing we probably could make good use of

omoerbeek · 2025-09-04T09:23:06Z

@Habbie @rgacogne @pieterlexis any opinions on the matter? Code changes needed are fairly minimal and we get a few nice features, so I am inclined to go for C++20, but I won't press it.

Habbie · 2025-09-04T09:25:44Z

(also be warned that if this happens, I will introduce and use spaceship operators in the code)

I was just mentioning to Otto yesterday that us not being on ++20 has not stopped you from doing three way compares, but that they would look more fun in 20 :)

Habbie · 2025-09-04T09:26:54Z

pdns/dnsdistdist/meson.build

    'buildtype=debugoptimized',
    'warning_level=2',          # TODO Move this to 3 to enable -Wpedantic
-    'cpp_std=c++17',
+    'cpp_std=c++20',


while we're here, I'd love to switch this to gnu++20, which improves portability (at least to Haiku, perhaps also other OSes)

As long as clang does the right thing...

I believe it does, but it deserves the same amount of testing you did for this PR. We can also do it separately. None of our currently shipping targets care about the difference anyway.

Habbie · 2025-09-04T09:27:49Z

any opinions on the matter? Code changes needed are fairly minimal and we get a few nice features, so I am inclined to go for C++20, but I won't press it.

The code changes look entirely acceptable to me. I have no objections to the move.

pieterlexis · 2025-09-04T10:37:45Z

I have no issues with moving to C++20 :)

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

rgacogne · 2025-09-04T12:58:47Z

Moving to C++20 sounds like a good idea to me. Do we need to test some additional distributions, like CentOS/Amazon Linux/etc?

miodvallat · 2025-09-04T13:00:22Z

Doesn't the CI run test these systems already? Or maybe you should trigger a tentative package build and see how well it fares.

omoerbeek · 2025-09-04T13:01:23Z

Doesn't the CI run test these systems already? Or maybe you should trigger a tentative package build and see how well it fares.

I tested a few, see above.

rgacogne · 2025-09-04T13:02:59Z

But not the ones tested by https://github.com/PowerDNS/pdns/blob/master/.github/workflows/builder.yml, right?

omoerbeek · 2025-09-04T13:06:15Z

But not the ones tested by https://github.com/PowerDNS/pdns/blob/master/.github/workflows/builder.yml, right?

Nope, there's no convenient button to test a specific branch there. Do you know an easy way to test this branch?

rgacogne · 2025-09-04T13:08:35Z

It is a reusable workflow but there does not seem to be any dispatcher for it. Give me a sec.

rgacogne · 2025-09-04T14:24:02Z

It worked: https://github.com/rgacogne/pdns/actions/runs/17466511081

omoerbeek · 2025-09-04T14:27:12Z

It worked: https://github.com/rgacogne/pdns/actions/runs/17466511081

Thanks, so the only thing left open is the warnings from tcpiohandler.hh mentioned above and maybe a switch to gnu++20. Both can be done later if we decide so.

rgacogne · 2025-09-04T14:37:04Z

For the tcpiohandler.hh warnings moving to std::atomic<std::shared_ptr<T>> makes sense to me, as it would also prevent accidentally accessing the shared pointer in a non-atomic way.

rgacogne · 2025-09-04T14:37:59Z

I'm also OK with switching dnsdist to gnu++20, we can always revert if we spot a problem later.

omoerbeek · 2025-09-08T06:53:20Z

For the tcpiohandler.hh warnings moving to std::atomic<std::shared_ptr<T>> makes sense to me, as it would also prevent accidentally accessing the shared pointer in a non-atomic way.

This is going to be problematic, as clang's libc++ does not seem to support that yet: https://en.cppreference.com/w/cpp/20.html search for "Atomic std::shared_ptr "

So the situation is: using GCC libstdc++ it will nag about using a deprecated feature, but we cannot switch until std::atomic<std::shared_ptr<T>> is supported on all our supported targets. We might use pragma's to get rid of the warnings on GCC libstdc++ platforms. Only when we arrive at c++26 this will break.

rgacogne · 2025-09-08T07:06:22Z

This is great.. I wonder if we can detect whether support is available without too much hassle.

omoerbeek · 2025-09-08T07:39:54Z

This is great.. I wonder if we can detect whether support is available without too much hassle.

C++20 comes with feature detection macros: https://en.cppreference.com/w/cpp/experimental/feature_test.html, __cpp_lib_atomic_shared_ptr. I'll give it a shot.

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

omoerbeek · 2025-09-08T09:13:55Z

As said in the commit message: I'm not really happy with the last commit. Soliciting feedback before going further on this road. WATCH OUT: only compile tested as well. So move to Draft.

miodvallat · 2025-09-08T09:17:02Z

pdns/tcpiohandler.cc

+  auto tmp = parentLoad();
+  if (tmp) {


Suggested change

auto tmp = parentLoad();

if (tmp) {

if (auto tmp = parentLoad(); tmp) {

miodvallat · 2025-09-08T09:19:10Z

pdns/tcpiohandler.hh

+    auto tmp = load();
+    if (tmp) {


Suggested change

auto tmp = load();

if (tmp) {

if (auto tmp = load(); tmp) {

miodvallat · 2025-09-08T09:20:48Z

As said in the commit message: I'm not really happy with the last commit. Soliciting feedback before going further on this road.

This might inded become error-prone if someone adds/edits code without realizing there are two possible code paths to use depending on library support.

rgacogne · 2025-09-08T09:49:51Z

As said in the commit message: I'm not really happy with the last commit. Soliciting feedback before going further on this road.

This might inded become error-prone if someone adds/edits code without realizing there are two possible code paths to use depending on library support.

Agreed. The options I see are:

keep the current code, ignore the warnings
write a wrapper class that hides the complexity in a single place

omoerbeek · 2025-09-08T09:52:38Z

I think we can live with the deprecation warnings until it itches too much for somebody.

… e.g. Suse and Haiku easier. Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

omoerbeek · 2025-09-08T12:24:33Z

It turns out that both dnsdist and auth still have trouble building on el-8 and bullseye. See https://github.com/omoerbeek/pdns/actions/runs/17548656275 and https://github.com/omoerbeek/pdns/actions/runs/17548660109.
I managed to build dnsdist on el-8 by switching to boost1.78, but for auth that looks to be a hassle, as it does not have a single BOOST_ROOT. Or is there a method I'm missing?

Did not look into bullseye yet. The issues might be the same as el-8 or not.

Habbie · 2025-09-08T12:26:37Z

Or is there a method I'm missing?

until 3a03e2d, pdns.spec had:

%if 0%{?rhel} < 8
export CPPFLAGS=-I/usr/include/boost169
export LDFLAGS=-L/usr/lib64/boost169
%endif

which apparently was enough

omoerbeek · 2025-09-08T12:51:27Z

That worked! So I have dnsdist and auth now also building on el-8 locally.

bullseye shows for auth:

#40 31.52 checking whether g++ supports C++20 features with -std=gnu++20... no
#40 31.57 configure: error: *** A compiler with support for C++20 language features is required.
#40 31.61 	tail -v -n \+0 config.log

omoerbeek · 2025-09-08T12:57:34Z

dnsdist on bulleye also goes wrong:

#32 98.77 In file included from dnsdist-rust-lib/dnsdist-configuration-yaml-items-generated.cc:25:
#32 98.77 In file included from ../dnsdist-configuration.hh:24:
#32 98.77 In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/functional:54:
#32 98.77 In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/tuple:39:
#32 98.77 In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/array:40:
#32 98.77 In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_algobase.h:65:
#32 98.77 In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/stl_iterator_base_types.h:71:
#32 98.77 In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/iterator_concepts.h:36:
#32 98.77 /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/ptr_traits.h:119:7: error: static_assert failed due to requirement '!is_same<std::__undefined, std::__undefined>::value' "pointer type defines element_type or is like SomePointer<T, Args>"
#32 98.77       static_assert(!is_same<element_type, __undefined>::value,
#32 98.77       ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#32 98.77 /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/ptr_traits.h:171:22: note: in instantiation of template class 'std::pointer_traits<rust::cxxbridge1::Slice<const unsigned char>::iterator>' requested here
#32 98.77     -> decltype(std::pointer_traits<_Ptr>::to_address(__ptr))
#32 98.77                      ^
#32 98.77 /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/ptr_traits.h:207:14: note: while substituting deduced template arguments into function template '__to_address' [with _Ptr = rust::cxxbridge1::Slice<const unsigned char>::iterator]
#32 98.77     { return std::__to_address(__ptr); }
#32 98.77              ^
#32 98.77 /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/iterator_concepts.h:635:9: note: in instantiation of function template specialization 'std::to_address<rust::cxxbridge1::Slice<const unsigned char>::iterator>' requested here
#32 98.77         { std::to_address(__i) }
#32 98.77                ^
#32 98.77 /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/iterator_concepts.h:635:4: note: in instantiation of requirement here
#32 98.77         { std::to_address(__i) }
#32 98.77           ^~~~~~~~~~~~~~~~~~~~
#32 98.77 /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/iterator_concepts.h:633:10: note: while substituting template arguments into constraint expression here
#32 98.77       && requires(const _Iter& __i)
#32 98.77          ^~~~~~~~~~~~~~~~~~~~~~~~~~
#32 98.77 /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/range_access.h:939:37: note: while checking the satisfaction of concept 'contiguous_iterator<rust::cxxbridge1::Slice<const unsigned char>::iterator>' requested here
#32 98.77       = random_access_range<_Tp> && contiguous_iterator<iterator_t<_Tp>>
#32 98.77                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#32 98.77 /usr/bin/../lib/gcc/x86_64-linux-gnu/10/../../../../include/c++/10/bits/range_access.h:939:37: note: while substituting template arguments into constraint expression here
#32 98.77       = random_access_range<_Tp> && contiguous_iterator<iterator_t<_Tp>>
#32 98.77                                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#32 98.77 dnsdist-rust-lib/rust/cxx.h:282:15: note: while checking the satisfaction of concept 'contiguous_range<rust::cxxbridge1::Slice<const unsigned char>>' requested here
#32 98.77 static_assert(std::ranges::contiguous_range<rust::Slice<const uint8_t>>);
#32 98.77               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
#32 98.77 1 error generated.

omoerbeek · 2025-09-08T12:58:41Z

I'll commit what I have now. bullseye seems to be a road block until somebody finds some time..

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

modules/geoipbackend/geoipbackend.cc

omoerbeek added 2 commits September 4, 2025 10:15

Avoid a few cases of warning: C++20 says that these are ambiguous, ev…

397a7db

…en though the second is reversed Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

c++20 does not allow implicit capture of this

533c5b9

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

omoerbeek added this to the common-soon milestone Sep 4, 2025

omoerbeek added auth rec dnsdist labels Sep 4, 2025

omoerbeek added 2 commits September 4, 2025 10:26

Avoid two cases of too early instantation of a member of a vector of …

b5a2f95

…an incomplete class Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

Bump minimal c++ version to c++20

f9e42a3

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

omoerbeek force-pushed the c++20-for-real branch from 58483c0 to f9e42a3 Compare September 4, 2025 08:26

Missing include

344eed4

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

Habbie reviewed Sep 4, 2025

View reviewed changes

Tidy

5456db4

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

Fix two more cases of ambiguous operator calls

9c7571b

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

omoerbeek marked this pull request as draft September 8, 2025 09:12

omoerbeek force-pushed the c++20-for-real branch from 001bdc3 to 2f41821 Compare September 8, 2025 09:14

miodvallat reviewed Sep 8, 2025

View reviewed changes

Use gnu++20 instead of c++20, according to Habbie it makes porting to…

0aacc6c

… e.g. Suse and Haiku easier. Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

omoerbeek force-pushed the c++20-for-real branch from 2f41821 to 0aacc6c Compare September 8, 2025 10:14

Various fixes to make dnsdist and auth work on el-8

9e72f6a

Signed-off-by: Otto Moerbeek <otto.moerbeek@open-xchange.com>

pieterlexis reviewed Sep 8, 2025

View reviewed changes

modules/geoipbackend/geoipbackend.cc Show resolved Hide resolved

	auto tmp = parentLoad();
	if (tmp) {
	if (auto tmp = parentLoad(); tmp) {

Move to C++20 #16077

Are you sure you want to change the base?

Move to C++20 #16077

Conversation

omoerbeek commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Short description

Checklist

Uh oh!

miodvallat commented Sep 4, 2025

Uh oh!

coveralls commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 17551617099

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

omoerbeek commented Sep 4, 2025

Uh oh!

omoerbeek commented Sep 4, 2025

Uh oh!

Habbie commented Sep 4, 2025

Uh oh!

Habbie Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

omoerbeek Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Habbie Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Habbie commented Sep 4, 2025

Uh oh!

pieterlexis commented Sep 4, 2025

Uh oh!

rgacogne commented Sep 4, 2025

Uh oh!

miodvallat commented Sep 4, 2025

Uh oh!

omoerbeek commented Sep 4, 2025

Uh oh!

rgacogne commented Sep 4, 2025

Uh oh!

omoerbeek commented Sep 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgacogne commented Sep 4, 2025

Uh oh!

rgacogne commented Sep 4, 2025

Uh oh!

omoerbeek commented Sep 4, 2025

Uh oh!

rgacogne commented Sep 4, 2025

Uh oh!

rgacogne commented Sep 4, 2025

Uh oh!

omoerbeek commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rgacogne commented Sep 8, 2025

Uh oh!

omoerbeek commented Sep 8, 2025

Uh oh!

omoerbeek commented Sep 8, 2025

Uh oh!

miodvallat Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

miodvallat Sep 8, 2025

Choose a reason for hiding this comment

Uh oh!

miodvallat commented Sep 8, 2025

Uh oh!

rgacogne commented Sep 8, 2025

Uh oh!

omoerbeek commented Sep 8, 2025

Uh oh!

omoerbeek commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

omoerbeek commented Sep 4, 2025 •

edited

Loading

coveralls commented Sep 4, 2025 •

edited

Loading

omoerbeek commented Sep 4, 2025 •

edited

Loading

omoerbeek commented Sep 8, 2025 •

edited

Loading

omoerbeek commented Sep 8, 2025 •

edited

Loading

omoerbeek commented Sep 8, 2025 •

edited

Loading

omoerbeek commented Sep 8, 2025 •

edited

Loading