path: root/builtin
diff options
authorJeff King <>2017-09-21 06:25:41 (GMT)
committerJunio C Hamano <>2017-09-22 03:49:57 (GMT)
commit1cf01a34eaccd6da613dba82291666db237916ab (patch)
tree53db8e7f6b3d291fe65c3ebeac3541877426880c /builtin
parentd0e9983980a25e0c398cc03342e5ad22ef85b8a8 (diff)
consistently use "fallthrough" comments in switches
Gcc 7 adds -Wimplicit-fallthrough, which can warn when a switch case falls through to the next case. The general idea is that the compiler can't tell if this was intentional or not, so you should annotate any intentional fall-throughs as such, leaving it to complain about any unannotated ones. There's a GNU __attribute__ which can be used for annotation, but of course we'd have to #ifdef it away on non-gcc compilers. Gcc will also recognize specially-formatted comments, which matches our current practice. Let's extend that practice to all of the unannotated sites (which I did look over and verify that they were behaving as intended). Ideally in each case we'd actually give some reasons in the comment about why we're falling through, or what we're falling through to. And gcc does support that with -Wimplicit-fallthrough=2, which relaxes the comment pattern matching to anything that contains "fallthrough" (or a variety of spelling variants). However, this isn't the default for -Wimplicit-fallthrough, nor for -Wextra. In the name of simplicity, it's probably better for us to support the default level, which requires "fallthrough" to be the only thing in the comment (modulo some window dressing like "else" and some punctuation; see the gcc manual for the complete set of patterns). This patch suppresses all warnings due to -Wimplicit-fallthrough. We might eventually want to add that to the DEVELOPER Makefile knob, but we should probably wait until gcc 7 is more widely adopted (since earlier versions will complain about the unknown warning type). Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
Diffstat (limited to 'builtin')
4 files changed, 4 insertions, 1 deletions
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 4ccbfaa..aee280e 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -113,6 +113,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,
if (textconv_object(path, obj_context.mode, &oid, 1, &buf, &size))
+ /* else fallthrough */
case 'p':
type = sha1_object_info(oid.hash, NULL);
diff --git a/builtin/checkout.c b/builtin/checkout.c
index 5c202b7..d091f06 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -436,6 +436,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,
* update paths in the work tree, and we cannot revert
* them.
+ /* fallthrough */
case 0:
return 0;
diff --git a/builtin/remote-ext.c b/builtin/remote-ext.c
index bfb21ba..6a9127a 100644
--- a/builtin/remote-ext.c
+++ b/builtin/remote-ext.c
@@ -57,7 +57,7 @@ static char *strip_escapes(const char *str, const char *service,
special = str[rpos];
if (rpos == 1)
- /* Fall-through to error. */
+ /* fallthrough */
die("Bad remote-ext placeholder '%%%c'.",
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 818fe74..f6a5e1a 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1189,6 +1189,7 @@ static int push_check(int argc, const char **argv, const char *prefix)
die("HEAD does not match the named branch in the superproject");
+ /* fallthrough */
die("src refspec '%s' must name a ref",