From: Wolfgang Bangerth <bangerth@colostate.edu>
Date: Sun, 15 Jul 2018 10:55:27 +0000 (-0600)
Subject: Clean up some code in AffineConstraints::is_consistent_in_parallel.
X-Git-Tag: v9.1.0-rc1~917^2
X-Git-Url: https://gitweb.dealii.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=af8de1503c1a17253e9e8b826fbd072ba57dbe24;p=dealii.git

Clean up some code in AffineConstraints::is_consistent_in_parallel.

The existing code has a lambda function that either returns a reference to
an existing element of a vector, or to a dummy member variable. But to
make things confusing, it *modifies* this dummy variable, which means that
what looks like some 'const' dummy object is not actually.

All of this is done to avoid copies, but in reality a copy is still
made at a later point where we insert the result of this lambda
function into a vector. This patch cleans this up by removing the
non-const dummy variable and instead just returning a copy that is
then moved into the vector. That's the same number of copies as before,
in most cases, and it's clearer to read because there is no more
potential for two parts of the code stepping on each others' toes
modifying the dummy variable.

These things are important to me because modifying the dummy variable
is a sure way to introduce really hard to find errors if anyone ever
wanted to multi-thread this function. It's not likely that anyone
will try this for this function soon, except possibly by using
things like parallel-for loops should they ever become available.
But it's worth not even getting into the habit of using this style.
---

diff --git a/include/deal.II/lac/affine_constraints.templates.h b/include/deal.II/lac/affine_constraints.templates.h
index c86903695e..291a7ee420 100644
--- a/include/deal.II/lac/affine_constraints.templates.h
+++ b/include/deal.II/lac/affine_constraints.templates.h
@@ -130,18 +130,15 @@ AffineConstraints<number>::is_consistent_in_parallel(
   const MPI_Comm               mpi_communicator,
   const bool                   verbose) const
 {
-  ConstraintLine empty;
-  empty.inhomogeneity = 0.0;
-
-  // Helper to return a reference to the ConstraintLine object that belongs to row @p row.
-  // We don't want to make copies but to return a reference, we need an empty
-  // object that we store above.
-  auto get_line = [&](const size_type row) -> const ConstraintLine & {
+  // Helper to return a ConstraintLine object that belongs to row @p row.
+  // If @p row is not constrained or not stored locally, return an empty
+  // constraint object that would correspond to a zero constraints
+  auto get_line = [&](const size_type row) -> ConstraintLine {
     const size_type line_index = calculate_line_index(row);
     if (line_index >= lines_cache.size() ||
         lines_cache[line_index] == numbers::invalid_size_type)
       {
-        empty.index = row;
+        const ConstraintLine empty = {row, {}, 0.0};
         return empty;
       }
     else
@@ -167,7 +164,7 @@ AffineConstraints<number>::is_consistent_in_parallel(
       IndexSet indices_to_send = non_owned & locally_owned_dofs[owner];
       for (const auto row_idx : indices_to_send)
         {
-          to_send[owner].push_back(get_line(row_idx));
+          to_send[owner].emplace_back(get_line(row_idx));
         }
     }
 
@@ -182,7 +179,7 @@ AffineConstraints<number>::is_consistent_in_parallel(
       // for each incoming line:
       for (auto &lineit : kv.second)
         {
-          const ConstraintLine &reference = get_line(lineit.index);
+          const ConstraintLine reference = get_line(lineit.index);
 
           if (lineit.inhomogeneity != reference.inhomogeneity)
             {