clang's -Wimplicit-fallthrough warnings (not yet enabled in mozilla-central) warn about switch cases that fall through without a break or return statement. MOZ_FALLTHROUGH (bug 1215411) is an annotation to suppress -Wimplicit-fallthrough warnings about switch cases that intentionally fall through without a break or return statement. MOZ_FALLTHROUGH is only needed on cases that have code. MOZ_FALLTHROUGH_ASSERT (bug 1235277) will suppress -Wimplicit-fallthrough warnings about switch cases that MOZ_ASSERT(false) (or its alias MOZ_ASSERT_UNREACHABLE) in debug builds, but intentionally fall through in release builds. Why do we need MOZ_FALLTHROUGH_ASSERT in addition to MOZ_FALLTHROUGH? In release builds, the MOZ_ASSERT(false) will expand to `do { } while (0)`, requiring a MOZ_FALLTHROUGH annotation to suppress a -Wimplicit-fallthrough warning. In debug builds, the MOZ_ASSERT(false) will expand to something like `if (true) { MOZ_CRASH(); }` and the MOZ_FALLTHROUGH annotation will cause a -Wunreachable-code warning. The MOZ_FALLTHROUGH_ASSERT macro breaks this warning stalemate.

js/src/asmjs/AsmJS.cpp:2636:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/asmjs/AsmJS.cpp:3209:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/builtin/ReflectParse.cpp:2583:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/ctypes/CTypes.cpp:4032:3 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/dtoa.c:1514:3 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/dtoa.c:1518:3 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/dtoa.c:1606:4 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/dtoa.c:2792:3 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/dtoa.c:2800:3 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/frontend/BytecodeEmitter.cpp:2072:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/frontend/BytecodeEmitter.cpp:8335:7: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough] js/src/frontend/FoldConstants.cpp:245:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/frontend/NameFunctions.cpp:161:15 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/frontend/NameFunctions.cpp:731:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/frontend/Parser.cpp:6023:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/frontend/Parser.cpp:6056:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/frontend/Parser.cpp:6326:15 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/frontend/Parser.cpp:9325:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/irregexp/RegExpEngine.cpp:3626:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/irregexp/RegExpParser.cpp:871:15 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/irregexp/RegExpParser.cpp:950:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/BacktrackingAllocator.cpp:2493:15 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/BaselineIC.cpp:1906:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/IonBuilder.cpp:717:15 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/IonBuilder.cpp:1538:15 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/IonBuilder.cpp:1893:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/JitcodeMap.h:926:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/JitcodeMap.h:944:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/Lowering.cpp:1750:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/Lowering.cpp:1806:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/MIR.cpp:2184:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/MIR.cpp:2264:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/MIR.cpp:3042:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/MIR.cpp:3329:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/MacroAssembler.cpp:552:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/RangeAnalysis.cpp:265:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/RangeAnalysis.cpp:273:11 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/RangeAnalysis.cpp:3009:15 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jit/shared/CodeGenerator-shared-inl.h:299:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsdtoa.cpp:140:13 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsgc.cpp:5937:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsgc.cpp:5953:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsgc.cpp:5999:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsgc.cpp:6012:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsiter.cpp:456:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsiter.cpp:457:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsiter.cpp:458:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsiter.cpp:459:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsiter.cpp:460:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsiter.cpp:461:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsiter.cpp:462:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsstr.cpp:1133:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsstr.cpp:1134:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsstr.cpp:1135:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsstr.cpp:1136:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsstr.cpp:1137:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsstr.cpp:1138:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/jsstr.cpp:1139:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/vm/Runtime.cpp:857:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/src/vm/Runtime.cpp:859:7 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/xpconnect/src/XPCConvert.cpp:497:5 [-Wimplicit-fallthrough] unannotated fall-through between switch labels js/xpconnect/src/XPCShellImpl.cpp:966:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels

Comment on attachment 8703395 [details] [diff] [review] js_MOZ_FALLTHROUGH.patch Review of attachment 8703395 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/frontend/NameFunctions.cpp @@ -730,2 @@ > case PNK_IMPORT_SPEC_LIST: { > - case PNK_EXPORT_SPEC_LIST: btw, clang issues a -Wimplicit-fallthrough warning here, apparently interpreting the left brace in `case PNK_IMPORT_SPEC_LIST: {` as code needing a fallthrough annotation. I moved both case labels outside the brace.

Comment on attachment 8703395 [details] [diff] [review] js_MOZ_FALLTHROUGH.patch Review of attachment 8703395 [details] [diff] [review]: ----------------------------------------------------------------- Great job! ::: js/src/asmjs/WasmStubs.cpp @@ +262,5 @@ > MOZ_CRASH("no int64 in asm.js"); > case ExprType::F32: > masm.convertFloat32ToDouble(ReturnFloat32Reg, ReturnDoubleReg); > // Fall through as ReturnDoubleReg now contains a Double > + MOZ_FALLTHROUGH; Can you delete the preceding line and append to the MOZ_FALLTHROUGH; line "// because ReturnDoubleReg now contains a Double"? ::: js/xpconnect/src/XPCConvert.cpp @@ +492,5 @@ > (**((nsAString**)d)).SetIsVoid(true); > return true; > } > // Fall through to T_DOMSTRING case. > + MOZ_FALLTHROUGH; I bet you can remove the comment now, since it's redundant.

Thanks. I'll update those comments before landing.