Bug 70472 - is_copy_constructible<vector<unique_ptr<int>>>::value is true
Summary: is_copy_constructible<vector<unique_ptr<int>>>::value is true
Status: ASSIGNED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 6.0
: P3 normal
Target Milestone: ---
Assignee: Jonathan Wakely
URL:
Keywords: wrong-code
Depends on:
Blocks:
 
Reported: 2016-03-31 07:06 UTC by Askar Safin
Modified: 2018-08-14 08:48 UTC (History)
4 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2017-11-09 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Askar Safin 2016-03-31 07:06:56 UTC
Consider this code:

#include <type_traits>
#include <vector>
#include <memory>
#include <iostream>

int
main (void)
{
  std::cout << std::is_copy_constructible<std::vector<std::unique_ptr<int>>>::value << "\n";
}

It gives 1. But, of course, it should give 0, because the following code doesn't compile:

#include <vector>
#include <memory>

int
main (void)
{
  typedef std::vector<std::unique_ptr<int>> v;
  v a;
  v b = a;
}

I think this problem is solvable. For example, it is solved for std::exprimental::optional in gcc 6.0.0's stdlibc++. std::is_copy_constructible<std::experimental::optional<std::unique_ptr<int>>>::value is false.

Also, please, make sure that std::is_copy_constructible gives right answers for all standard containers. And same for other type_traits (move_constructible etc).

I don't know is std::is_copy_constructible<std::vector<std::unique_ptr<int>>>::value == false required by the standard. If no, then, please, add this requirement to it.

