gmane.comp.gcc.libstdc++.devel

Re: iter_swap on move_iterator

Christopher Jefferson — 2012-05-19T08:01:39

Furthermore, we used to use move_iterator internally in std::sort, and 
had to take it out because it would sometimes lead to values being lost, 
if the type had a 'by value' operator< like:

bool operator<(MyType t1, MyType t2) { ... }

Because as soon as we compared we would move the value into the 
comparitor and it would be lost.

I would personally advise against supporting move_iterator in the 
standard algorithms, unless you are also prepared to make it safe in 
such situations (which might itself be impossible, without breaking 
other things).

In short, move_iterator is in my opinion easily misused and not a proper 
iterator and not compiling it might be a 'feature' :)

Chris

Re: iter_swap on move_iterator

Jonathan Wakely — 2012-05-19T00:07:35

I agree, and will just add that support for swapping rvalues of
standard library classes was added to a C++0x draft but then
intentionally removed again as undesirable and IIRC in some cases
unsafe.

Re: iter_swap on move_iterator

Daniel Krügler — 2012-05-18T18:12:47

2012/5/18 François Dumont <frs.dumont< at >gmail.com>:

I don't think so. The library requirements on the iterator type is
ValueSwappable, which
means that "for any dereferenceable object x of type X, *x is
swappable". So, unless
there exists active rvalue-swap support provided by user-provided value type of
the wrapped iterator of std::move_iterator (or by the reference type
of the wrapped
iterator, once LWG 2106 becomes accepted), the library implementation should
not interfere here.

- Daniel

Re: iter_swap on move_iterator

François Dumont — 2012-05-18T12:21:45

Ok for how to have iter_swap work with move_iterator, users must grant 
the necessary swap overload. But what about the other algos like 
stable_sort I have tested at first ? Shouldn't they be adapted to work 
with move_iterator ?
Agree but the compiler message do not show the '&&', this is especially 
obvious in:

/home/fdt/dev/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/move.h:166:5: 
note:   no known conversion for argument 1 from 
'std::move_iterator<greedy_ops::X*>::value_type {aka greedy_ops::X}' to 
'greedy_ops::X&'

The compiler is perfectly able to take a lvalue reference to a 
value_type so this message makes no sens. If it was saying 
"...::reference {aka greedy_ops::X&&}' to ..." it would be clearer.

François

Re: iter_swap on move_iterator

Daniel Krügler — 2012-05-17T18:47:00

2012/5/17 François Dumont <frs.dumont< at >gmail.com>:

Yes, when the referenced type (or the iterator's reference) provides a
swap overload
that accepts rvalues. Remember that move_iterator will produce an rvalue
on dereference, this rvalue is provided to the swap overload that is found
by ADL. Given your description I would expect that the test type does not
provide a swap overload accepting rvalues, thus the std::swap is selected,
but cannot accept the arguments.


Yes, but move_iterator<>::reference is value_type&&.

- Daniel

iter_swap on move_iterator

François Dumont — 2012-05-17T17:53:55

Hi

     While working on removal of code duplications in algos 
implementation I challenge them with move_iterator. I started with 
stable_sort and it doesn't compile. I summarize the problem to the fact 
that iter_swap can't be used on move_iterator. As, in C++11, iter_swap 
simply forward to swap(*__a, *__b) it is normal that it can't accept 
rvalue but shouldn't iter_swap be adapted to accept move_iterator ?

     I also find the compilation error message surprising. I have added 
a call to iter_swap in 24_iterators/move_iterator/greedy_ops.cc and the 
error message is:

In file included from 
/home/fdt/dev/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/char_traits.h:41:0, 

                  from 
/home/fdt/dev/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/include/ios:41, 

                  from 
/home/fdt/dev/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/include/ostream:40, 

                  from 
/home/fdt/dev/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/include/iterator:64, 

                  from 
/home/fdt/dev/gcc/libstdc++-v3/testsuite/24_iterators/move_iterator/greedy_ops.cc:20: 

/home/fdt/dev/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h: 
In instantiation of 'void std::iter_swap(_ForwardIterator1, 
_ForwardIterator2) [with _ForwardIterator1 = 
std::move_iterator<greedy_ops::X*>; _ForwardIterator2 = 
std::move_iterator<greedy_ops::X*>]':
/home/fdt/dev/gcc/libstdc++-v3/testsuite/24_iterators/move_iterator/greedy_ops.cc:39:24:   
required from here
/home/fdt/dev/gcc-build/x86_64-unknown-linux-gnu/libstdc++-v3/include/bits/stl_algobase.h:148:7: 
error: no matching function for call to 
'swap(std::move_iterator<greedy_ops::X*>::value_type, 
std::move_iterator<greedy_ops::X*>::value_type)'
        swap(*__a, *__b);
        ^

