llvm dev - Mar 2017 - Excessive use of LLVM_FALLTHROUGH?

I came across a weird-looking use of LLVM_FALLTHROUGH which I think is
completely spurious, but I figured I should check with the group mind
before ripping it out.  Basically, if you have multiple cases with no
code in between, you do *not* need LLVM_FALLTHROUGH, right?

  switch (Foo) {
  case Bar1:
    LLVM_FALLTHROUGH;  // not needed
  case Bar2:
    some code;
    return;
  case Bar3:
    LLVM_FALLTHROUGH; // not needed
  case Bar4:
    code without a break/return;
    LLVM_FALLTHROUGH; // <-- This one is needed.
  case Bar5:
    more code;
    break;
  default:
    llvm_unreachable("Foo with no Bar");
  }

So, can I take out all the ones marked "not needed" as an NFC cleanup?
Thanks,
--paulr

P.S. If you care, the specific example I tripped over is in
llvm/lib/CodeGen/AsmPrinter/DIE.cpp, DIEInteger::EmitValue().

On Wed, Mar 1, 2017 at 2:59 PM, Robinson, Paul via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> So, can I take out all the ones marked "not needed" as an NFC
cleanup?
>
Yep, go for it! It doesn't look like we build with -Wimplicit-fallthrough,
so all these annotations are really just for readability and future
proofing for when someone enables it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170301/3e9fb10e/attachment.html>

Dug and found
https://github.com/llvm-mirror/llvm/blob/88d207542b618ca6054b24491ddd67f8ca397540/include/llvm/Support/Compiler.h#L233


#if __cplusplus > 201402L && __has_cpp_attribute(fallthrough)
235
<https://github.com/llvm-mirror/llvm/blob/88d207542b618ca6054b24491ddd67f8ca397540/include/llvm/Support/Compiler.h#L235>
#define LLVM_FALLTHROUGH [[fallthrough]]
236
<https://github.com/llvm-mirror/llvm/blob/88d207542b618ca6054b24491ddd67f8ca397540/include/llvm/Support/Compiler.h#L236>
#elif !__cplusplus
which is for [[fallthrough]] in
http://en.cppreference.com/w/cpp/language/attributes

which just tells compiler not to emit warning on fall-through.

They're all warning removers.

Kevin

On Wed, Mar 1, 2017 at 2:59 PM, Robinson, Paul via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> I came across a weird-looking use of LLVM_FALLTHROUGH which I think is
> completely spurious, but I figured I should check with the group mind
> before ripping it out.  Basically, if you have multiple cases with no
> code in between, you do *not* need LLVM_FALLTHROUGH, right?
>
>   switch (Foo) {
>   case Bar1:
>     LLVM_FALLTHROUGH;  // not needed
>   case Bar2:
>     some code;
>     return;
>   case Bar3:
>     LLVM_FALLTHROUGH; // not needed
>   case Bar4:
>     code without a break/return;
>     LLVM_FALLTHROUGH; // <-- This one is needed.
>   case Bar5:
>     more code;
>     break;
>   default:
>     llvm_unreachable("Foo with no Bar");
>   }
>
> So, can I take out all the ones marked "not needed" as an NFC
cleanup?
> Thanks,
> --paulr
>
> P.S. If you care, the specific example I tripped over is in
> llvm/lib/CodeGen/AsmPrinter/DIE.cpp, DIEInteger::EmitValue().
>
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170301/8b8be20c/attachment.html>

Aha!  Trying out –Wimplicit-fallthrough, you don't need an annotation for
successive cases with no code in between.
So all of these really are just noise.
Thanks,
--paulr

From: Reid Kleckner [mailto:rnk at google.com]
Sent: Wednesday, March 01, 2017 3:04 PM
To: Robinson, Paul
Cc: llvm-dev at lists.llvm.org
Subject: Re: [llvm-dev] Excessive use of LLVM_FALLTHROUGH?

On Wed, Mar 1, 2017 at 2:59 PM, Robinson, Paul via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
So, can I take out all the ones marked "not needed" as an NFC cleanup?

Yep, go for it! It doesn't look like we build with -Wimplicit-fallthrough,
so all these annotations are really just for readability and future proofing for
when someone enables it.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20170301/e175be56/attachment-0001.html>