Message ID | 509F0D27.8080408@redhat.com |
---|---|
State | New |
Headers | show |
On Sat, 10 Nov 2012, Jason Merrill wrote: > As mentioned in http://gcc.gnu.org/wiki/Cxx11AbiCompatibility, C++11 changes > the return type of complex::real and imag, leading to a binary > incompatibility between C++98 and C++11 code if the functions are used > without inlining. This patch adds an ABI tag to the C++11 variants so that > they have different mangled names. This does not change the exports from > libstdc++. It looks like it won't prevent from using ref-qualifiers when they are implemented if we want to (PR 51013), which is good. > For the map void->iterator change, I think it would make sense to just > unconditionally return an iterator; It does break C++98 code that does "return f();" in a function returning void, but that's negligible. > there's no binary compatibility issue > with older code that expects it to return void, the problem is only with > C++11 code calling a C++98 instantiation that returns void. That's going to happen in practice, isn't it? So do you mean to add the abi tag to this unified function, for safety?
On 11 November 2012 02:27, Jason Merrill wrote: > > Is this patch OK for trunk? Does someone on the library team want to look > at the other cases? The patch looks good to me. Thanks for implementing the attribute. I'll start looking at some of the other cases. For std::list I assume we want the attribute on the type itself, not just member functions, since its size changes. For vector::data() we should be able to make the attribute conditional on is_same<_Tp*,allocator_type::pointer> so it doesn't change mangling unless necessary. As well as the existing incompatibilities, I need to add a new virtual function to support returning future_status::deferred from std::future::wait_{for,until}. Is it appropriate to use the same "cxx11" tag for that too, even though the existing implementation is also "cxx11" because the type wasn't in C++98? Or do we want to use a different tag, such as "cxx11v2" or just "v2"?
Hi, On 11/11/2012 12:55 PM, Jonathan Wakely wrote: > On 11 November 2012 02:27, Jason Merrill wrote: >> Is this patch OK for trunk? Does someone on the library team want to look >> at the other cases? > The patch looks good to me. Thanks for implementing the attribute. > > I'll start looking at some of the other cases. > > For std::list I assume we want the attribute on the type itself, not > just member functions, since its size changes. If it's that easy, I can do this bit, ie, simply restore my old patch, which was already complete and tested. Of course thanks a lot from me too! Paolo.
On 11/11/2012 06:55 AM, Jonathan Wakely wrote: > For std::list I assume we want the attribute on the type itself, not > just member functions, since its size changes. Yes. Note that I believe for the list and string changes we want to change the default ABI for C++98 and C++11, not just C++11. And provide a way for users to select the old ABI if they need to pass string/list objects to and from previously compiled code. > As well as the existing incompatibilities, I need to add a new virtual > function to support returning future_status::deferred from > std::future::wait_{for,until}. Is it appropriate to use the same > "cxx11" tag for that too, even though the existing implementation is > also "cxx11" because the type wasn't in C++98? Or do we want to use > a different tag, such as "cxx11v2" or just "v2"? If this is a C++11-only class, then I don't think we need a tag at all. I don't think we've been trying to maintain binary compatibility between major releases for C++11 code. The goal of all of this is to allow old C++98 code to coexist in the same running program with new C++11 code so long as they aren't trying to use the same string/list objects. Jason
On Sun, 11 Nov 2012, Jason Merrill wrote: > On 11/11/2012 06:55 AM, Jonathan Wakely wrote: >> For std::list I assume we want the attribute on the type itself, not >> just member functions, since its size changes. When we are changing the whole type, I assume the point of using this attribute instead of the standard solutions (move it to some other (inline) namespace, for instance) is the -Wabi-tag warning? > Yes. Note that I believe for the list and string changes we want to change > the default ABI for C++98 and C++11, not just C++11. And provide a way for > users to select the old ABI if they need to pass string/list objects to and > from previously compiled code. That way is already needed while building libstdc++, and we need to compile all files that use string twice during the build, exporting only the symbols that contain "cxx11" from one of the builds. Changing string/list is still an ABI break. Is it for 4.8, or should it wait until 4.9?
On 11/12/2012 01:50 AM, Marc Glisse wrote: > When we are changing the whole type, I assume the point of using this > attribute instead of the standard solutions (move it to some other > (inline) namespace, for instance) is the -Wabi-tag warning? Right. Jason
commit be79353c10252bc99cac5f9d9ce045207e665238 Author: Jason Merrill <jason@redhat.com> Date: Sat Nov 10 13:10:15 2012 -0500 * include/std/complex (real, imag): Add ABI tag in C++11 mode. diff --git a/libstdc++-v3/include/std/complex b/libstdc++-v3/include/std/complex index f9221a8..24fa414 100644 --- a/libstdc++-v3/include/std/complex +++ b/libstdc++-v3/include/std/complex @@ -141,9 +141,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cplusplus >= 201103L // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 387. std::complex over-encapsulated. + __attribute ((abi_tag ("cxx11"))) constexpr _Tp real() { return _M_real; } + __attribute ((abi_tag ("cxx11"))) constexpr _Tp imag() { return _M_imag; } #else @@ -1061,9 +1063,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cplusplus >= 201103L // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 387. std::complex over-encapsulated. + __attribute ((abi_tag ("cxx11"))) constexpr float real() { return __real__ _M_value; } + __attribute ((abi_tag ("cxx11"))) constexpr float imag() { return __imag__ _M_value; } #else @@ -1210,9 +1214,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cplusplus >= 201103L // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 387. std::complex over-encapsulated. + __attribute ((abi_tag ("cxx11"))) constexpr double real() { return __real__ _M_value; } + __attribute ((abi_tag ("cxx11"))) constexpr double imag() { return __imag__ _M_value; } #else @@ -1360,9 +1366,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION #if __cplusplus >= 201103L // _GLIBCXX_RESOLVE_LIB_DEFECTS // DR 387. std::complex over-encapsulated. + __attribute ((abi_tag ("cxx11"))) constexpr long double real() { return __real__ _M_value; } + __attribute ((abi_tag ("cxx11"))) constexpr long double imag() { return __imag__ _M_value; } #else diff --git a/libstdc++-v3/testsuite/26_numerics/complex/abi_tag.cc b/libstdc++-v3/testsuite/26_numerics/complex/abi_tag.cc new file mode 100644 index 0000000..a845466 --- /dev/null +++ b/libstdc++-v3/testsuite/26_numerics/complex/abi_tag.cc @@ -0,0 +1,25 @@ +// Test that the C++11 variants of real/imag have an ABI tag +// { dg-do compile } +// { dg-options -std=c++11 } + +#include <complex> + +// { dg-final { scan-assembler "_ZNKSt7complexIfE4realB5cxx11Ev" } } +float (std::complex<float>::*p1)() const = &std::complex<float>::real; +// { dg-final { scan-assembler "_ZNKSt7complexIdE4realB5cxx11Ev" } } +double (std::complex<double>::*p2)() const = &std::complex<double>::real; +// { dg-final { scan-assembler "_ZNKSt7complexIeE4realB5cxx11Ev" } } +long double (std::complex<long double>::*p3)() const + = &std::complex<long double>::real; +// { dg-final { scan-assembler "_ZNKSt7complexIiE4realB5cxx11Ev" } } +int (std::complex<int>::*p4)() const = &std::complex<int>::real; + +// { dg-final { scan-assembler "_ZNKSt7complexIfE4imagB5cxx11Ev" } } +float (std::complex<float>::*p5)() const = &std::complex<float>::imag; +// { dg-final { scan-assembler "_ZNKSt7complexIdE4imagB5cxx11Ev" } } +double (std::complex<double>::*p6)() const = &std::complex<double>::imag; +// { dg-final { scan-assembler "_ZNKSt7complexIeE4imagB5cxx11Ev" } } +long double (std::complex<long double>::*p7)() const + = &std::complex<long double>::imag; +// { dg-final { scan-assembler "_ZNKSt7complexIiE4imagB5cxx11Ev" } } +int (std::complex<int>::*p8)() const = &std::complex<int>::imag;