Why is the comipiler talking about a call to swap on 
move_iterator<greedy_ops::X*>::value_type. It should be on the return 
type of move_iterator operator* which is move_iterator<>::reference, no ?

François

Re: [v3] fix libstdc++/53263

Paolo Carlini — 2012-05-15T20:41:26

Thanks!

Paolo.

Re: [v3] fix libstdc++/53263

François Dumont — 2012-05-15T20:30:56

Details added

2012-05-11  François Dumont <fdumont< at >gcc.gnu.org>

     PR libstdc++/53263
     * include/debug/safe_iterator.h (__gnu_debug::__base): Move...
     * include/debug/functions.h: ... Here.
     (__check_sorted_aux): Add overload for random access safe
     iterators.
     (__check_partition_lower_aux): New.
     (__check_partition_lower): Use latter.
     (__check_partition_upper_aux): New.
     (__check_partition_upper): Use latter.
     * include/debug/macros.h (__glibcxx_check_heap)
     (__glibcxx_check_heap_pred): Use __gnu_debug::__base on iterator range.

François

On 05/14/2012 09:55 PM, Paolo Carlini wrote:

Re: Remove algo code duplication

Marc Glisse — 2012-05-14T20:02:47


Or instead of using directly a functor on the values, one could write the 
algorithm using a functor on iterators, which would take a and b and 
return either *a<*b or cmp(*a,*b). Maybe not so nice.

Re: [v3] fix libstdc++/53263

Paolo Carlini — 2012-05-14T19:55:16

What does it mean "Add debug function overloads"? Which are the *names* 
of those overloads? We want to know that, because we want to be able to 
grep the ChangeLog and see exactly when somebody touched each and every 
functions.

Paolo.

Re: [v3] fix libstdc++/53263

François Dumont — 2012-05-14T19:53:03

I don't think I miss any file, tell me otherwise.

My bugzilla account is fully functional now.

Thanks

François

Re: [v3] fix libstdc++/53263

Paolo Carlini — 2012-05-12T21:44:35

Weird. Thanks a lot Ian. Francois, please double check that now you are Ok.

Thanks,
Paolo.

Re: [v3] fix libstdc++/53263

Ian Lance Taylor — 2012-05-12T05:59:04

[ Adding overseers to make a record of this ]


Jonathan Wakely <jwakely.gcc< at >gmail.com> writes:


Anybody < at >gcc.gnu.org is supposed to get various permissions in bugzilla.
I have no idea why these were not turned on for fdumont< at >gcc.gnu.org.  I
just turned them on.

Ian

Re: [v3] fix libstdc++/53263

Jonathan Wakely — 2012-05-11T23:28:13

Oops, Paolo pointed out I didn't actually CC Ian... done this time!
Ian, see above, we'd be grateful for your assistance.

Re: [v3] fix libstdc++/53263

Jonathan Wakely — 2012-05-11T23:08:06

My understanding is that any < at >gcc.gnu.org account should have full
permissions to update and close any bugs.

I'v CC'd Ian as I think he can view and edit any user's bugzilla
permissions and might know why fdumont< at >gcc doesn't have the full set
of permissions that a gcc.gnu.org account usually has, or at least
could set them.

Re: [v3] fix libstdc++/53263

Paolo Carlini — 2012-05-11T22:46:01

Note, as a rule the ChangeLog should explicitly mention all the files 
you are touching. If you can, please amend it. Thanks!
Didn't Jon provide some guidance about this? Jon?

