On 10/11/18 8:07 PM, David Malcolm wrote:
On Thu, 2018-10-11 at 10:31 -0400, Jason Merrill wrote:
On Thu, Oct 11, 2018 at 10:28 AM Jason Merrill <ja...@redhat.com>
wrote:
On Wed, Oct 10, 2018 at 5:01 PM David Malcolm <dmalc...@redhat.com>
wrote:
On Tue, 2018-10-09 at 18:38 -0400, Jason Merrill wrote:
On Tue, Oct 9, 2018 at 1:19 PM David Malcolm <dmalcolm@redhat.c
om>
wrote:
+ /* Emulation of a "move" constructor, but really a copy
+ constructor. */
+
+ name_hint (const name_hint &other)
+ : m_suggestion (other.m_suggestion),
+ m_deferred (const_cast<name_hint &>
(other).take_deferred ())
+ {
+ }
+
+ /* Emulation of "move" assigment, but really copy
assignment. */
+
+ name_hint& operator= (const name_hint &other)
+ {
+ m_suggestion = other.m_suggestion;
+ m_deferred = const_cast<name_hint &>
(other).take_deferred ();
+ return *this;
+ }
+
+ /* Take ownership of this name_hint's deferred_diagnostic,
for
use
+ in chaining up deferred diagnostics. */
+ gnu::unique_ptr<deferred_diagnostic> take_deferred () {
return
move (m_deferred); }
Why do you want to propagate this hackery into name_hint? I
would
expect the defaulted special member functions to do the right
thing
with m_deferred: in -std=c++98 the implicit copy ops call the
gnu::unique_ptr copy ops that actually move, and in -std=c++11
and up
we're calling the move constructor for std::unique_ptr, which
does
the
right thing.
This also doesn't limit the hack to C++98 mode the way unique-
ptr.h
does.
Jason
Thanks for looking at this.
I ran into issues trying to pass around name_hint instances:
../../src/gcc/cp/name-lookup.c: In function 'name_hint
suggest_alternatives_in_other_namespaces(location_t, tree)':
../../src/gcc/cp/name-lookup.c:5591:52: error: use of deleted
function 'name_hint::name_hint(const name_hint&)'
5591 | return ns_hints.maybe_decorate_with_limit (result);
| ^
In file included from ../../src/gcc/cp/name-lookup.c:36:
../../src/gcc/c-family/name-hint.h:91:7: note:
'name_hint::name_hint(const name_hint&)' is implicitly deleted
because the default definition would be ill-formed:
91 | class name_hint
| ^~~~~~~~~
../../src/gcc/c-family/name-hint.h:91:7: error: use of deleted
function 'std::unique_ptr<_Tp, _Dp>::unique_ptr(const
std::unique_ptr<_Tp, _Dp>&) [with _Tp = deferred_diagnostic; _Dp
= std::default_delete<deferred_diagnostic>]'
In file included from /home/david/coding/gcc-python/gcc-svn-
trunk/install-dogfood/include/c++/9.0.0/memory:80,
from ../../src/gcc/../include/unique-ptr.h:78,
from ../../src/gcc/system.h:730,
from ../../src/gcc/cp/name-lookup.c:23:
/home/david/coding/gcc-python/gcc-svn-trunk/install-
dogfood/include/c++/9.0.0/bits/unique_ptr.h:394:7: note: declared
here
394 | unique_ptr(const unique_ptr&) = delete;
| ^~~~~~~~~~
../../src/gcc/cp/name-lookup.c:5512:1: note: initializing
argument 1 of 'name_hint
namespace_hints::maybe_decorate_with_limit(name_hint)'
5512 | namespace_hints::maybe_decorate_with_limit (name_hint
hint)
| ^~~~~~~~~~~~~~~
I can't use the default copy constructor or assignment operators
for an
object containing a gnu::unique_ptr on C++11, as std::unique_ptr
has:
// Disable copy from lvalue.
unique_ptr(const unique_ptr&) = delete;
unique_ptr& operator=(const unique_ptr&) = delete;
If I understand things right, in C++11 I should be using move
construction/move assignment for this.
I can't write "&&" in the function params to explicitly request
an
rvalue-reference, as the code need to be compatible with C++98.
std::move is only defined in C++11 onwards.
Our include/unique-ptr.h defines a gnu::move: for C++11 it's
std::move,
but for C++98 it's only defined for the unique_ptr template.
A solution that seems to work appears to be to define gnu::move
for
C++98 for all types rather than just gnu::unique_ptr,
implementing it
in terms of copying an object via lvalue reference, so that we
can
explicitly request a move using "gnu::move" (==std::move on C++),
without using C++11 syntax, and falling back to a copy on C++98
(which effectively moves the ptr from the "victim").
Does that sound sane?
I wouldn't change the unique-ptr.h move to take all types, given
that
it copies rather than just passing the reference through, which
could
be expensive for unsuspecting users. And given how it subverts the
C++98 type system, I'd rather explicitly opt into it. So, let's
overload it for name_hint. And I'd probably return a reference,
e.g.
#if __cplusplus < 201103
// std::move emulation to support the use of gnu::unique_ptr in
name_hint.
namespace gnu {
inline const name_hint &
move(name_hint &m) { return m; }
}
#endif
to avoid the unnecessary copy. Actually, I'd be inclined to do
that
for gnu::unique_ptr as well, but would want to make sure that it
doesn't break gdb.
And if we made that change to the unique-ptr.h version, i.e.
template<typename T>
const T&
move (T& v)
{
return v;
}
then adding an overload for name_hint wouldn't be necessary.
Jason
Thanks! Here's an updated version which uses the above.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu
As before I also tested non-bootstrap builds with both gcc 4.8 and with
trunk (to build without and with C++11), running various testcases
under valgrind.
OK for trunk?
OK.
Jason
