On 17/09/18 16:41 +0100, Jonathan Wakely wrote:
On 17/09/18 16:41 +0200, Stephan Bergmann wrote:
Compiling LibreOffice with recent GCC trunk, I came across an error
like with this reproducer
$ cat test14.cc
#include <memory>
struct S1;
struct S2;
struct D { void operator ()(S1 *); };
class C {
std::unique_ptr<S2, D> m;
C();
C(C const &);
~C();
C & operator =(C const &);
};
$ gcc/inst/bin/g++ -c test14.cc
In file included from /home/sbergman/gcc/inst/include/c++/9.0.0/memory:80,
from test14.cc:1:
/home/sbergman/gcc/inst/include/c++/9.0.0/bits/unique_ptr.h: In instantiation of
‘class std::__uniq_ptr_impl<S2, D>’:
/home/sbergman/gcc/inst/include/c++/9.0.0/bits/unique_ptr.h:172:33: required from
‘class std::unique_ptr<S2, D>’
test14.cc:6:28: required from here
/home/sbergman/gcc/inst/include/c++/9.0.0/bits/unique_ptr.h:145:22: error:
static assertion failed: unique_ptr's deleter must be invocable with a pointer
145 | static_assert( __is_invocable<_Dp&, pointer&>::value,
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
where S2 is an incomplete type, so the static_assert in unique_ptr
(added recently with <https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=199f729ee3a6bd564151cda4caac289fbe7105cc>
"Implement LWG 2905 changes to constrain unique_ptr constructors")
can't determine that D can be invoked with an S2* (which it can if
S2 is derived from S1).
Oh dear, this is another example of the problem described in
https://cplusplus.github.io/LWG/lwg-active.html#3099
I think strictly speaking, your code is undefined according to
[unique.ptr.single] p1. That requires the is_invocable condition to be
true. I think that should be considered a defect in the standard, so
I've requested a new issue to fix that.
I'm not sure whether this is a bug in LibreOffice (which would need
to make sure S2 is a complete type when defining C),
Or you can define D::pointer as S1* and then the assertion passes.
struct D {
using pointer = S1*;
void operator ()(S1 *);
};
or a too aggressive static_assert in libstdc++?
Yes, I'll have to remove it.
I've committed the attached patch (and requested a new issue to fix
the standard). Thanks for bringing this to our attention.
commit d9451e907ff44cd4203da7dda76d41d01577d00b
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Tue Sep 18 15:25:51 2018 +0100
Fix location of invocable check for unique_ptr deleter
The deleter only needs to be invocable when the unique_ptr destructor
and reset member function are instantiated. In other contexts it might
not be possible to pass unique_ptr<T, D>::pointer to the deleter, if
that requires a derived-to-base conversion from T* and T is incomplete.
* include/bits/unique_ptr.h (__uniq_ptr_impl): Remove static assertion
checking invocable condition.
(unique_ptr::~unique_ptr, unique_ptr::reset): Restore static assertion
here, where types must be complete. Pass pointer to deleter as an
rvalue.
* testsuite/20_util/unique_ptr/requirements/incomplete.cc: New test.
diff --git a/libstdc++-v3/include/bits/unique_ptr.h b/libstdc++-v3/include/bits/unique_ptr.h
index ddc6ae0e233..0717c1e2728 100644
--- a/libstdc++-v3/include/bits/unique_ptr.h
+++ b/libstdc++-v3/include/bits/unique_ptr.h
@@ -142,8 +142,6 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
static_assert( !is_rvalue_reference<_Dp>::value,
"unique_ptr's deleter type must be a function object type"
" or an lvalue reference type" );
- static_assert( __is_invocable<_Dp&, pointer&>::value,
- "unique_ptr's deleter must be invocable with a pointer" );
__uniq_ptr_impl() = default;
__uniq_ptr_impl(pointer __p) : _M_t() { _M_ptr() = __p; }
@@ -282,9 +280,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
/// Destructor, invokes the deleter if the stored pointer is not null.
~unique_ptr() noexcept
{
+ static_assert(__is_invocable<deleter_type&, pointer>::value,
+ "unique_ptr's deleter must be invocable with a pointer");
auto& __ptr = _M_t._M_ptr();
if (__ptr != nullptr)
- get_deleter()(__ptr);
+ get_deleter()(std::move(__ptr));
__ptr = pointer();
}
@@ -389,10 +389,12 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
void
reset(pointer __p = pointer()) noexcept
{
+ static_assert(__is_invocable<deleter_type&, pointer>::value,
+ "unique_ptr's deleter must be invocable with a pointer");
using std::swap;
swap(_M_t._M_ptr(), __p);
if (__p != pointer())
- get_deleter()(__p);
+ get_deleter()(std::move(__p));
}
/// Exchange the pointer and deleter with another object.
diff --git a/libstdc++-v3/testsuite/20_util/unique_ptr/requirements/incomplete.cc b/libstdc++-v3/testsuite/20_util/unique_ptr/requirements/incomplete.cc
new file mode 100644
index 00000000000..96e02e4c903
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/unique_ptr/requirements/incomplete.cc
@@ -0,0 +1,33 @@
+// Copyright (C) 2018 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3. If not see
+// <http://www.gnu.org/licenses/>.
+
+// { dg-do compile { target c++11 } }
+
+#include <memory>
+
+struct Base; // incomplete
+
+struct BaseDeleter {
+ void operator()(Base*) const;
+};
+
+struct Derived; // incomplete
+
+struct X {
+ std::unique_ptr<Derived, BaseDeleter> p;
+ ~X(); // defined elsewhere, where Derived is complete
+};
