Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Martin Sebor
On 05/16/2017 01:20 PM, Tim Song wrote: On Tue, May 16, 2017 at 2:50 PM, Martin Sebor wrote: I'm not sure I understand why the ovl_iterator assignment needs to be provided but if it does, not also defining one on the derived class will call the base and return a reference to

Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Martin Sebor
On 05/16/2017 01:05 PM, Nathan Sidwell wrote: Martin, Thanks for taking a look. There's a whole patch series I hope to land over the next couple of weeks. Things may be clearer at that point. Just a couple of suggestions, It looks like the classes model the concept of Forward Iterator.

Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Nathan Sidwell
On 05/16/2017 03:27 PM, Tim Song wrote: https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Safe_bool Oh, I see. I'm not going to stop someone adding it, but for me, it's quite far down my priority list. I suspect that we don't want adding two ovl_iterators together to compile. Why would

Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Tim Song
On Tue, May 16, 2017 at 3:24 PM, Nathan Sidwell wrote: > On 05/16/2017 03:20 PM, Tim Song wrote: > >> Also, operator bool() seems suspect. Consider the safe bool idiom? > > ? https://en.wikibooks.org/wiki/More_C%2B%2B_Idioms/Safe_bool I suspect that we don't want adding two

Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Nathan Sidwell
On 05/16/2017 03:20 PM, Tim Song wrote: Also, operator bool() seems suspect. Consider the safe bool idiom? ? And is it intended that tree implicitly converts to both iterators, or should those constructors be explicit? Maybe. Not caused a problem in practice -- never passing iterators

Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Tim Song
On Tue, May 16, 2017 at 2:50 PM, Martin Sebor wrote: > I'm not sure I understand why the ovl_iterator assignment needs > to be provided but if it does, not also defining one on the derived > class will call the base and return a reference to the base, making > the result no

Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Nathan Sidwell
On 05/16/2017 03:05 PM, Nathan Sidwell wrote: PS More descriptive names would be a nice as well (neither lkp_ nor ovl_ is intuitive enough at first glance.) Maybe lookup_iter and overload_iter? Works for me -- I just noticed we had things like vec_iterator already, and continued that

Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Nathan Sidwell
Martin, Thanks for taking a look. There's a whole patch series I hope to land over the next couple of weeks. Things may be clearer at that point. Just a couple of suggestions, It looks like the classes model the concept of Forward Iterator. May I suggest to make them model it more

Re: [C++ PATCH] OVERLOAD iterators #1

2017-05-16 Thread Martin Sebor
On 05/16/2017 08:50 AM, Nathan Sidwell wrote: This patch implements new iterators for OVERLOADs. There are two iterators: ovl_iterator for a plain iterator, that held on a binding lkp_iterator for the overload set returned by lookup. To use them simply: for (lkp_iterator iter (INIT); iter;