Fixing warning with clang 7.x

andrsd commented 5 years ago

clang 7.x suggest to use std::move. Otherwise is generates a warning so when people have -Werror enabled the build fails for them.

andrsd commented 5 years ago

Not knowing the code myself, this is what clang suggested how to fix it. Feel free to merge or patch in other way, @roystgnr .

roystgnr commented 5 years ago

What's the specific warning text? It does look as if clang is making a very good optimization recommendation here but I'd like to know any more details.

andrsd commented 5 years ago

It looks like this when building MOOSE:

/path/moose/scripts/../libmesh/installed/include/metaphysicl/dualnumber.h:406:1: error: local variable 'in' will be copied despite being returned by name [-Werror,-Wreturn-std-move]
DualNumber_std_unary(abs, (in.value() > 0) - (in.value() < 0),) // std < and > return 0 or 1
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/path/moose/scripts/../libmesh/installed/include/metaphysicl/dualnumber.h:372:10: note: expanded from macro 'DualNumber_std_unary'
  return in; \
         ^~
/path/moose/framework/build/header_symlinks/MooseUtils.h:235:16: note: in instantiation of function template specialization 'std::abs<double, MetaPhysicL::NumberArray<50, double> >'
      requested here
  return (std::abs(var1 - var2) <= tol);
               ^
/path/moose/framework/build/header_symlinks/MooseUtils.h:300:11: note: in instantiation of function template specialization
      'MooseUtils::absoluteFuzzyEqual<MetaPhysicL::DualNumber<double, MetaPhysicL::NumberArray<50, double> >, MetaPhysicL::DualNumber<double, MetaPhysicL::NumberArray<50, double> >,
      MetaPhysicL::DualNumber<double, MetaPhysicL::NumberArray<50, double> >, 0>' requested here
  return (absoluteFuzzyEqual(var1, var2, tol * (std::abs(var1) + std::abs(var2))));
          ^
/path/moose/framework/src/utils/RankFourTensor.C:901:20: note: in instantiation of function template specialization 'MooseUtils::relativeFuzzyEqual<MetaPhysicL::DualNumber<double,
      MetaPhysicL::NumberArray<50, double> >, MetaPhysicL::DualNumber<double, MetaPhysicL::NumberArray<50, double> >, 0>' requested here
  if (!MooseUtils::relativeFuzzyEqual(K1 - 4.0 * mu / 3.0, K2 + 2.0 * mu / 3.0))
                   ^
/path/moose/scripts/../libmesh/installed/include/metaphysicl/dualnumber.h:406:1: note: call 'std::move' explicitly to avoid copying
DualNumber_std_unary(abs, (in.value() > 0) - (in.value() < 0),) // std < and > return 0 or 1
^
/path/moose/scripts/../libmesh/installed/include/metaphysicl/dualnumber.h:372:10: note: expanded from macro 'DualNumber_std_unary'
  return in; \
         ^

lindsayad commented 5 years ago

@roystgnr I hate to do this a day after tagging a release, but for idaholab/package_builder#189 (e.g. upgrading MOOSE compilers to gcc 8 and clang 7), I would like to boostrap this PR. Are you alright with that? Also could increment the patch version if you think silencing the compiler warning/performing this optimization is worth that.

roystgnr commented 5 years ago

Encouraging everybody to use -Werror all the time is probably my fault, isn't it? Even if https://github.com/libMesh/libmesh/pull/1797 isn't what got the ball rolling there, I'm certainly not going to complain about annoying consequences I was asking for in the first place. I'm okay with either solution, although if you can't decide then I'd say just bootstrap from this PR; any bug fix is worth a version increment but this isn't technically a bug fix.

roystgnr / MetaPhysicL

Fixing warning with clang 7.x #41