[squid-dev] [PATCH] YesNoNone class updates

[Amos Jeffries]

On 15/01/2016 11:38 a.m., Alex Rousskov wrote:
> On 01/14/2016 12:31 PM, Amos Jeffries wrote:
>> On 15/01/2016 6:26 a.m., Alex Rousskov wrote:
>>> On 01/13/2016 03:56 PM, Amos Jeffries wrote:
>>>> On 13/01/2016 5:11 a.m., Alex Rousskov wrote:
>>>>> On 01/11/2016 06:36 PM, Amos Jeffries wrote:
>>>> +    /** forbidden operation.
>>>> +     * Use x.configure(bool) when the value is configured.
>>>> +     * Use x = YesNoNone(bool) to assign defaults.
>>>> +     */
>>>> +    YesNoNone &operator =(bool) = delete;
>>>
>>> Why do we need to delete something that does not exist? If you do not
>>> add the above code, do you really gain the ability to assign booleans to
>>> YesNoNone objects? If not, please remove.
> 
> 
>> We have a bool constructor and implicit copy-assignment. Which tempts
>> the compiler to try bool constructing then copying. 
> 
> Fortunately, we do not have just any "bool constructor". We have an
> _explicit_ bool constructor which prohibits implicit "bool constructing
> then copying", no matter how tempting it might seem to the compiler.
> 
> What would be the purpose of marking the constructor "explicit" if the
> imaginary conversion sequence you have described would still be
> possible, depending on the compiler or other factors? [Rhetorical]
> 
> Please either remove the bool assignment operator (instead of deleting
> what does not exist) or provide a use case where its deleted existence
> changes something other than the compiler error message text.
> 
> FWIW, in my tests, without the operator, the errors look like this:
> 
> error: conversion from ‘bool’ to non-scalar type ‘YesNoNone’ requested
> 

FYI: with GCC-5.3 and libstdc++6 I get both of these two buried amidst a
page of verbose compiler output:
 error: missing operator 'YesNoNone & operator =(const YesNoNone &)'
 error: missing operator 'YesNoNone && operator =(const YesNoNone &&)'

So the bool conversion seems to be happening despite explicit, but
compile dying at the lack of copy/move-assignment later down those
potential chains of operation.

Done anyway.

...
> 
> I do not think another round of reviews is needed.
> 

Updated and applied as trunk rev.14494.

Amos