<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Aug 24, 2015 at 2:10 PM, Amos Jeffries <span dir="ltr"><<a href="mailto:squid3@treenet.co.nz" target="_blank">squid3@treenet.co.nz</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 24/08/2015 1:14 p.m., Alex Rousskov wrote:<br>
> On 08/23/2015 04:01 PM, Kinkie wrote:<br>
>>     Adding comment: "will reallocate if ilen happens to equal pool size"<br>
><br>
> I would recommend not adding a comment at all if "no comment" and the<br>
> above "low-level description of the current c_str logic" are the two<br>
> choices, but it is your call. Not important.<br>
<br>
</span>Agreed.<br>
<br>
Which is also why I just suggested the terse "performance regression,<br>
c_str() reallocates" in all cases where it *can*. Removal of those cases<br>
is still important (thus X vs TODO) but not reasonable at current time.<br></blockquote><div><br></div><div>The performance regression will go as soon as we get strList to generate SBufs.<br></div><div> <br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
><br>
>>     example.cc:11:28: error: invalid operands to binary expression<br>
>>     ('foo' and 'foo')<br>
>>     std::int8_t var = foo::one | foo::four;<br>
>>                       ~~~~~~~~ ^ ~~~~~~~~~<br>
>>     1 error generated.<br>
><br>
> Thank you. We should not mix header IDs and header masks (a mask is not<br>
> an ID). I am not saying that you must change how Squid masks headers in<br>
> your patch, of course, but it does feel like the compiler has identified<br>
> a design flaw in our code. We should fix that at some point. It is your<br>
> call whether your patch is the right place for that fix.<br>
<br>
</span>This is not about header IDs - thats a different alteration leading to<br>
this change. This bitmask thing is about enumerating the property flags<br>
that can be set in the table record. So we dont have to list false<br>
explicitly for all the irrelevant ones, or to depend on ordering.<br>
<br>
<br>
The design flaw I think is in use of enum class at all. Its just not<br>
suitable for bitmasks since the OR/AND-ing individual mask values does<br>
not produce a valid point-value within the defined set of enum class<br>
point-values. Wantig the compiler to reject that is exactly when enum<br>
class should be used.<br>
<br>
The basic c-style enum provides implicit type-casting to int_type which<br>
is what we need when using an enum to name bitmask bits. It does not<br>
necessarily mean badness. Just use with care. Its bad reputation is from<br>
people who didn't, or were forced to abuse it.<br><br></blockquote><div><br></div><div>I am a bit wary of the implicit type-casting behavior, so I agree with Alex that we should in time get rid of this pattern. Just not now, we have more than enough stuff on our plate.<br><br> <br></div></div>Am I cleared to commit? The comments to my latest patch submissions were all about comments, and have been adopted.<br><br></div><div class="gmail_extra">Thanks<br></div><div class="gmail_extra"><br clear="all"><br>-- <br><div class="gmail_signature">    Francesco</div>
</div></div>