Skip to content

Commit

Permalink
Work around GCC 5.4 bug in Wshadow (RobotLocomotion#8339)
Browse files Browse the repository at this point in the history
The problem is that variables within a generic lambda are misinterpreted
as shadowing the varablies in the code that calls the lambda.  The GCC
bug report is https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67273.

We can revert this patch once GCC 6 is our minimum supported version.
  • Loading branch information
jwnimmer-tri authored Mar 13, 2018
1 parent 0bf6d9d commit 4cef1e4
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 15 deletions.
2 changes: 1 addition & 1 deletion bindings/pydrake/util/test/wrap_function_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ GTEST_TEST(WrapFunction, FunctionPointer) {
// Lambdas / basic functors.
GTEST_TEST(WrapFunction, Lambda) {
int value{0};
auto func_1_lambda = [](int value) {};
auto func_1_lambda = [](int) {};
WrapIdentity(func_1_lambda)(value);

std::function<void(int)> func_1_func = func_1_lambda;
Expand Down
10 changes: 10 additions & 0 deletions math/test/jacobian_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,12 @@ TEST_F(AutodiffJacobianTest, QuadraticForm) {
Matrix3d A;
FillWithNumbersIncreasingFromZero(A);

// Work around GCC 5.4 Wshadow bug; the bug is fixed as of GCC 6.1.
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67273.
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wshadow"
auto quadratic_form = [&](const auto& x) {
#pragma GCC diagnostic pop
using Scalar = typename std::remove_reference<decltype(x)>::type::Scalar;
return (x.transpose() * A.cast<Scalar>().eval() * x).eval();
};
Expand Down Expand Up @@ -98,7 +103,12 @@ TEST_F(AutoDiffHessianTest, QuadraticFunction) {
FillWithNumbersIncreasingFromZero(D);
FillWithNumbersIncreasingFromZero(e);

// Work around GCC 5.4 Wshadow bug; the bug is fixed as of GCC 6.1.
// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67273.
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wshadow"
auto quadratic_function = [&](const auto& x) {
#pragma GCC diagnostic pop
using Scalar = typename std::remove_reference<decltype(x)>::type::Scalar;
return ((A.cast<Scalar>() * x + b.cast<Scalar>()).transpose() *
C.cast<Scalar>() * (D.cast<Scalar>() * x + e.cast<Scalar>()))
Expand Down
37 changes: 23 additions & 14 deletions systems/framework/test_utilities/scalar_conversion.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,21 @@ static T copysign_int_to_non_symbolic_scalar(int magic, const T& value) {
template <template <typename> class S, typename Callback>
::testing::AssertionResult is_autodiffxd_convertible(
const S<double>& dut, Callback callback) {
// We must use salted local variable names ("_67273" suffix) to work around
// GCC 5.4 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67273 because the
// `callback` is a generic lambda. The bug is fixed as of GCC 6.1.

// Check if a proper type came out; return early if not.
std::unique_ptr<System<AutoDiffXd>> converted = dut.ToAutoDiffXdMaybe();
::testing::AssertionResult result =
is_dynamic_castable<S<AutoDiffXd>>(converted);
if (!result) { return result; }
std::unique_ptr<System<AutoDiffXd>> converted_67273 =
dut.ToAutoDiffXdMaybe();
::testing::AssertionResult result_67273 =
is_dynamic_castable<S<AutoDiffXd>>(converted_67273);
if (!result_67273) { return result_67273; }

// Allow calling code to specify additional tests on the converted System.
const S<AutoDiffXd>& downcast =
dynamic_cast<const S<AutoDiffXd>&>(*converted);
callback(downcast);
const S<AutoDiffXd>& downcast_67273 =
dynamic_cast<const S<AutoDiffXd>&>(*converted_67273);
callback(downcast_67273);

return ::testing::AssertionSuccess();
}
Expand All @@ -61,17 +66,21 @@ ::testing::AssertionResult is_autodiffxd_convertible(const S<double>& dut) {
template <template <typename> class S, typename Callback>
::testing::AssertionResult is_symbolic_convertible(
const S<double>& dut, Callback callback) {
// We must use salted local variable names ("_67273" suffix) to work around
// GCC 5.4 bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67273 because the
// `callback` is a generic lambda. The bug is fixed as of GCC 6.1.

// Check if a proper type came out; return early if not.
std::unique_ptr<System<symbolic::Expression>> converted =
std::unique_ptr<System<symbolic::Expression>> converted_67273 =
dut.ToSymbolicMaybe();
::testing::AssertionResult result =
is_dynamic_castable<S<symbolic::Expression>>(converted);
if (!result) { return result; }
::testing::AssertionResult result_67273 =
is_dynamic_castable<S<symbolic::Expression>>(converted_67273);
if (!result_67273) { return result_67273; }

// Allow calling code to specify additional tests on the converted System.
const S<symbolic::Expression>& downcast =
dynamic_cast<const S<symbolic::Expression>&>(*converted);
callback(downcast);
const S<symbolic::Expression>& downcast_67273 =
dynamic_cast<const S<symbolic::Expression>&>(*converted_67273);
callback(downcast_67273);

return ::testing::AssertionSuccess();
}
Expand Down

0 comments on commit 4cef1e4

Please sign in to comment.