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 where the fallthrough is intentional. dom/xslt/xpath/txCoreFunctionCall.cpp:214:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels dom/xslt/xpath/txLocationStep.cpp:47:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels dom/xslt/xpath/txLocationStep.cpp:79:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels dom/xslt/xpath/txXPCOMExtensionFunction.cpp:290:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels dom/xslt/xslt/txOutputFormat.cpp:86:9 [-Wimplicit-fallthrough] unannotated fall-through between switch labels

Comment on attachment 8708660 [details] [diff] [review] dom-xslt-MOZ_FALLTHROUGH.patch Review of attachment 8708660 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/xslt/xpath/txCoreFunctionCall.cpp @@ +209,5 @@ > { > break; > } > } > + MOZ_CRASH("Unexpected mType?!"); Can we remove the default case in the switch too? (Technically this could also be a missing return in one of the inner switch non-default cases, but ok). ::: dom/xslt/xpath/txXPCOMExtensionFunction.cpp @@ +285,5 @@ > if (iid.Equals(NS_GET_IID(txIXPathObject))) { > return eOBJECT; > } > } > + MOZ_FALLTHROUGH; Maybe move this up a line, so it's inside the case block like the others.

(In reply to Peter Van der Beken [:peterv] from comment #1) > ::: dom/xslt/xpath/txCoreFunctionCall.cpp > @@ +209,5 @@ > > { > > break; > > } > > } > > + MOZ_CRASH("Unexpected mType?!"); > > Can we remove the default case in the switch too? (Technically this could > also be a missing return in one of the inner switch non-default cases, but > ok). I tried removing the default case, but then clang complains with -Wswitch warnings because the inner switch doesn't cover every txCoreFunctionCall::eType enum value. So this new MOZ_CRASH("Unexpected mType?!") is actually catching two different bugs and should maybe be split into two MOZ_CRASH: 1. MOZ_CRASH("Unexpected mType") in the default case. 2. MOZ_CRASH("Missing return from inner switch non-default cases")