Re: [PATCH 16/22] Class-ify ui_out_level

Antoine Tremblay
2016-11-30T12:41:35Z

Pedro Alves writes:

> On 11/26/2016 04:22 PM, Simon Marchi wrote:
>> On 2016-11-24 13:41, Pedro Alves wrote:
>>>> -struct ui_out_level
>>>> +/* A level of nesting (either a list or a tuple) in a ui_out
>>>> output. */
>>>> +
>>>> +class ui_out_level
>>>> +{
>>>> + public:
>>>> +
>>>> + ui_out_level (ui_out_type type)
>>>
>>> explicit ?
>>
>> I had a discussion about this with Antoine. Is it a good practice to
>> use explicit all the time, and only omit it when there's a good reason
>> why?
>
> Yes, I believe so.
>
> Let's turn the question around: why _would_ you want to support
> implicit conversion from ui_out_type to ui_out_level?
> E.g, would this code make any sense?
>
> ui_out_level level = ui_out_type_list;
> if (level == ui_out_type_tuple)
>
> I'd leave implicit conversions for when you're actually trying
> to code a converting constructor.
>
>> I initially put it on all the constructors, but decided to drop it
>> after our discussion.
>

Note that in the discussion I argued that it would be a good idea to
keep it for single argument constructors, but that I though that there
was little chance of confusion for multiple argument constructors like:

ui_out_table (int entry_level, int nr_cols, const std::string &id)

(See: https://gcc.gnu.org/codingconventions.html#Conversions)

I'm starting to think we need to document these things as the GCC coding
conventions may not be exactly what we want and reading the ML there's
more and more questions about this...