Prevent GMP values in lambdas from outliving their stack frame

[sethfowler]

So I recently noticed that while p4c builds on macOS, the tests fail. I investigated a little and tracked down the issue. Here's what's happening:

In constantFolding.cpp, the binary operators are handling by a common method, DoConstantFolding::binary(), which takes a std::function as an argument. The std::function accepts two GMP values (i.e., mpz_class values) and performs the appropriate binary operation, returning a new value. Here's an example:

return binary(e, [](mpz_class a, mpz_class b) { return a + b; });

The problem here is that the return type of these lambdas are inferred, and the inferred type is not mpz_class. For this example, the inferred return type is actually:

__gmp_expr<__mpz_struct [1], __gmp_binary_expr<__gmp_expr<mpz_t, mpz_t>, __gmp_expr<mpz_t, mpz_t>, __gmp_binary_plus>> 

What's happening here is that operations on mpz_class values build up an expression tree which is then evaluated when it's assigned to another mpz_class value. This didn't get noticed because while binary() accepts a std::function<mpz_class(mpz_class, mpz_class)>, std::function will happily store a lambda whose return type is implicitly convertible to an mpz_class, which is the case for __gmp_expr.

So, why is this bad? Well, take a look at this code snippet from gmpxx.h:

template <class T, class U, class Op>
struct __gmp_binary_expr
{
  typename __gmp_resolve_ref<T>::ref_type val1;
  typename __gmp_resolve_ref<U>::ref_type val2;

  __gmp_binary_expr(const T &v1, const U &v2) : val1(v1), val2(v2) { }
private:
  __gmp_binary_expr();
};

As this code implies, __gmp_resolve_ref::ref_type is a reference. The lambda is returning a __gmp_expr that contains references to values which are on its stack frame. This is obviously ill-advised. =)

This works in GCC, presumably, because the code it generates doesn't clobber those values before the __gmp_expr is evaluated. Clang users are not so lucky, which means that DoConstantFolding::binary() ends up generating garbage values, causing tests to fail.

This patch adds explicit return types to all the lambdas in constantFolding.cpp that have this issue. This fixes the problem, since the implicit conversion happens before the stack frame gets popped, and with this patch applied all tests pass on macOS.

[ChrisDodd]

Interesting -- it works fine with clang on Linux (newer version of clang?), just fails on MacOS. This would seem like a bug in OSX clang as auto inferencing is never supposed to infer a reference type without an explicit reference after the auto (eg, auto & or auto &&). But in this case its an implicit auto as well.

[sethfowler]

@ChrisDodd the problem isn't that the lambda return type is inferred as a reference, it's that __gmp_expr stores references internally. Those operator overloads hide lots of magic. =)