boostorg / multiprecision

Boost.Multiprecision
Boost Software License 1.0
194 stars 111 forks source link

Make expression templates implicitly convert... #543

Closed jzmaddock closed 1 year ago

jzmaddock commented 1 year ago

to types that the number type will also convert to. Fixes: https://github.com/boostorg/multiprecision/issues/541.

jzmaddock commented 1 year ago

Only failure is a network issue, merging.

afabri commented 1 year ago

Is this already in boost 1.82 ?

jzmaddock commented 1 year ago

It is, however, this https://github.com/boostorg/multiprecision/pull/545 has already cropped up, and as I fear this is the tip of an iceberg shaped tricky problem, I'm contemplating pulling the fix from 1.82.

lrineau commented 1 year ago

This code does not compile with Boost-1.82 but compiled with previous versions. Is that related?

#include <boost/multiprecision/cpp_int.hpp>

using Rational = boost::multiprecision::cpp_rational;

template <typename N> struct Sqrt {
  Sqrt() {}
  Sqrt(N) {}
  friend Sqrt<N> operator*(const Sqrt<N>& x, const Sqrt<N>&) { return x;}
  friend Sqrt<N> operator*(const N&, const Sqrt<N>& x) { return x;}
};

int main() {
  Rational r;
  Sqrt<Rational> s;

  Rational(r - r) * s;
  Sqrt<Rational> toto(r - r);
  Sqrt<Rational>(r - r) * s;
  (r - r) * s;  // ambiguous call to `operator*` here
}
jzmaddock commented 1 year ago

Yes, confirmed.

I've asked for permission to pull this fix from 1.82. It may even be that the issue is basically unfixable :(

afabri commented 1 year ago

It works for mpq_class which (we should double check) also uses expression templates.

lrineau commented 1 year ago

I agree this PR should be reverted. The class mpq_class from <gmpxx.h> does not has those operator T() to convert expression to everything. Instead their equivalent of number has non-explicit conversion constructors from the expression templates.

Since this PR, Boost MP expression templates are convertible to too many types, and that can lead to compilation errors with ambiguous calls (like https://github.com/boostorg/multiprecision/pull/543#issuecomment-1481575350).

jzmaddock commented 1 year ago

Reverted in develop and master.