[PATCH] D91895: [Clang] improve -Wimplicit-fallthrough GCC compat

[Kees Cook via Phabricator via cfe-commits]

kees added a comment.

The kernel's [[ https://www.kernel.org/doc/html/latest/process/deprecated.html#implicit-switch-case-fall-through

| stance ]] on switch statements reads: |
|



  All switch/case blocks must end in one of:
  
  break;
  fallthrough;
  continue;
  goto <label>;
  return [expression];

And we've had multiple real bugs now of the "missing break before default return" style:

https://git.kernel.org/linus/291ddeb621e4a9f1ced8302a777fbd7fbda058c6

So, from the kernel's perspective, please don't make `-Wimplicit-fallthrough` more permissive -- we're finding real bugs. :)

And to speak to another example in the thread:

  // drivers/hid/usbhid/hid-core.c
   490   case -ESHUTDOWN:  /* unplug */                                                                                                                                                                                                      
   491     unplug = 1; 
   492   case -EILSEQ:   /* protocol error or unplug */ // -Wimplicit-fallthrough...how?!                                                                                                                                     
   493   case -EPROTO:   /* protocol error or unplug */                                                                                                                                                                                      
   494   case -ECONNRESET: /* unlink */                                                                                                                                                                                                      
   495   case -ENOENT:                                                                                                                                                                                                                       
   496   case -EPIPE:    /* report not available */                                                                                                                                                                                          
   497     break;

I think this should warn too. While this won't turn into a "missing break" error, there's no way to know (from looking at code) what the _intent_ is here. Should it have done "return?" Why is "break" assumed to be safe here, etc?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91895/new/

https://reviews.llvm.org/D91895