clang-tidy performance findings

[masterleinad]

Working with @tjhei on doing CI with clang-tidy we found some more places to fix.

[bangerth]

Hm, this and all of the other places you modify in a similar way make use of the fact that the initializer of the reference is a temporary whose lifetime is extended to the one of the reference (if dim>1) or it's an object of the same time (if dim==1). (In the mapping case above, it's either a copy of a MappingCollection, or a MappingCollection temp object created from a mapping.)

I find it unappealing to have these references because the semantics of initialization of a reference to a temporary are so difficult to understand.

[tjhei]

how about we replace the code with:

if (dim==1)
 this->unit_support_points = points.get_points();
else
  this->unit_support_points = Quadrature<dim>(points).get_points();

This makes it more clear what is happening and should avoid the warning.

[bangerth]

But that just works around the warning, doesn't it? I mean, clang presumably only complains in the 1d case because it says that there is no need to actually copy the quadrature formula, it's enough to take a reference. And in 2d/3d, putting a reference to the variable just happens to work. (Well, it's not coincidence, of course.) But in your replacement code, you still copy the argument.

[tjhei]

But in your replacement code, you still copy the argument.

I don't understand. I am not making a copy, the last line constructs a dim-dimensional quadrature from a 1 dimensional one.

[masterleinad]

Unfortunately, that doesn't compile without using if constexpr 😕

[masterleinad]

Now, I just mark all the cases where we would create a reference to a local copy as NOLINT and clang-tidy ignores them.

[masterleinad]

/run-tests

[tjhei]

Does anybody want to hit "merge" here? I think this is good to go.