[llvm-commits] Removing redundant default cases

[David Blaikie]

On Tue, Jan 10, 2012 at 8:56 AM, David Blaikie <dblaikie at gmail.com> wrote:
> Thanks Bill,
>
> Committed as r147855 building -Wall -Werror clean in clang ToT. I
> haven't tried re-adding my prototype patch to clang that found these
> errors in the first place, so I'm not sure we're still totally
> extraneous-label, but I'll fixup any instances that come up when I
> update that patch & hopefully get it committed.

Actually I have one other case that I have a fix for already - but
it's a bit more involved so I didn't send it out with the others. See
the attached patch.

In this case the enums in CommandLine.h for the llvm::Option type all
have an extra value, their mask, which is only used internally within
llvm::Option to bitpack an enum of each type into a single unsigned.
Instead I've pulled these out into a bitfield & removed the
unnecessary masks (this does add a more subtle dependency - any
increase in the number of elements in these flags will require
adjusting the size of the corresponding bitfield (not sure if the
compiler's already smart enough to detect this & warn - perhaps it
should be) - but it's less painful than the original code that
would've required updating every value in every enum that is after the
one you modified).

This allows clients of llvm::Option to use these enums without having
to account for a value they should never actually see.

Yes/no/maybe?

- David

> (another case where it'd be nice to add that warning to the "on by
> default in LLVM/Clang builds" (though if I'm lucky I can justify
> adding the extraneous-label warning as on by default in clang
> universally, to match -Wswitch-enum being so) so any tips on how/where
> we can add warnings to a list so people don't have to explicitly
> enable them would be great (maybe we should just have -Wall -Werror on
> by default since that's the recommended practice?))
>
> - David
>
> On Tue, Jan 10, 2012 at 1:15 AM, Bill Wendling <wendling at apple.com> wrote:
>> Hi David,
>>
>> The patch looks good to me. Please make sure that LLVM compiles without warnings with your patch. :-)
>>
>> -bw
>>
>> On Jan 10, 2012, at 12:26 AM, David Blaikie wrote:
>>
>>> <bump>
>>>
>>> On Mon, Dec 19, 2011 at 6:16 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>> [reattached the patch (sorry I didn't attach it in the bump - didn't
>>>> want to fill people's mailboxes with duplicates, but I realize the
>>>> original mail might've been so old as to have dropped out of people's
>>>> caches) - right this second I don't have a moment to update it so this
>>>> may or may not apply cleanly to ToT - I'll update again when I have a
>>>> moment to do that and possibly to reapply my clang hack to see if any
>>>> new violations have been introduced]
>>>>
>>>> - David
>>>>
>>>> On Mon, Dec 19, 2011 at 5:11 PM, Chris Lattner <clattner at apple.com> wrote:
>>>>> Hi David,
>>>>>
>>>>> Can you repost the patch?  The general idea of the patch seems sound.
>>>>>
>>>>> -Chris
>>>>>
>>>>> On Dec 14, 2011, at 12:05 PM, David Blaikie wrote:
>>>>>
>>>>>> Bump - any interest? (this may be a bit outdated, I can resurrect &
>>>>>> update it if desired)
>>>>>>
>>>>>> On Fri, Sep 23, 2011 at 7:22 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>>>>> A similar patch for one I've just sent out for clang - removing any default
>>>>>>> cases in a switch that already has all the enum values specified (only for
>>>>>>> switches over enums).
>>>>>>>
>>>>>>> I don't know whether this stylistic/coding convention is consistent between
>>>>>>> llvm and clang, so thought I'd ask first. Ideally once we get a good warning
>>>>>>> for this in clang we could turn it on by default.
>>>>>> _______________________________________________
>>>>>> llvm-commits mailing list
>>>>>> llvm-commits at cs.uiuc.edu
>>>>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>>>>
>>> <llvm_excess_default_fixes.diff>_______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: command_line_enums.diff
Type: text/x-diff
Size: 17571 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120110/23bb8f7c/attachment.diff>