49

I was updating a project to use C++17 and found a few instances where code that followed this pattern was causing a compile error on recent versions of clang:

#include <boost/variant.hpp>

struct vis : public boost::static_visitor<void>
{
    void operator()(int) const { }
};

int main()
{
    boost::variant<int> v = 0;
    boost::apply_visitor(vis{}, v);
}

Using clang v8.0 in C++17 mode, this fails with the following error:

<source>:11:30: error: temporary of type 'boost::static_visitor<void>' has protected destructor
    boost::apply_visitor(vis{}, v);
                             ^
/opt/compiler-explorer/libs/boost_1_64_0/boost/variant/static_visitor.hpp:53:5: note: declared protected here
    ~static_visitor() = default;

However, it compiles cleanly in C++14 mode. I found that if I change the brace initialization vis{} to parentheses vis(), then it compiles correctly in both modes. Every version of gcc that I've tried allows both variants in C++17 mode.

Is this a correct change in behavior from C++14 to C++17, or is this a clang bug? If it is correct, why is it now invalid in C++17 (or maybe it always was, but clang just allows it in earlier standard revisions)?

Barry
  • 286,269
  • 29
  • 621
  • 977
Jason R
  • 11,159
  • 6
  • 50
  • 81
  • 7
    Boost [fixed](https://github.com/boostorg/variant/commit/dd728220b00772cee5d3aea5850a2bb09eecc66b#diff-e8948f8833a40a3c873b7f8d4ff5f9d7) this in 1.70. It's also discussed [here](https://reviews.llvm.org/D53860). – interjay May 29 '19 at 19:54

1 Answers1

63

clang is correct here. Here's a reduced example:

struct B {
protected:
    B() { }
};

struct D : B { };

auto d = D{};

In C++14, D is not an aggregate because it has a base class, so D{} is "normal" (non-aggregate) initialization which invokes D's default constructor, which in turn invokes B's default constructor. This is fine, because D has access to B's default constructor.

In C++17, the definition of aggregate was widened - base classes are now allowed (as long as they're non-virtual). D is now an aggregate, which means that D{} is aggregate initialization. And in aggregate-initialization, this means that we (the caller) are initializing all the subobjects - including the base class subobject. But we do not have access to B's constructor (it is protected), so we cannot invoke it, so it is ill-formed.


Fear not, the fix is easy. Use parentheses:

auto d = D();

This goes back to invoking D's default constructor as before.

Barry
  • 286,269
  • 29
  • 621
  • 977
  • 7
    Perfect, that's what I was looking for: the widened definition of aggregate is the culprit. – Jason R May 29 '19 at 19:56
  • Is clang right to be right? What is the accessibility context of the subobject initializer? There is a core language issue about that: [issue #2244](http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#2244). As a coder, I would prefer that name accessibility checks for [dcl.init.aggr/5.1](http://eel.is/c++draft/dcl.init.aggr#5.1) and [dcl.init.aggr/5.2](http://eel.is/c++draft/dcl.init.aggr#5.2) are done, in both cases, in the context of the aggregate. – Oliv May 30 '19 at 06:03
  • 2
    I heard that parentheses and braces are unified in C++20. Does that affect the validity of the fix? – L. F. May 30 '19 at 07:16
  • @L.F. No, it does not. `D()` being [value initialization](http://eel.is/c++draft/dcl.init#17.4) precedes the [new bullet](http://eel.is/c++draft/dcl.init#17.6.2.2). – Barry May 30 '19 at 13:34
  • @Oliv I think clang is right to be right. I don't know what the accessibility direction is. – Barry May 30 '19 at 13:36
  • 2
    Slightly wrong reduction. The issue in C++17 is the dtor (`static_visitor` is a C++17 aggregate itself, so the protectedness of the ctor doesn't matter). In C++20, both are problems and the boost fix (which fixed the dtor only) will break again, which makes this a nice example of the C++20 OMG-these-contrived-examples-are-so-surprising-we-must-fix-it change breaking things. – T.C. May 30 '19 at 15:35
  • @T.C. gcc and clang both complain about [the ctor](https://godbolt.org/z/G7sq0C)? – Barry May 30 '19 at 16:09
  • They both complain about the ctor on your (slightly wrong) reduced example. – T.C. May 30 '19 at 16:12
  • @T.C. Oh I see what you mean now. I guess it's a slightly different problem, with the same underlying reason. – Barry May 30 '19 at 16:26