auto and const_iterators

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

auto and const_iterators

Rik-4
jwe,

I noticed that you changed most instances of for loops to use the keyword
auto to determine type.  This definitely helps to reduce line length and
improve the readability of the code, but I don't think auto can be used to
get a const_iterator.  Taking one example from lo-regexp.cc

-        regexp::match_data::const_iterator p = rx_lst.begin ();
+        auto p = rx_lst.begin ();

The method begin() is going to return a regular iterator.  This will
certainly work, but then there won't be a compile time check if the code
attempts to modify rx_lst accidentally, which it shouldn't be doing.  I
don't think you can solve it by using 'const auto p', because then the
iterator itself is fixed so p++ will fail if you try to advance.  If this
were C++14 then we could call the method cbegin() which has a function
prototype that returns const_iterator so auto would correctly deduce the
right type.  Stack Overflow doesn't recommend using auto in this scenario:
https://stackoverflow.com/questions/15233188/how-do-i-get-a-const-iterator-using-auto#15233309
Although, if you're open to further code changes then it would be possible
to work on a const reference to the original.

const auto& c_rx_list = rx_lst;
auto p = c_rx_list.begin ();

Most of the iterators in cset 3ff9192b676e are not constant so it's really
deciding what to do about the handful that are const.  Should they just be
reverted to using long syntax?

--Rik




Reply | Threaded
Open this post in threaded view
|

Re: auto and const_iterators

John W. Eaton
Administrator
On 05/02/2018 05:29 PM, Rik wrote:

> I noticed that you changed most instances of for loops to use the keyword
> auto to determine type.  This definitely helps to reduce line length and
> improve the readability of the code, but I don't think auto can be used to
> get a const_iterator.  Taking one example from lo-regexp.cc
>
> -        regexp::match_data::const_iterator p = rx_lst.begin ();
> +        auto p = rx_lst.begin ();
>
> The method begin() is going to return a regular iterator.  This will
> certainly work, but then there won't be a compile time check if the code
> attempts to modify rx_lst accidentally, which it shouldn't be doing.  I
> don't think you can solve it by using 'const auto p', because then the
> iterator itself is fixed so p++ will fail if you try to advance.  If this
> were C++14 then we could call the method cbegin() which has a function
> prototype that returns const_iterator so auto would correctly deduce the
> right type.  Stack Overflow doesn't recommend using auto in this scenario:
> https://stackoverflow.com/questions/15233188/how-do-i-get-a-const-iterator-using-auto#15233309.
> Although, if you're open to further code changes then it would be possible
> to work on a const reference to the original.

Oops.

> const auto& c_rx_list = rx_lst;
> auto p = c_rx_list.begin ();
>
> Most of the iterators in cset 3ff9192b676e are not constant so it's really
> deciding what to do about the handful that are const.  Should they just be
> reverted to using long syntax?

Either solution is OK with me, but I see more lines that remove
const_iterator than iterator.  Are the only ones we have to change the
ones where the object itself is not const but we were accessing elements
with a const_iterator?

jwe

Reply | Threaded
Open this post in threaded view
|

Re: auto and const_iterators

Rik-4
On 05/02/2018 02:52 PM, John W. Eaton wrote:
>
> Either solution is OK with me, but I see more lines that remove
> const_iterator than iterator.  Are the only ones we have to change the
> ones where the object itself is not const but we were accessing elements
> with a const_iterator?

That's right.  It's only when we want to confirm to the compiler that the
access should be read-only which is only necessary for a mutable object. 
If the entire object is const already then all accesses are necessarily
read-only.

--Rik

Reply | Threaded
Open this post in threaded view
|

Re: auto and const_iterators

John W. Eaton
Administrator
On 05/02/2018 06:06 PM, Rik wrote:

> On 05/02/2018 02:52 PM, John W. Eaton wrote:
>>
>> Either solution is OK with me, but I see more lines that remove
>> const_iterator than iterator.  Are the only ones we have to change the
>> ones where the object itself is not const but we were accessing elements
>> with a const_iterator?
>
> That's right.  It's only when we want to confirm to the compiler that the
> access should be read-only which is only necessary for a mutable object.
> If the entire object is const already then all accesses are necessarily
> read-only.

OK, I can fix this, but is there an easier way to find them other than
manually looking at each of the instances where a const_iterator
declaration was replaced by auto?

jwe

Reply | Threaded
Open this post in threaded view
|

Re: auto and const_iterators

Rik-4
On 05/02/2018 03:10 PM, John W. Eaton wrote:

> On 05/02/2018 06:06 PM, Rik wrote:
>> On 05/02/2018 02:52 PM, John W. Eaton wrote:
>>>
>>> Either solution is OK with me, but I see more lines that remove
>>> const_iterator than iterator.  Are the only ones we have to change the
>>> ones where the object itself is not const but we were accessing elements
>>> with a const_iterator?
>>
>> That's right.  It's only when we want to confirm to the compiler that the
>> access should be read-only which is only necessary for a mutable object.
>> If the entire object is const already then all accesses are necessarily
>> read-only.
>
> OK, I can fix this, but is there an easier way to find them other than
> manually looking at each of the instances where a const_iterator
> declaration was replaced by auto?

Unfortunately, I don't think so :(

--Rik

Reply | Threaded
Open this post in threaded view
|

Re: auto and const_iterators

John W. Eaton
Administrator
On 05/02/2018 06:22 PM, Rik wrote:

> On 05/02/2018 03:10 PM, John W. Eaton wrote:
>> On 05/02/2018 06:06 PM, Rik wrote:
>>> On 05/02/2018 02:52 PM, John W. Eaton wrote:
>>>>
>>>> Either solution is OK with me, but I see more lines that remove
>>>> const_iterator than iterator.  Are the only ones we have to change the
>>>> ones where the object itself is not const but we were accessing elements
>>>> with a const_iterator?
>>>
>>> That's right.  It's only when we want to confirm to the compiler that the
>>> access should be read-only which is only necessary for a mutable object.
>>> If the entire object is const already then all accesses are necessarily
>>> read-only.
>>
>> OK, I can fix this, but is there an easier way to find them other than
>> manually looking at each of the instances where a const_iterator
>> declaration was replaced by auto?
>
> Unfortunately, I don't think so :(

I pushed the following changeset:

   http://hg.savannah.gnu.org/hgweb/octave/rev/416856765a55

If the object was already const or could be made const (directly, not
with a second const reference), I used auto.  Otherwise, I made a note
that auto could be used in combination with cbegin, cend, crbegin, or
crend in the future.

Then I checked and found that cbegin, cend, crbegin, and crend methods
were already present in C++11 standard containers.  If I understand
correctly, C++14 adds a template that can be used to create these from
objects that don't provide them.  So I also pushed this changeset:

   http://hg.savannah.gnu.org/hgweb/octave/rev/4d7790d9793f

jwe