Bug 58191 - Can't use boost transform_iterator with _GLIBCXX_DEBUG
Summary: Can't use boost transform_iterator with _GLIBCXX_DEBUG
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: libstdc++ (show other bugs)
Version: 4.8.1
: P3 enhancement
Target Milestone: 4.8.3
Assignee: François Dumont
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-08-19 05:45 UTC by Danil Ilinykh
Modified: 2013-10-16 20:18 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2013-08-30 00:00:00


Attachments
Preprocessed file archive (100.29 KB, application/x-lzma)
2013-08-19 05:47 UTC, Danil Ilinykh
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Danil Ilinykh 2013-08-19 05:45:03 UTC
Given:
A simple code:

int calc_value(int original_value)
{
	return original_value % 3;
}

int main()
{
	std::vector<int> numbers;
	std::upper_bound(
		boost::make_transform_iterator(numbers.begin(), calc_value),
		boost::make_transform_iterator(numbers.end(), calc_value),
		1);
}

Gcc version: g++ (Ubuntu 4.8.1-2ubuntu1~13.04) 4.8.1
Additional command line params: -D_GLIBCXX_DEBUG

Result: Compilation error
In file included from /usr/include/c++/4.8/debug/debug.h:127:0,
                 from /usr/include/c++/4.8/bits/stl_iterator_base_funcs.h:65,
                 from /usr/include/c++/4.8/bits/stl_algobase.h:66,
                 from /usr/include/c++/4.8/vector:60,
                 from 1.cpp:1:
/usr/include/c++/4.8/debug/functions.h: In instantiation of ‘bool __gnu_debug::__check_partitioned_upper(_ForwardIterator, _ForwardIterator, const _Tp&) [with _ForwardIterator = boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default>; _Tp = int]’:
/usr/include/c++/4.8/bits/stl_algo.h:2506:358:   required from ‘_FIter std::upper_bound(_FIter, _FIter, const _Tp&) [with _FIter = boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default>; _Tp = int]’
1.cpp:16:4:   required from here
/usr/include/c++/4.8/debug/functions.h:428:42: error: no matching function for call to ‘__check_partitioned_upper_aux(boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default>&, boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default>&, const int&, std::iterator_traits<boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default> >::iterator_category)’
         std::__iterator_category(__first));
                                          ^
/usr/include/c++/4.8/debug/functions.h:428:42: note: candidates are:
/usr/include/c++/4.8/debug/functions.h:393:5: note: template<class _ForwardIterator, class _Tp> bool __gnu_debug::__check_partitioned_upper_aux(_ForwardIterator, _ForwardIterator, const _Tp&, std::forward_iterator_tag)
     __check_partitioned_upper_aux(_ForwardIterator __first,
     ^
/usr/include/c++/4.8/debug/functions.h:393:5: note:   template argument deduction/substitution failed:
/usr/include/c++/4.8/debug/functions.h:428:42: note:   cannot convert ‘std::__iterator_category<boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default> >((*(const boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default>*)(& __first)))’ (type ‘std::iterator_traits<boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default> >::iterator_category {aka boost::detail::iterator_category_with_traversal<std::input_iterator_tag, boost::random_access_traversal_tag>}’) to type ‘std::forward_iterator_tag’
         std::__iterator_category(__first));
                                          ^
/usr/include/c++/4.8/debug/functions.h:412:5: note: template<class _Iterator, class _Sequence, class _Tp> bool __gnu_debug::__check_partitioned_upper_aux(const __gnu_debug::_Safe_iterator<_Iterator, _Sequence>&, const __gnu_debug::_Safe_iterator<_Iterator, _Sequence>&, const _Tp&, std::random_access_iterator_tag)
     __check_partitioned_upper_aux(
     ^
/usr/include/c++/4.8/debug/functions.h:412:5: note:   template argument deduction/substitution failed:
/usr/include/c++/4.8/debug/functions.h:428:42: note:   ‘boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default>’ is not derived from ‘const __gnu_debug::_Safe_iterator<_Iterator, _Sequence>’
         std::__iterator_category(__first));
                                          ^
