Add null identifiers to genmatch

Richard Sandiford rdsandiford@gmail.com
Mon Nov 16 21:52:00 GMT 2015


Jeff Law <law@redhat.com> writes:
>>>> Boolean params are best avoided if possible, IMO.  In this case,
>>>> it seems this could instead be a new wrapper function, like:
>>> This hasn't been something we've required for GCC.    I've come across
>>> this recommendation a few times over the last several months as I
>>> continue to look at refactoring and best practices for codebases such as
>>> GCC.
>>
>> FWIW, GDB is in progress of converting to C++ and we're pulling in
>> GCC's C++ conventions as reference, so I thought I'd see what the GCC
>> community thought here.
> FWIW, I often look at GCC's conventions, google's conventions, then 
> start doing google searches around this kind of stuff.  And as always, 
> the quality of the latter can vary wildly :-)
>
>>
>>>
>>> By encoding the boolean in the function's signature, it (IMHO) does make
>>> the code a bit easier to read, primarily because you don't have to go
>>> lookup the tense of the boolean).  The problem is when the boolean is
>>> telling us some property an argument, but there's more than one argument
>>> and other similar situations.
>>
>> There's more than one way to avoid boolean parameters.
>> If you have more than one of those, the boolean trap is even
>> worse IMO.  In such cases, enums can help make things clearer for
>> the reader.  E.g.:
>>
>>   foo (true, false);
>>   foo (false, true);
>>
>> vs:
>>
>>   foo (whatever::value1, bar::in_style);
> Yea, I saw the latter at some point as well.  In general I don't think 
> we've used enums as well as we could/should have in GCC through the years.

Yeah.  Kenny was adamant that for wide-int we should have an UNSIGNED/SIGNED
enum rather than a boolean flag.  And I think that does make things clearer.
I always have to remind myself whether "true" means "unsigned" or "signed",
especially for RTL functions.

I certainly prefer the enum to separate functions though.  They can get
messy if a new call site is added that needs a variable parameter.

Thanks,
Richard



More information about the Gcc-patches mailing list