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 <dmalc...@redhat.com>
> > 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.
Jason