/usr/include/c++/4.8/debug/functions.h:498:5: note: template<class _Iterator, class _Sequence, class _Tp, class _Pred> bool __gnu_debug::__check_partitioned_upper_aux(const __gnu_debug::_Safe_iterator<_Iterator, _Sequence>&, const __gnu_debug::_Safe_iterator<_Iterator, _Sequence>&, const _Tp&, _Pred, std::random_access_iterator_tag)
     __check_partitioned_upper_aux(
     ^
/usr/include/c++/4.8/debug/functions.h:498:5: note:   template argument deduction/substitution failed:
/usr/include/c++/4.8/debug/functions.h:428:42: note:   ‘boost::transform_iterator<int (*)(int), __gnu_debug::_Safe_iterator<__gnu_cxx::__normal_iterator<int*, std::__cxx1998::vector<int, std::allocator<int> > >, std::__debug::vector<int> >, boost::use_default, boost::use_default>’ is not derived from ‘const __gnu_debug::_Safe_iterator<_Iterator, _Sequence>’
         std::__iterator_category(__first));
                                          ^
/usr/include/c++/4.8/debug/functions.h:477:5: note: template<class _ForwardIterator, class _Tp, class _Pred> bool __gnu_debug::__check_partitioned_upper_aux(_ForwardIterator, _ForwardIterator, const _Tp&, _Pred, std::forward_iterator_tag)
     __check_partitioned_upper_aux(_ForwardIterator __first,
     ^
/usr/include/c++/4.8/debug/functions.h:477:5: note:   template argument deduction/substitution failed:
/usr/include/c++/4.8/debug/functions.h:428:42: note:   candidate expects 5 arguments, 4 provided
         std::__iterator_category(__first));
                                          ^

gcc 4.7.3 works fine
Comment 1 Danil Ilinykh 2013-08-19 05:47:58 UTC
Created attachment 30671 [details]
Preprocessed file archive
Comment 2 Paolo Carlini 2013-08-19 09:31:30 UTC
With 100 KB of code to look at, this could be anything, could be front-end, could be library, could well be boost.

Francois, did we change anything in the library for 4.8.x?
Comment 3 Daniel Krügler 2013-08-19 09:34:40 UTC
First, this issue should be categorized as belonging to the component "libstdc++", not to "c++".

