[cfe-dev] -Wswitch warning patch

Michal korgulec at gmail.com
Sat Jan 23 11:59:30 PST 2010


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.

>  - There may be multiple enumeration constants with the same value
> (FirstDecl = 1, VarDecl = 1, FunctionDecl = 2, etc.); please make sure that
> we do not warn more than once if both FirstDecl and VarDecl are missing, and
> that the presence of such enumerators does not cause spurious warnings.
> (std::unique could eliminate duplicates in the list of enumerators).
>
>  - Try to make sure that recovery is good; if we complain about the second
> enumerator missing, it shouldn't cause a cascade of failures.
>
Not sure if I understood you correctly, if a switch misses more than two
different values should we report only the first two? If so, I think
that at least
there should be a warning notifying that there are more, but just not
printed, missing
enum constants.


Michal.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Wswitch.patch
Type: application/octet-stream
Size: 5021 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20100123/697a4655/attachment.obj>


More information about the cfe-dev mailing list