std::complex::operator*= too restrictive with types it accepts

[llvmbot]

Bugzilla Link	19189
Version	unspecified
OS	All
Depends On	llvm/llvm-project#29937
Reporter	LLVM Bugzilla Contributor
CC	@mclow

Extended Description

Changeset r187529 for has imposed additional restrictions on the accepted types in operator*=. An example of the problematic change is below.

Where previously only the existence of

complex<_Tp> operator*(complex<_Tp>, complex<_Xp>)

was necessary for the implementation of operator*=, now a constructor _Tp(_Xp) is required.

Is this new constraint deliberately there?

An example situation where the new behaviour is problematic is for physical units types that should never be directly constructed from doubles, but which can be multiplied by doubles.

Index: complex

--- complex (revision 187528) +++ complex (revision 187529) @@ -309,12 +309,12 @@ } template _LIBCPP_INLINE_VISIBILITY complex& operator*=(const complex<_Xp>& __c) {

• this = this * __c;
• this = this complex(c.real(), c.imag()); return this; } template _LIBCPP_INLINE_VISIBILITY complex& operator/=(const complex<_Xp>& __c) {
• this = this / __c;
• this = this / complex(c.real(), c.imag()); return *this; } };

[hfinkel]

mentioned in issue llvm/llvm-project#29937

[mclow]

This is related to #​30589

[mclow]

Thanks for looking at this again.

You're right, operator* is only defined for two identical types. In my example of physical units, I have to (and can) define my own

template complex<Energy> operator * ( complex<Energy>, complex )

I ran across this yesterday, and it reminded me of this bug:

[complex.numbers]/2: The effect of instantiating the template complex for any type other than float, double, or long double is unspecified.

[llvmbot]

Thanks for looking at this again.

You're right, operator* is only defined for two identical types. In my example of physical units, I have to (and can) define my own

template complex<Energy> operator * ( complex<Energy>, complex )

However, I cannot define my own

template complex<Energy> & operator *= ( complex<Energy> &, complex )

because of ambiguous overload with std::complex::operator*=

[mclow]

I made that change.

The problem here (as I understand it) was that the only way we have to multiply two complex numbers requires them to both be of the same type.

The call in complex is on line #​587): template complex<_Tp> operator*(const complex<_Tp>& __z, const complex<_Tp>& __w);

To deal with that, I need to create a temporary complex of the desired type.

Consider the following code: complex i {1,1}; complex ld{3,3}; i *= ld;

This would not work with the pre-187529 code.

It's possible that a refactoring of the multiply code to be more like this:

template<class _Tp>
complex<_Tp>
__complex_multiply(const complex<_Tp>& __z, const _Tp&real, const _Tp &imag);

(and then have operator* call that) would re-enable the behavior that you want.

But I'd have to do some benchmarking, etc to make sure that adding an additional call doesn't impact performance too much.

[llvmbot]

assigned to @mclow