Second, the defect report is invalid, because std::upper_bound requires a minimum iterator category of "forward iterator", but boost::transform_iterator with a function object that returns a value (not a real reference) has an iterator category "input iterator", which is correct, since forward iterators are required to return a real reference for operator*.
Comment 4 Paolo Carlini 2013-08-19 09:36:15 UTC
Oh well, thanks Daniel for helping on this.
Comment 5 Daniel Krügler 2013-08-19 09:47:45 UTC
(In reply to Paolo Carlini from comment #2)
> Francois, did we change anything in the library for 4.8.x?

I think that Francois added more iterator concept checking and this one looks correct. Unfortunately I would say, this is correct, because the standard says so, but from a practical point of view, std::upper_bound "just works" for any forward traversal iterator and it does not require that the return type of operator* *really* is a reference type. We also cannot blame boost here, because it is not their fault that the standard requires for forward iterators to have a real reference return type for iterator dereference. My recommendation to the issue reporter would be to provide a function object that returns a real reference. This can be easily done by a unary functor adaptor such as the following one:

template<class UnaryFunction, class ArgumentType>
struct LValueUnaryFunctionAdaptor
{
  typedef typename std::result_of<const UnaryFunction(ArgumentType)>::type value_type;
  LValueUnaryFunctionAdaptor(UnaryFunction func) : func(func) {}
  typedef const value_type& result_type;
  result_type operator()(ArgumentType arg) const
  {
    return res = func(arg);
  }
private:
  UnaryFunction func;
  mutable value_type res;
};

template<class ArgumentType, class UnaryFunction>
inline
LValueUnaryFunctionAdaptor<UnaryFunction, ArgumentType> make_LValueUnaryFunction(UnaryFunction func)
{
  return LValueUnaryFunctionAdaptor<UnaryFunction, ArgumentType>(func);
}

So instead of

calc_value

he could use

make_LValueUnaryFunction<int>(calc_value)

This trick doesn't make the transform_iterator fully conforming, but it should convince the compile-time test machinery.
Comment 6 Danil Ilinykh 2013-08-19 09:51:23 UTC
Thank you!
Comment 7 Jonathan Wakely 2013-08-19 10:05:03 UTC
(In reply to Daniel Krügler from comment #5)
> (In reply to Paolo Carlini from comment #2)
> > Francois, did we change anything in the library for 4.8.x?
> 
> I think that Francois added more iterator concept checking and this one
> looks correct.

I think the change in behaviour is a consequence of http://gcc.gnu.org/bugzilla/show_bug.cgi?id=53263#c11

Potentially we could disable the partitioning checks when input iterators are used (unless _GLIBCXX_DEBUG_PEDANTIC is defined) or explicitly check for input iterators and give a better diagnostic.
Comment 8 Paolo Carlini 2013-08-19 11:08:16 UTC
Thanks everybody. Lower priority wrt regressions, I agree we could do even better here, just bailing out for input iterators would be Ok with me too (wouldn't be the first time we do that), but anything that suits you and our debug-mode expert Francois is basically Ok with me. Thanks again.
Comment 9 François Dumont 2013-08-30 20:16:06 UTC
Author: fdumont
Date: Fri Aug 30 20:16:03 2013
New Revision: 202119

URL: http://gcc.gnu.org/viewcvs?rev=202119&root=gcc&view=rev
Log:
2013-08-30  François Dumont  <fdumont@gcc.gnu.org>

	PR libstdc++/58191
	* include/debug/macros.h (__glibcxx_check_partitioned_lower): Add
	__gnu_debug::__base calls on iterators passed to internal debug
	check.
	(__glibcxx_check_partitioned_lower_pred): Likewise.
	(__glibcxx_check_partitioned_upper): Likewise.
	(__glibcxx_check_partitioned_upper_pred): Likewise.
	(__glibcxx_check_sorted): Likewise.
	(__glibcxx_check_sorted_pred): Likewise.
	(__glibcxx_check_sorted_set): Likewise.
	(__glibcxx_check_sorted_set_pred): Likewise.
	* include/debug/functions.h (__check_partitioned_lower):
	Remove code to detect safe iterators.
	(__check_partitioned_upper): Likewise.
	(__check_sorted): Likewise.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/debug/functions.h
    trunk/libstdc++-v3/include/debug/macros.h
Comment 10 Paolo Carlini 2013-08-30 21:06:52 UTC
Let's have this open as <enhancement>, already implemented for 4.9.0.
Comment 11 François Dumont 2013-10-16 20:09:21 UTC
Author: fdumont
Date: Wed Oct 16 20:09:18 2013
New Revision: 203719

URL: http://gcc.gnu.org/viewcvs?rev=203719&root=gcc&view=rev
Log:
2013-10-16  François Dumont  <fdumont@gcc.gnu.org>

	PR libstdc++/58191
	* include/debug/macros.h (__glibcxx_check_partitioned_lower): Add
	__gnu_debug::__base calls on iterators passed to internal debug
	check.
	(__glibcxx_check_partitioned_lower_pred): Likewise.
	(__glibcxx_check_partitioned_upper): Likewise.
	(__glibcxx_check_partitioned_upper_pred): Likewise.
	* include/debug/functions.h (__check_partitioned_lower):
	Remove code to detect safe iterators.
	(__check_partitioned_upper): Likewise.

Modified:
    branches/gcc-4_8-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_8-branch/libstdc++-v3/include/debug/functions.h
    branches/gcc-4_8-branch/libstdc++-v3/include/debug/macros.h
Comment 12 François Dumont 2013-10-16 20:18:36 UTC
Fixed for 4.8.3 version.