[webkit-reviews] review granted: [Bug 127637] Add FALLTHROUGH and -Wimplicit-fallthrough to warn about unannotated implicit fallthroughs in switches : [Attachment 222243] [PATCH] Proposed Fix

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Joseph Pecoraro
<joepeck at webkit.org>'s request for review:
Bug 127637: Add FALLTHROUGH and -Wimplicit-fallthrough to warn about
unannotated implicit fallthroughs in switches
https://bugs.webkit.org/show_bug.cgi?id=127637

Attachment 222243: [PATCH] Proposed Fix
https://bugs.webkit.org/attachment.cgi?id=222243&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=222243&action=review


> Source/WTF/wtf/Compiler.h:159
> +#if !defined(FALLTHROUGH) && COMPILER_SUPPORTS(FALLTHROUGH_WARNINGS)
> +#if COMPILER(CLANG)

Should just be a single #if with another && in it.

> Source/WTF/wtf/Compiler.h:165
> +#define FALLTHROUGH do { } while (false)

Can this just be defined to nothing? I wouldn’t think it needs to be a
statement (see my comments below about semicolons).

> Source/WTF/wtf/dtoa/fast-dtoa.cc:257
> +		   FALLTHROUGH;

Does this really require a semicolon after it? That doesn’t seem quite right to
me, but maybe that’s how the clang feature works?

I don’t think we should keep the "// else fallthrough" comments since we are
adding the FALLTHROUGH macros, which say the same thing.