Thanks,
Paolo.

Re: [v3] fix libstdc++/53263

Paolo Carlini — 2012-05-10T21:18:51

Hi,

On 05/09/2012 11:02 PM, François Dumont wrote:

Thanks. Considering that this isn't a regression and also that nobody 
reported the issue for so many years, the patch seems a bit largish to 
me to go into the branch. Thus, let's apply to mainline only and 
consider the issue closed. If people insist, seriously insist ;) we may 
reconsider for 4.7.2.

Thanks again!
Paolo.

PS: are you finally able to manage Bugzilla, yes?

Re: h8300-elf build broken

DJ Delorie — 2012-05-10T21:07:10


Done!

Re: [Mingw-w64-public] Thoughts on supporting the C++11 thread library on Windows

Jonathan Wakely — 2012-05-10T14:52:26

Yep, if you don't explicitly configure with --enable-threads then the
configure script picks a suitable default, which is "win32" on
Windows.



Yes, I don't think we have an open bugzilla report about that
portability problem, but it's a known issue.


Have those changes come back upstream? It sounds as though they should do.



That makes sense for <thread> because the standard is the
authoritative source.  For GCC's configuration the GCC sources are the
authoritative source :-)

Sorry if my last mail seemed impatient, I do appreciate your feedback
based on your own experience of implementing <thread>.

Re: [Mingw-w64-public] Thoughts on supporting the C++11 thread library on Windows

Gabriel Dos Reis — 2012-05-10T00:22:26

For the record, I agree with your proposal.
The intent of my forwarding your original message to mingw-64 was to
draw their attention since they have been doing work in this area for
some times and the likely compiler port maintainers are there.
I do hope your patience will pay off :-)

Re: [Mingw-w64-public] Thoughts on supporting the C++11 thread library on Windows

Jonathan Wakely — 2012-05-09T23:06:17

If you use GCC built with --enable-threads=posix then you shouldn't
need to implement <thread>, it should be provided.

In any case, as you're not using the win32 thread model you shouldn't
are about my proposals and you're just derailing the topic ;-)



The code is all open source, feel free to read it.

--enable-threads=win32 tells GCC to use the gthr-win32.h header to
implement the gthreads abstraction API.  Gthreads provides a
pthreads-like API which libstdc++ (and libobjc and other bits of GCC)
use for threading facilities.  WIth --enable-threads=posix GCC uses
the gthr-posix.h file which provides an implementation of gthreads
based on pthreads (which is a one-to-one mapping.) With
--enable-threads=win32 GCC uses the gthr-win32.h file which provides
an implementation based on Windows primitives, which as I said several
days ago means that the __gthread_mutex_t type is implemented as
semaphores.

libstdc++ uses the __gthread_mutex_t type internally when it needs a
mutex, and when gthr-posix.h is used also uses __gthread_mutex_t to
implement std::mutex.

My original email, which I now seem to be repeating over and over,
suggests changes to gthr-win32.h to allow std::mutex and C++11 other
types to be defined in terms of the Windows primitives currently used
in gthr-win32.h

That gthr-win32.h header already exists, and already defines
__gthread_mutex_t in terms of a semaphore, and it's too late to change
that.  So maybe if you re-read my original mail now it will make more
sense.  I proposed extending gthr-win32.h to allow std::thread and
std::mutex to be provided WITHOUT CHANGING THE EXISTING IMPLEMENTATION
OF gthr-win32.h, which would not allow native condition variables to
be used because std::mutex is based on a semaphore and changing
gthr-win32.h to not use a semaphore would not be backwards compatible
and so is not an option.

In order to solve that problem I proposed a completely new gthreads
implementation, for argument's sake gthr-win64.h, which being
completely new could implement std::mutex differently, e.g. as a
critical section, and std::condition_variable as a Windows condition
variable,

As you are using gthr-posix.h you shouldn't need to care, you should
already have std::mutex, std::thread, std::condition_variable etc.

So now I hope everyone's on the same page, but I don't actually think
this thread has anything new since my original email on this subject
four days ago  :-\