fix C++17 build

[minijackson]

While trying to compile pvxs to see if I could package it using meson, I came across an unrelated compilation error:

pvxs> In file included from ../src/describe.cpp:19:
pvxs> ../src/pvxs/client.h: In static member function 'static pvxs::client::RequestBuilder pvxs::client::Context::request()':
pvxs> ../src/pvxs/client.h:860:59: error: 'pvxs::client::detail::CommonBuilder<SubBuilder, Base>::CommonBuilder() [with SubBuilder = pvxs::client::RequestBuilder; Base = pvxs::client::detail::CommonBase]' is protected within this context
pvxs>   860 | RequestBuilder Context::request() { return RequestBuilder{}; }
pvxs>       |                                                           ^
pvxs> ../src/pvxs/client.h:595:5: note: declared protected here
pvxs>   595 |     CommonBuilder() = default;
pvxs>       |     ^~~~~~~~~~~~~

And after a bit of searching for the cause, I found that this error is present only when the C++ standard is c++17 or above on both GCC and Clang:

https://godbolt.org/z/cW18s13fo

As I understood it, C++17 allows aggregate initialisation for classes which have public base classes. This changes the line from a statement calling the default constructor, to a statement performing aggregate initialisation, and the base class is default initialized.

In other words, in C++17, the line:

return RequestBuilder{};

is understood as:

return RequestBuilder{CommonBuilder<...>{}};

And since the default-constructor of CommonBuilder is protected, GCC and Clang errors out.

Here, we explicitely call the default constructor, implicitely defined in RequestBuilder.

The only weird thing I can't wrap my head around, is why if the base class has no other constructor than the default one, no error is produced...

This fixes builds using GCC 11+ since it defaults to C++17.

[AppVeyorBot]

:white_check_mark: Build pvxs 1.0.807 completed (commit https://github.com/mdavidsaver/pvxs/commit/78450d7e10 by @minijackson)

[mdavidsaver]

Thanks for reporting this. I've added a CI build with std=c++17 (edbcd46a5f41366e9601eb9832cd61c260ff58e4) which I hope will catch any future issues like this.

This isn't the first time I've look through it, and still don't think that I fully understand all of the nuance of that cppreference.com page. Maybe better to avoid mixing aggregates and inheritance. I've opted for a different, and more verbose, change 4164d1b8fb3afeba75b91a5d6d5dfa96fc417733 which replaces the implicit default constructors with explicit empty constructors.

I don't think there is any downside to doing this? Except possibly some missed opportunities for an implicit constexpr (which seems trivial).

This is my first big project being able to use >= c++11 features. If I'm being honest with myself, many of these = default are there "because I could". So, a lesson (re)learned...

[minijackson]

I think that's a good solution. I've been told that C++20 allows aggregate initialization with a parenthesized list of values. I would have thought that my solution would not work in C++20, but apparently it does? Not sure why...

In any case, I think we can close this PR