Compilation failure on Apple Clang 5.0.1

There is a problem with colouring data structure in MeshTopology.cpp:58.

The first argument of the map is a const, so once it is initialised it cannot be changed. An error is raised as soon as the assignment of an already initialised object is attempted.

As a proof of concept consider this:

int a(1.0);

const int b;
b = 2.0; // Error

const int c(a); // Ok

Possible solutions:

Remove const from definition. I have no clue on why first argument is const.

diff --git a/dolfin/mesh/Mesh.cpp b/dolfin/mesh/Mesh.cpp
index dc225a1..1bc1279 100644
--- a/dolfin/mesh/Mesh.cpp
+++ b/dolfin/mesh/Mesh.cpp
@@ -375,7 +375,7 @@ const std::vector<std::size_t>&
 Mesh::color(std::vector<std::size_t> coloring_type) const
 {
   // Find color data
-  std::map<const std::vector<std::size_t>, std::pair<std::vector<std::size_t>,
+  std::map<std::vector<std::size_t>, std::pair<std::vector<std::size_t>,
            std::vector<std::vector<std::size_t> > > >::const_iterator
     coloring_data = this->topology().coloring.find(coloring_type);

diff --git a/dolfin/mesh/MeshColoring.cpp b/dolfin/mesh/MeshColoring.cpp
index 540edf6..c7d763b 100644
--- a/dolfin/mesh/MeshColoring.cpp
+++ b/dolfin/mesh/MeshColoring.cpp
@@ -136,7 +136,7 @@ CellFunction<std::size_t> MeshColoring::cell_colors(const Mesh& mesh,
 {

   // Get color data
-  std::map<const std::vector<std::size_t>, std::pair<std::vector<std::size_t>,
+  std::map<std::vector<std::size_t>, std::pair<std::vector<std::size_t>,
            std::vector<std::vector<std::size_t> > > >::const_iterator coloring_data;
   coloring_data = mesh.topology().coloring.find(coloring_type);

diff --git a/dolfin/mesh/MeshRenumbering.cpp b/dolfin/mesh/MeshRenumbering.cpp
index 73e0c3e..c899c75 100644
--- a/dolfin/mesh/MeshRenumbering.cpp
+++ b/dolfin/mesh/MeshRenumbering.cpp
@@ -100,7 +100,7 @@ dolfin::Mesh MeshRenumbering::renumber_by_color(const Mesh& mesh,
   cout << "Close editor" << endl;

   // Initialise coloring data
-  typedef std::map<const std::vector<std::size_t>, std::pair<std::vector<std::size_t>,
+  typedef std::map<std::vector<std::size_t>, std::pair<std::vector<std::size_t>,
            std::vector<std::vector<std::size_t> > > >::const_iterator ConstMeshColoringData;

   // Get old coloring
@@ -172,7 +172,7 @@ void MeshRenumbering::compute_renumbering(const Mesh& mesh,
   const std::size_t coordinates_size = mesh.geometry().size()*mesh.geometry().dim();
   new_coordinates.resize(coordinates_size);

-  typedef std::map<const std::vector<std::size_t>, std::pair<std::vector<std::size_t>,
+  typedef std::map<std::vector<std::size_t>, std::pair<std::vector<std::size_t>,
            std::vector<std::vector<std::size_t> > > >::const_iterator MeshColoringData;

   info("Renumbering mesh by cell colors.");
diff --git a/dolfin/mesh/MeshTopology.h b/dolfin/mesh/MeshTopology.h
index 62669da..7dc65ae 100644
--- a/dolfin/mesh/MeshTopology.h
+++ b/dolfin/mesh/MeshTopology.h
@@ -139,7 +139,7 @@ namespace dolfin
     /// of color 'col'.
     // Developer note: std::vector is used in place of a MeshFunction
     //                 to avoid circular dependencies in the header files
-    std::map<const std::vector<std::size_t>,
+    std::map<std::vector<std::size_t>,
       std::pair<std::vector<std::size_t>, std::vector<std::vector<std::size_t> > > > coloring;

   private:

Smart pointer, and then use reset method in MeshTopology::operator=.
Remove assignment operator, which doesn't make any sense if attributes cannot be assigned too.

I would also add a typedef for the coloring type, it is easier to define iterators thereafter.

Moreover, I think that the copy constructor of MeshTopology is not correctly implemented, because it is based on operator=, so the object is first created (with default values for the attributes) and then assigned. Since there are no pointers in the classe, I would use the default (implicit) copy constructor.

Comments (1)