<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 24, 2015 at 8:37 PM, Alex Rousskov <span dir="ltr"><<a href="mailto:rousskov@measurement-factory.com" target="_blank">rousskov@measurement-factory.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">On 08/24/2015 12:08 PM, Amos Jeffries wrote:<br>
> On 25/08/2015 4:49 a.m., Kinkie wrote:<br>
<br>
</span>> + * behavior is undefined if the iterator<br>
> + * is incremented (or decremented) outside the range representing valid<br>
> + * enum symbols<br>
<br>
I hope that behavior is well defined: For many enums/iterators,<br>
iterators are "incremented outside the range representing valid enum<br>
symbols" to reach end() and rend(). I asked you about these beyond-range<br>
increments, and you assured me that they work fine. We should not add a<br>
comment saying otherwise.<br></blockquote><div><br></div><div>I may have difficulty expressing myself here. The iterators work just fine outside the range. Dereferencing them is another matter, as there is no valid symbol to cast them to.<br></div><div>  <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
++end() may be undefined, but the result of ++i should be well defined<br>
for an i value pointing to the last (or first) enum value. That result<br>
is what we usually compare with end() or rend()!<br></blockquote><div><br></div><div>Yes, and it is.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
If you changed your mind, we have to remove reverse iterators and<br>
require end markers for all enums (as one of the possible solutions).<br>
That would guarantee that iterators are not incremented or decremented<br>
beyond the enum symbols range.<br></blockquote><div><br></div><div>I don't think that's needed. What I want to highlight in that comment is that there's no preemptive checks on these out-of-bounds conditions happening; this is I believe an analogy with what happens with many std:: iterators.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
>> + *   zeroth, first, second, onePastLast, fourth<br>
<br>
<br>
Please make onePastLast last.<br></blockquote><div><br></div><div>That's actually intentional, even though the name may be unfortunate, and could be renamed to "three". I'm trying to highlight the fact that the dereferencing will only happen on EnumType::first and EnumType::second. Any suggestion on how to clearly express it is welcome.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
You might want to add a comment that EnumRange() cannot be used to<br>
iterate whole enums without [end] markers and, hence, all enums that<br>
require iteration of the entire range must have markers.<br></blockquote><div><br></div><div>I'm trying to detail that in the documentation for EnumRangeT. Not clearly enough, I guess.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="">
> * the iterator_type being used in static_cast before the typedef _looks_<br>
> very dangerous (even if its fine and works).<br>
>  - I would place typedef up the top in their own protected area to avoid<br>
> a) depending on implicit compiler behaviour and b) future developer<br>
> mistakes/misunderstanding.<br>
<br>
</span>And you can make the typedef public to simplify.<br></blockquote><div><br></div><div>Yes, ok.<br></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<span class=""><br>
<br>
> I dont see the first-user code that we normally require for accepting<br>
> these. I know its coming with a rather big followup, just would be good<br>
> if there was a simple use that could be used as exemplar.<br>
<br>
</span>I have asked for usage examples in regular Squid code as well. I agree<br>
that they should become a part of this patch, but I still do not insist<br>
on that.<br></blockquote><div><br></div><div>Ok.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
I do not have any other new comments and have no objections to the patch.<br></blockquote><div><br></div><div>Thank you, this is quite a lot already.<br></div></div><br clear="all"><br>-- <br><div class="gmail_signature">    Francesco</div>
</div></div>