I noticed this bug when I tried to implement my own class template similar to std::experimental::optional. Compiler give big error message when I combine my class with other standard containers. If I add noexcept to my move constructor, then the error message disappears. So I think this bug somehow related to bug 55043 (bug #55043). If you want, I can describe this my class template in detail. Also, this bug is very similar to this comment: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55043#c15 .

I use gcc-snapshot Debian package. Package version is 20160320-1. Output of "/usr/lib/gcc-snapshot/bin/gcc -v":
COLLECT_GCC=/usr/lib/gcc-snapshot/bin/gcc
COLLECT_LTO_WRAPPER=/usr/lib/gcc-snapshot/libexec/gcc/x86_64-linux-gnu/6.0.0/lto-wrapper
Target: x86_64-linux-gnu
Configured with: ../src/configure -v --with-pkgversion='Debian 20160320-1' --with-bugurl=file:///usr/share/doc/gcc-snapshot/README.Bugs --enable-languages=c,ada,c++,java,go,fortran,objc,obj-c++ --prefix=/usr/lib/gcc-snapshot --enable-shared --enable-linker-build-id --disable-nls --with-sysroot=/ --enable-clocale=gnu --enable-libstdcxx-debug --enable-libstdcxx-time=yes --with-default-libstdcxx-abi=new --enable-gnu-unique-object --disable-vtable-verify --enable-libmpx --enable-plugin --with-system-zlib --disable-browser-plugin --enable-java-awt=gtk --enable-gtk-cairo --with-java-home=/usr/lib/jvm/java-1.5.0-gcj-6-snap-amd64/jre --enable-java-home --with-jvm-root-dir=/usr/lib/jvm/java-1.5.0-gcj-6-snap-amd64 --with-jvm-jar-dir=/usr/lib/jvm-exports/java-1.5.0-gcj-6-snap-amd64 --with-arch-directory=amd64 --with-ecj-jar=/usr/share/java/eclipse-ecj.jar --enable-objc-gc --enable-multiarch --with-arch-32=i586 --with-abi=m64 --with-multilib-list=m32,m64,mx32 --enable-multilib --with-tune=generic --disable-werror --enable-checking=yes --build=x86_64-linux-gnu --host=x86_64-linux-gnu --target=x86_64-linux-gnu
Thread model: posix
gcc version 6.0.0 20160320 (experimental) [trunk revision 234355] (Debian 20160320-1)

I use stdlibc++ shipped with this gcc.
Comment 1 Askar Safin 2016-03-31 07:40:44 UTC
Also, this code doesn't compile: http://paste.debian.net/422907/ and I think this is related to this bug. If I decomment noexcept line, it compiles
Comment 2 Jonathan Wakely 2016-04-18 10:34:36 UTC
(In reply to Askar Safin from comment #0)
> Also, please, make sure that std::is_copy_constructible gives right answers
> for all standard containers. And same for other type_traits
> (move_constructible etc).

That's easy to say but much harder to do.

> I don't know is
> std::is_copy_constructible<std::vector<std::unique_ptr<int>>>::value ==
> false required by the standard. If no, then, please, add this requirement to
> it.

It's not required, and it would be impossible to require it in general. The problem is that std::vector does have a copy constructor, so the trait value is true, but instantiating that constructor produces an error when the value_type is not copyable. The trait is not required to instantiate the constructor (and doing so would cause other problems).

> I noticed this bug when I tried to implement my own class template similar
> to std::experimental::optional. Compiler give big error message when I
> combine my class with other standard containers. If I add noexcept to my
> move constructor, then the error message disappears.

Types used with std::vector must be nothrow-move-constructible or copy-constructible. If your type has a throwing move-ctor then vector will try to copy it, which causes an instantiation error.


> So I think this bug
> somehow related to bug 55043 (bug #55043). If you want, I can describe this
> my class template in detail. Also, this bug is very similar to this comment:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=55043#c15 .

It's the same issue as PR 55043, but harder to fix for std::vector because the relevant constructors are not defined as defaulted.

> I use stdlibc++ shipped with this gcc.

It's libstdc++ not stdlibc++
Comment 3 Askar Safin 2016-04-18 13:10:59 UTC
(In reply to Jonathan Wakely from comment #2)
> It's not required, and it would be impossible to require it in general. The
> problem is that std::vector does have a copy constructor, so the trait value
> is true, but instantiating that constructor produces an error when the
> value_type is not copyable.
std::is_copy_constructible<std::experimental::optional<...>> is correct. Look at the implementation. There are _Enable_special_members and _Enable_copy_move in <bits/enable_special_members.h>. Then there is _Optional_base in <experimental/optional>. And then there is

template<typename _Tp>
    class optional
    : private _Optional_base<_Tp>,
      private _Enable_copy_move<
        // Copy constructor.
        is_copy_constructible<_Tp>::value,
        ...
      >

in <experimental/optional>. Also, link I post ( http://paste.debian.net/422907/ ) give simplified example how to do this.

> Types used with std::vector must be nothrow-move-constructible or
> copy-constructible. If your type has a throwing move-ctor then vector will
> try to copy it, which causes an instantiation error.
If I remove copy-ctor and copy-assignment using "= delete" in that paste.debian.org link and leave noexcept commented, then all will build successfully. So, it seems for me vector uses the following algorithm:

if (has_noexcept_move_ctor)
  {
    use_noexcept_move_ctor ();
  }
else if (has_copy_ctor) // determined using trait. So if copy ctor is defined, it will be used, even if it doesn't compile. If copy ctor is deleted using "= delete", it will not be used
  {
    use_copy_ctor ();
  }
else
  {
    use_move_ctor ();
  }
Comment 4 Jonathan Wakely 2016-04-18 13:30:20 UTC
Yes, I know how to do it, that doesn't mean we can do so easily in existing types.
Comment 5 TC 2016-04-18 20:38:56 UTC
In any event, it would be wrong to SFINAE on std::is_copy_constructible<value_type>. The requirement is CopyInsertable, not CopyConstructible. The allocator's construct() can mutilate the constructor arguments to its heart's content before passing them on, and I don't see a way to check this.
Comment 6 Jonathan Wakely 2016-04-19 10:34:09 UTC
(In reply to TC from comment #5)
> In any event, it would be wrong to SFINAE on
> std::is_copy_constructible<value_type>. The requirement is CopyInsertable,
> not CopyConstructible. The allocator's construct() can mutilate the
> constructor arguments to its heart's content before passing them on, and I
> don't see a way to check this.

<bits/alloc_traits.h> has this:

  // true if _Alloc::value_type is CopyInsertable into containers using _Alloc
  template<typename _Alloc>
    struct __is_copy_insertable
    : __is_copy_insertable_impl<_Alloc>::type
    { };

But using it requires that std::vector::~vector() is defined as defaulted, which would not be a simple change.

We used to use that for the unordered containers until r204790.
Comment 7 TC 2016-04-19 11:26:42 UTC
(In reply to Jonathan Wakely from comment #6)
> (In reply to TC from comment #5)
> > In any event, it would be wrong to SFINAE on
> > std::is_copy_constructible<value_type>. The requirement is CopyInsertable,
> > not CopyConstructible. The allocator's construct() can mutilate the
> > constructor arguments to its heart's content before passing them on, and I
> > don't see a way to check this.
> 
> <bits/alloc_traits.h> has this:
> 
>   // true if _Alloc::value_type is CopyInsertable into containers using
> _Alloc
>   template<typename _Alloc>
>     struct __is_copy_insertable
>     : __is_copy_insertable_impl<_Alloc>::type
>     { };
> 
> But using it requires that std::vector::~vector() is defined as defaulted,
> which would not be a simple change.
> 
> We used to use that for the unordered containers until r204790.

That also requires the allocator's `construct` be SFINAE-friendly. Most aren't.
Comment 8 Askar Safin 2017-06-26 15:11:01 UTC
Recently I noticed this bug can be easily fixed simply by manually implementing is_copy_constructible. So, please, apply the fix. And same for other type traits (is_copy_assignable etc).

#include <vector>
#include <type_traits>
#include <iostream>
#include <memory>

namespace std{
template <typename _Tp> struct is_copy_constructible<std::vector<_Tp>> : public is_copy_constructible<_Tp>
{
};
}

struct foo
{
  std::vector<foo> x;             
};                                

struct bar
{
  std::vector<bar> x;
  std::unique_ptr<int> y;
};

// First column is what we want, second is actual result
int
main ()
{
  std::cout << "1 " << std::is_copy_constructible<std::vector<int>>::value << "\n";
  std::cout << "0 " << std::is_copy_constructible<std::vector<std::unique_ptr<int>>>::value << "\n";
  std::cout << "1 " << std::is_copy_constructible<foo>::value << "\n";
  std::cout << "0 " << std::is_copy_constructible<bar>::value << "\n";
}
Comment 9 Jonathan Wakely 2017-06-26 15:37:24 UTC
That would only work for vector<T> not vector <T, A>, for the reasons given above.
Comment 10 Jonathan Wakely 2017-11-09 00:48:03 UTC
I have a fix for this now.
Comment 11 Jakub Jelinek 2018-05-02 10:06:14 UTC
GCC 8.1 has been released.
Comment 12 Jakub Jelinek 2018-07-26 11:19:53 UTC
GCC 8.2 has been released.
Comment 13 Simon Brand 2018-08-14 08:48:34 UTC
Is there any decent workaround for this for pre-GCC8 users? My current approach is to add specializations to my own trait whenever a user of my library needs support for types with this issue. I can happily use intrinsics if some would make this easier.