diff mbox series

libstdc++: LWG 3301 transform_view::iterator has incorrect iterator_category

Message ID 20200224232643.851356-2-ppalka@redhat.com
State New
Headers show
Series libstdc++: LWG 3301 transform_view::iterator has incorrect iterator_category | expand

Commit Message

Patrick Palka Feb. 24, 2020, 11:26 p.m. UTC 
libstdc++-v3/ChangeLog:

	LWG 3301 transform_view::_Iterator has incorrect iterator_category
	* include/std/ranges (transform_view::_Iterator::_S_iter_cat): Adjust
	determination of iterator_category as per LWG 3301.
	* testsuite/std/ranges/adaptors/transform.cc: Augment test.
---
 libstdc++-v3/include/std/ranges               | 16 +++++++++----
 .../std/ranges/adaptors/transform.cc          | 24 +++++++++++++++++++
 2 files changed, 35 insertions(+), 5 deletions(-)

Comments

Patrick Palka Feb. 24, 2020, 11:39 p.m. UTC | #1 
On Mon, 24 Feb 2020, Patrick Palka wrote:

> libstdc++-v3/ChangeLog:
> 
> 	LWG 3301 transform_view::_Iterator has incorrect iterator_category
> 	* include/std/ranges (transform_view::_Iterator::_S_iter_cat): Adjust
> 	determination of iterator_category as per LWG 3301.
> 	* testsuite/std/ranges/adaptors/transform.cc: Augment test.
> ---
>  libstdc++-v3/include/std/ranges               | 16 +++++++++----
>  .../std/ranges/adaptors/transform.cc          | 24 +++++++++++++++++++
>  2 files changed, 35 insertions(+), 5 deletions(-)
> 
> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
> index ab8fbaca38f..aed90e9710e 100644
> --- a/libstdc++-v3/include/std/ranges
> +++ b/libstdc++-v3/include/std/ranges
> @@ -1570,12 +1570,18 @@ namespace views
>  	  static constexpr auto
>  	  _S_iter_cat()
>  	  {
> -	    using _Cat
> -              = typename iterator_traits<_Base_iter>::iterator_category;
> -	    if constexpr (derived_from<_Cat, contiguous_iterator_tag>)
> -	      return random_access_iterator_tag{};
> +	    using _Res = invoke_result_t<_Fp&, range_reference_t<_Base>>;

Consider this line fixed to use 'typename'.

> +	    if constexpr (is_lvalue_reference_v<_Res>)
> +	      {
> +		using _Cat
> +		  = typename iterator_traits<_Base_iter>::iterator_category;
> +		if constexpr (derived_from<_Cat, contiguous_iterator_tag>)
> +		  return random_access_iterator_tag{};
> +		else
> +		  return _Cat{};
> +	      }
>  	    else
> -	      return _Cat{};
> +	      return input_iterator_tag{};
>  	  }
>  
>  	  static constexpr decltype(auto)
> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> index ad51fffb43d..0845febe2cf 100644
> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> @@ -77,10 +77,34 @@ test03()
>    VERIFY( ranges::equal(v, (int[]){1,2,3,4,5}) );
>  }
>  
> +void
> +test04()
> +{
> +  // LWG 3302

Consider this comment fixed to say 3301.
Jonathan Wakely Feb. 25, 2020, 5:23 p.m. UTC | #2 
On 24/02/20 18:39 -0500, Patrick Palka wrote:
>On Mon, 24 Feb 2020, Patrick Palka wrote:
>
>> libstdc++-v3/ChangeLog:
>>
>> 	LWG 3301 transform_view::_Iterator has incorrect iterator_category
>> 	* include/std/ranges (transform_view::_Iterator::_S_iter_cat): Adjust
>> 	determination of iterator_category as per LWG 3301.
>> 	* testsuite/std/ranges/adaptors/transform.cc: Augment test.
>> ---
>>  libstdc++-v3/include/std/ranges               | 16 +++++++++----
>>  .../std/ranges/adaptors/transform.cc          | 24 +++++++++++++++++++
>>  2 files changed, 35 insertions(+), 5 deletions(-)
>>
>> diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
>> index ab8fbaca38f..aed90e9710e 100644
>> --- a/libstdc++-v3/include/std/ranges
>> +++ b/libstdc++-v3/include/std/ranges
>> @@ -1570,12 +1570,18 @@ namespace views
>>  	  static constexpr auto
>>  	  _S_iter_cat()
>>  	  {
>> -	    using _Cat
>> -              = typename iterator_traits<_Base_iter>::iterator_category;
>> -	    if constexpr (derived_from<_Cat, contiguous_iterator_tag>)
>> -	      return random_access_iterator_tag{};
>> +	    using _Res = invoke_result_t<_Fp&, range_reference_t<_Base>>;
>
>Consider this line fixed to use 'typename'.
>
>> +	    if constexpr (is_lvalue_reference_v<_Res>)
>> +	      {
>> +		using _Cat
>> +		  = typename iterator_traits<_Base_iter>::iterator_category;
>> +		if constexpr (derived_from<_Cat, contiguous_iterator_tag>)
>> +		  return random_access_iterator_tag{};
>> +		else
>> +		  return _Cat{};
>> +	      }
>>  	    else
>> -	      return _Cat{};
>> +	      return input_iterator_tag{};
>>  	  }
>>
>>  	  static constexpr decltype(auto)
>> diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
>> index ad51fffb43d..0845febe2cf 100644
>> --- a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
>> +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
>> @@ -77,10 +77,34 @@ test03()
>>    VERIFY( ranges::equal(v, (int[]){1,2,3,4,5}) );
>>  }
>>
>> +void
>> +test04()
>> +{
>> +  // LWG 3302
>
>Consider this comment fixed to say 3301.

OK, thanks.
Patrick Palka Feb. 25, 2020, 6:15 p.m. UTC | #3 
On Tue, 25 Feb 2020, Jonathan Wakely wrote:

> On 24/02/20 18:39 -0500, Patrick Palka wrote:
> > On Mon, 24 Feb 2020, Patrick Palka wrote:
> > 
> > > libstdc++-v3/ChangeLog:
> > > 
> > > 	LWG 3301 transform_view::_Iterator has incorrect iterator_category
> > > 	* include/std/ranges (transform_view::_Iterator::_S_iter_cat): Adjust
> > > 	determination of iterator_category as per LWG 3301.
> > > 	* testsuite/std/ranges/adaptors/transform.cc: Augment test.
> > > ---
> > >  libstdc++-v3/include/std/ranges               | 16 +++++++++----
> > >  .../std/ranges/adaptors/transform.cc          | 24 +++++++++++++++++++
> > >  2 files changed, 35 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/libstdc++-v3/include/std/ranges
> > > b/libstdc++-v3/include/std/ranges
> > > index ab8fbaca38f..aed90e9710e 100644
> > > --- a/libstdc++-v3/include/std/ranges
> > > +++ b/libstdc++-v3/include/std/ranges
> > > @@ -1570,12 +1570,18 @@ namespace views
> > >  	  static constexpr auto
> > >  	  _S_iter_cat()
> > >  	  {
> > > -	    using _Cat
> > > -              = typename iterator_traits<_Base_iter>::iterator_category;
> > > -	    if constexpr (derived_from<_Cat, contiguous_iterator_tag>)
> > > -	      return random_access_iterator_tag{};
> > > +	    using _Res = invoke_result_t<_Fp&, range_reference_t<_Base>>;
> > 
> > Consider this line fixed to use 'typename'.
> > 
> > > +	    if constexpr (is_lvalue_reference_v<_Res>)
> > > +	      {
> > > +		using _Cat
> > > +		  = typename iterator_traits<_Base_iter>::iterator_category;
> > > +		if constexpr (derived_from<_Cat, contiguous_iterator_tag>)
> > > +		  return random_access_iterator_tag{};
> > > +		else
> > > +		  return _Cat{};
> > > +	      }
> > >  	    else
> > > -	      return _Cat{};
> > > +	      return input_iterator_tag{};
> > >  	  }
> > > 
> > >  	  static constexpr decltype(auto)
> > > diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> > > b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> > > index ad51fffb43d..0845febe2cf 100644
> > > --- a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> > > +++ b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
> > > @@ -77,10 +77,34 @@ test03()
> > >    VERIFY( ranges::equal(v, (int[]){1,2,3,4,5}) );
> > >  }
> > > 
> > > +void
> > > +test04()
> > > +{
> > > +  // LWG 3302
> > 
> > Consider this comment fixed to say 3301.
> 
> OK, thanks.

Thanks for the review, I just committed all five patches.
diff mbox series

Patch

diff --git a/libstdc++-v3/include/std/ranges b/libstdc++-v3/include/std/ranges
index ab8fbaca38f..aed90e9710e 100644
--- a/libstdc++-v3/include/std/ranges
+++ b/libstdc++-v3/include/std/ranges
@@ -1570,12 +1570,18 @@  namespace views
 	  static constexpr auto
 	  _S_iter_cat()
 	  {
-	    using _Cat
-              = typename iterator_traits<_Base_iter>::iterator_category;
-	    if constexpr (derived_from<_Cat, contiguous_iterator_tag>)
-	      return random_access_iterator_tag{};
+	    using _Res = invoke_result_t<_Fp&, range_reference_t<_Base>>;
+	    if constexpr (is_lvalue_reference_v<_Res>)
+	      {
+		using _Cat
+		  = typename iterator_traits<_Base_iter>::iterator_category;
+		if constexpr (derived_from<_Cat, contiguous_iterator_tag>)
+		  return random_access_iterator_tag{};
+		else
+		  return _Cat{};
+	      }
 	    else
-	      return _Cat{};
+	      return input_iterator_tag{};
 	  }
 
 	  static constexpr decltype(auto)
diff --git a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
index ad51fffb43d..0845febe2cf 100644
--- a/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
+++ b/libstdc++-v3/testsuite/std/ranges/adaptors/transform.cc
@@ -77,10 +77,34 @@  test03()
   VERIFY( ranges::equal(v, (int[]){1,2,3,4,5}) );
 }
 
+void
+test04()
+{
+  // LWG 3302
+    {
+      auto f = [] (int x) { return x; };
+      int x[] = {1,2,3,4,5};
+      auto v = x | views::transform(f);
+      auto i = v.begin();
+      using Cat = decltype(i)::iterator_category;
+      static_assert(std::same_as<Cat, std::input_iterator_tag>);
+    }
+
+    {
+      auto f = [] (int &x) -> int& { return x; };
+      int x[] = {1,2,3,4,5};
+      auto v = x | views::transform(f);
+      auto i = v.begin();
+      using Cat = decltype(i)::iterator_category;
+      static_assert(std::derived_from<Cat, std::forward_iterator_tag>);
+    }
+}
+
 int
 main()
 {
   test01();
   test02();
   test03();
+  test04();
 }