Opened 8 years ago

Last modified 7 years ago

#11164 new Bugs

Graph adjacency_list, add_vertex compile error on clang 3.6

Reported by: dstoeckel@… Owned by: Jeremiah Willcock
Milestone: To Be Determined Component: graph
Version: Boost 1.57.0 Severity: Problem
Keywords: Cc:

Description

I have an issue similar to #10382. When compiling the following program

#include <boost/graph/adjacency_list.hpp>

struct Payload {
        Payload() = default;
        Payload(const Payload&) {};
        Payload& operator=(const Payload&) { return *this; };
};

struct PayloadD : public Payload {
};

class Props {
  public:
    PayloadD payload;
};

int main(int argc, char** argv)
{
  using boost::adjacency_list;
  using boost::vecS;
  using boost::directedS;
  typedef adjacency_list<vecS, vecS, directedS, Props> Graph;

  Graph graph;
  boost::add_vertex(graph);

  return 0;
}

Using clang 3.6 I get the error message:

/usr/bin/../lib64/gcc/x86_64-unknown-linux-gnu/4.9.2/../../../../include/c++/4.9.2/bits/stl_construct.h:75:38: error: call to implicitly-deleted copy constructor of 'boost::detail::stored_edge_property<unsigned long, boost::no_property>'
    { ::new(static_cast<void*>(__p)) _T1(std::forward<_Args>(__args)...); }

...

/usr/include/boost/graph/detail/adjacency_list.hpp:317:7: note: copy constructor is implicitly deleted because 'stored_edge_property<unsigned long, boost::no_property>' has a user-declared move constructor
      stored_edge_property(self&& x) = default;

The same program compiles fine with gcc 4.8.2, 4.9.2, and clang 3.5.

The attached patch changes stored_edge_property such that it always creates a custom copy constructor and copy assignment operator. With this changes clang 3.6 can build the program.

Attachments (1)

fix_stored_edge_property.patch (1.5 KB ) - added by dstoeckel@… 8 years ago.
Always declare a copy constructor and copy assignment operator.

Download all attachments as: .zip

Change History (4)

by dstoeckel@…, 8 years ago

Always declare a copy constructor and copy assignment operator.

comment:1 by anonymous, 7 years ago

This patch seems obviously wrong. The type is supposed to be move-only, and tried to emulate that in C++03 with weird mutating auto_ptr-style copy operations. In C++11 that isn't needed because you can make the type move-only, but your patch would restore the weird not-really-a-copy-constructor.

comment:2 by anonymous, 7 years ago

Your Payload type needs to be nothrow-move-constructible, so that adding a vertex to the graph doesn't try to copy it, but moves it instead.

comment:3 by anonymous, 7 years ago

Ok, that explains a lot. In addition it needs to be remain default constructible, and if the containing adjacency list is copied also copy-assignable/constructible. This is also stated in the documentation:

http://www.boost.org/doc/libs/1_60_0/libs/graph/doc/adjacency_list.html:

The types of all property values must be Copy Constructible, Assignable, and Default Constructible.

and also in practice:

boost/graph/detail/adjacency_list.hpp:2156

m_vertices[v].m_property = x.m_vertices[i].m_property;

The reason why I proposed the above patch was that I have a legacy C++98 code base that broke due to this requirement. Actually, every property requiring a custom copy-constructor/assignment operator should be hit by this. While this was fine before, now the full set of constructors/operators needs to be implemented...

Fortunately, in my case adapting the code and sprinkling in some #ifdefs for older/non-standard platforms is unproblematical.

However, to avoid confusion, the documentation should be updated to reflect the changed requirements.

Note: See TracTickets for help on using tickets.