[Bug libstdc++/51013] New: complex::{imag,real}() should maintain lvalue-returning extension in C++11

[Bug libstdc++/51013] New: complex::{imag,real}() should maintain lvalue-returning extension in C++11

[jyasskin at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

             Bug #: 51013
           Summary: complex::{imag,real}() should maintain
                    lvalue-returning extension in C++11
    Classification: Unclassified
           Product: gcc
           Version: 4.6.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: jyasskin@gcc.gnu.org


In C++98, libstdc++ has an extension to return T& from complex<T>::real() and
::imag() (introduced in r75680). Paolo removed this extension for C++11 in
r135878 citing
http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#387. However, the
point of DR387 was to increase the amount of access complex<> provides to its
internals, not to remove existing extensions providing such access.

Removing this extension affects code that passes references to the real or
imaginary parts of a complex number to other routines. While such code can
generally be rewritten in a straightforward way, it seems like an unnecessary
hurdle that should be avoided given all the other changes needed for C++11
compatibility.

The DR387-provided reinterpret_cast<> way of getting access to the addresses
would work if we were migrating to C++11 atomically, but that's impossible in a
large codebase. I believe the reinterpret_cast produces undefined behavior in
C++98 because of the aliasing violation, so we can't use it in code that needs
to be correct in both versions of the language.

So I think the extension should be reinstated.

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|paolo at gcc dot gnu.org    |gdr at gcc dot gnu.org

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-11-07 21:09:26 UTC ---
I did *not* *remove* anything, simply, for C++11, real() and imag() are exactly
*per the letter* of the Standard, that is (similarly for the specializations):

      constexpr _Tp 
      real() const { return _M_real; }

      constexpr _Tp 
      imag() const { return _M_imag; }

and I don't see why we should do something different. But, if Gaby thinks,
everything considered, that we want something different, I'm not going to
object to patches to that effect (from a technical point of view, note the
functions are now constexpr, thus you can't simply have two overloads anymore)

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

Marc Glisse <marc.glisse at normalesup dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |marc.glisse at normalesup
                   |                            |dot org

--- Comment #2 from Marc Glisse <marc.glisse at normalesup dot org> 2011-11-07 23:42:46 UTC ---
(In reply to comment #1)
> I did *not* *remove* anything, simply, for C++11, real() and imag() are exactly
> *per the letter* of the Standard, that is (similarly for the specializations):
> 
>       constexpr _Tp 
>       real() const { return _M_real; }

Isn't the const redundant here?
Actually, I only see constexpr for the specializations in the standard, not for
the general case, am I looking at the wrong place?

>       constexpr _Tp 
>       imag() const { return _M_imag; }
> 
> and I don't see why we should do something different. But, if Gaby thinks,
> everything considered, that we want something different, I'm not going to
> object to patches to that effect (from a technical point of view, note the
> functions are now constexpr, thus you can't simply have two overloads anymore)

Why can't you still have these overloads?
constexpr _Tp real(); // const
_Tp&real();

Note that I am not taking position on whether it should be added, I am just
confused by the technical reasons.

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-11-08 00:05:20 UTC ---
(In reply to comment #2)
> Isn't the const redundant here?

Maybe, the code predates constexpr.

> Actually, I only see constexpr for the specializations in the standard, not for
>the general case, am I looking at the wrong place?

The constexpr are there, in the primary too, around line # 150.

> Why can't you still have these overloads?
> constexpr _Tp real(); // const
> _Tp&real();
> 
> Note that I am not taking position on whether it should be added, I am just
> confused by the technical reasons.

Sure you can, in principle. The mathematical/physical concept of beauty from
symmetry prevents me for posting such patch, now and ever ;)

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jason at gcc dot gnu.org

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-11-08 00:12:26 UTC ---
I'm sorry, I misunderstood you, you meant C++11 does not mandate the constexpr
in the primary. Actually, I guess it doesn't hurt, Jason added it, let's add
him in CC, to be safe.

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #5 from Jason Merrill <jason at gcc dot gnu.org> 2011-11-08 01:08:16 UTC ---
Hmm, I think adding the overloads would interfere with usage of complex
temporaries in a constant expression.

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #6 from Marc Glisse <marc.glisse at normalesup dot org> 2011-11-08 07:44:27 UTC ---
(In reply to comment #4)
> I'm sorry, I misunderstood you, you meant C++11 does not mandate the constexpr
> in the primary. Actually, I guess it doesn't hurt,

I agree, it was your expression "per the letter" that confused me, but I am
fine with the code.

(In reply to comment #5)
> Hmm, I think adding the overloads would interfere with usage of complex
> temporaries in a constant expression.

Really ? std::bitset seems to have this kind of overload for operator[].
Surprisingly, it is the only type that does, most only have a constexpr
constructor, so I am not sure if there is a bug in bitset, a missing feature
elsewhere, or if the situations are different.

Actually, it seems that with 4.7,libstdc++ provides a constexpr overload for
array::operator[] as an extension, which seems to answer the question.

Or do you mean that the following would cause trouble (which seems likely, but
I don't have any compiler at hand that supports both constexpr and *this
references)?
constexpr _Tp real()const&;
_Tp&real()&;
_Tp real()&&;

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #7 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-11-08 08:11:58 UTC ---
Just a note to the accidental reader: C++11 as published is well known to miss
quite a number of constexpr. Adding those is conforming, a specific DR covers
that. In any case, the primary isn't really the issue here, I suppose, assuming
there is an issue whatsoever (not for me ;)

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[jason at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #8 from Jason Merrill <jason at gcc dot gnu.org> 2011-11-08 15:18:51 UTC ---
I meant that with the current libstdc++ complex, this is valid:

constexpr float f = complex<float>(2.4).real();

but adding a non-constexpr overload would cause that one to be selected for the
call above, making the declaration invalid.

Once we have ref-qualifiers, it should be OK to add the non-const overload with
an lvalue ref-qualifier, though.

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #9 from Marc Glisse <marc.glisse at normalesup dot org> 2011-11-08 15:51:52 UTC ---
(In reply to comment #8)
> I meant that with the current libstdc++ complex, this is valid:
> 
> constexpr float f = complex<float>(2.4).real();
> 
> but adding a non-constexpr overload would cause that one to be selected for the
> call above, making the declaration invalid.

Ah, yes, thank you.

> Once we have ref-qualifiers, it should be OK to add the non-const overload with
> an lvalue ref-qualifier, though.

Assuming we want to provide this extension :-)

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[daniel.kruegler at googlemail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

Daniel Krügler <daniel.kruegler at googlemail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |daniel.kruegler at
                   |                            |googlemail dot com

--- Comment #10 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2011-11-08 18:16:45 UTC ---
(In reply to comment #8)
> Once we have ref-qualifiers, it should be OK to add the non-const overload with
> an lvalue ref-qualifier, though.

I think you need to add a differently named function, because a function with
ref-qualifier cannot coexist with other functions of the same name and the same
parameter-type-list that do not have a ref-qualifier.

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[marc.glisse at normalesup dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #11 from Marc Glisse <marc.glisse at normalesup dot org> 2011-11-08 18:40:13 UTC ---
(In reply to comment #10)
> (In reply to comment #8)
> > Once we have ref-qualifiers, it should be OK to add the non-const overload with
> > an lvalue ref-qualifier, though.
> 
> I think you need to add a differently named function, because a function with
> ref-qualifier cannot coexist with other functions of the same name and the same
> parameter-type-list that do not have a ref-qualifier.

Can't you qualify the constexpr version with const&? That wouldn't exactly
match the signature in the standard, which is bad if someone tries to store
&complex::real in a pointer-to-member variable. But otherwise it should act
about the same as simply const.

I guess that's one more reason not to provide the extension...

(It is funny that for a constexpr bitset a, (~a)[0] is not constexpr :-/ )

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[daniel.kruegler at googlemail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #12 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2011-11-09 10:36:59 UTC ---
(In reply to comment #11)
> Can't you qualify the constexpr version with const&? 

Yes.

> That wouldn't exactly
> match the signature in the standard, which is bad if someone tries to store
> &complex::real in a pointer-to-member variable. But otherwise it should act
> about the same as simply const.

>From the p.o.v of the standard library you don't break anything, because user
code cannot rely on the exact signatures of non-virtual member functions of
library components, 17.6.5.5 [member.functions] gives a lot of freedom to
implementations in this regard.

> (It is funny that for a constexpr bitset a, (~a)[0] is not constexpr :-/ )

I agree.

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[glisse at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #13 from Marc Glisse <glisse at gcc dot gnu.org> ---
(In reply to Jason Merrill from comment #8)
> Once we have ref-qualifiers, it should be OK to add the non-const overload
> with an lvalue ref-qualifier, though.

Now we have ref-qualifiers. Do we want to provide the extension in C++11 mode,
or officially say we won't?

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[ross.martin at ieee dot org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

Ross Martin <ross.martin at ieee dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ross.martin at ieee dot org

--- Comment #14 from Ross Martin <ross.martin at ieee dot org> ---
This change to not be able to pull out a reference to the real or imaginary
parts has messed me up.  The assumption being made by this new complex class is
that the real and imaginary parts are just a float or a double.  That is false
in my code, and doesn't seem to be a good assumption for a general-purpose
complex implementation.  My code has real and imaginary parts that are
themselves full classes.  These classes can, at times, act like
double-precision numbers, so it makes sense to build complex numbers out of
them for arithmetic manipulations.  However, I need direct access to the real
and imaginary parts to be able to access other parts of the class parameters,
and this new complex-class implementation explicitly forbids any direct access
to the real and imaginary parts in a type-safe way that doesn't rely on
knowledge of internal class data organization.  Not a good change, IMHO.

[Bug libstdc++/51013] complex::{imag,real}() should maintain lvalue-returning extension in C++11

[redi at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=51013

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> ---
(In reply to Ross Martin from comment #14)
> This change to not be able to pull out a reference to the real or imaginary
> parts has messed me up.  The assumption being made by this new complex class
> is that the real and imaginary parts are just a float or a double.

No, because if they are float or double then the corresponding explicit
specialization would be used. If the primary template is used then the type
must be some other type.

>  That is
> false in my code, and doesn't seem to be a good assumption for a
> general-purpose complex implementation.

It's what the C++ standard specifies.


>  My code has real and imaginary
> parts that are themselves full classes.  These classes can, at times, act
> like double-precision numbers, so it makes sense to build complex numbers
> out of them for arithmetic manipulations.  However, I need direct access to
> the real and imaginary parts to be able to access other parts of the class
> parameters, and this new complex-class implementation explicitly forbids any
> direct access to the real and imaginary parts in a type-safe way that
> doesn't rely on knowledge of internal class data organization.  Not a good
> change, IMHO.

Did you read http://www.open-std.org/jtc1/sc22/wg21/docs/lwg-defects.html#387 ?

Have you looked at [complex.numbers] in the C++ standard? It says exactly how
to do it:


  If z is an lvalue expression of type cv std::complex<T> then

    the expression reinterpret_cast<cv T(&)[2]>(z) is well-formed; and
    reinterpret_cast<cv T(&)[2]>(z)[0] designates the real part of z; and
    reinterpret_cast<cv T(&)[2]>(z)[1] designates the imaginary part of z.

  Moreover, if a is an expression of pointer type cv complex<T>* and the
  expression a[i] is well-defined for an integer expression i then:

    reinterpret_cast<cv T*>(a)[2*i] designates the real part of a[i]; and
    reinterpret_cast<cv T*>(a)[2*i+1] designates the imaginary part of a[i].


Rather than forbidding direct access to the real and imaginary parts the
standard explicitly requires this to work, so your concern about relying on
internal class data organization is unfounded. You can rely on it because the
standard says so.