[cfe-dev] -Wswitch warning patch

Douglas Gregor dgregor at apple.com
Wed Feb 3 11:02:45 PST 2010


On Jan 30, 2010, at 9:23 AM, Michal wrote:

> Attaching (I hope) final version.
> 
> On Thu, Jan 28, 2010 at 7:01 PM, Douglas Gregor <dgregor at apple.com> wrote:
>> 
>> On Jan 23, 2010, at 11:59 AM, Michal wrote:
>> 
>> Thank you both for replies. Attaching a newer, but not yet finished version.
>> 
>> 
>> 
>> +      // Iterate over Enum values and check which are missing in switch
>> 
>> +      CaseValsTy::const_iterator CI = CaseVals.begin();
>> 
>> +      for(EnumDecl::enumerator_iterator EI = ED->enumerator_begin(); EI !=
>> 
>> ED->enumerator_end(); EI++) {
>> 
>> +        if(CI == CaseVals.end() || EI->getInitVal().slt(CI->first))
>> 
>> Why use "slt" here? The values aren't guaranteed to be signed.
>> 
>> I misunderstood slt code. I already asked here how to compare ASPInts but
>> I am not sure whether it is appropriate to use your answer here. Are you ok
>> with
>> adding another function that copies and extends an ASPInt, and calling it
>> for
>> each comparison?
>> As for now I am using ordinary <,==, etc, which obviously breaks if enum has
>> no
>> negative values.
>> 
>> Every enumeration has an underlying integral type. You can ask that type
>> whether it is signed or unsigned, and use ASTContext's getTypeSize() to
>> determine its size (in bits). From that information, construct the APSInts
>> and compare them with ==.
> 
> I've choosen to copy enum constants and promote them to the same type
> as condition.
> This decision is mainly because it is possible to have to enum
> constants of different
> signedness.

Makes sense. The patch looks good, and I can commit it; do you have test cases?

	- Doug



More information about the cfe-dev mailing list