{"thread":{"id":"47670","subject":"[PATCH] Fixes compile warning with -Wimplicit-fallthrough CFLAGS","startedAt":"2018-01-22T23:51:31Z","lastAt":"2018-01-23T18:37:37Z","messageCount":7,"participants":["Elia Pinto","Jeff King","Eric Sunshine","Jacob Keller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"337125","messageId":"1516665078-107974-1-git-send-email-gitter.spiros@gmail.com","threadId":"47670","inReplyTo":null,"subject":"[PATCH] Fixes compile warning with -Wimplicit-fallthrough CFLAGS","fromName":"Elia Pinto","fromEmail":"gitter.spiros@gmail.com","sentAt":"2018-01-22T23:51:18Z","receivedAt":"2018-01-22T23:51:31Z","isPatch":true,"sender":{"key":"gitter.spiros@gmail.com","avatar":"https://avatars.githubusercontent.com/u/158490?v=4"},"body":"This patch add explicit fallthrough compiler attribute\nwhen needed on switch case statement eliminating\nthe compile warning [-Werror=implicit-fallthrough=].\nIt does this by means of a macro that takes into account\nthe versions of the compilers that include that attribute.\n\nThe fallthrough (or clang::fallthrough) attribute is used to annotate\nintentional fall-through between switch labels.\nTraditionally these are marked with a specific comment, but\nthis attribute is meant to replace comments with a more strict\nannotation, which can be checked by the compiler (gcc-7 or clang).\nThe flags in question were introduced in gcc 7 and are also enabled\nwith -Wextra.\n\nIt would also have been possible to introduce a specific comment\naccepted by gcc 7 instead of the fallthrough attribute for this warning,\nbut clang does not have a similar implementation. The macro replaces\nthe previous, not uniform, comments and can acts as a comment itself.\n\nSigned-off-by: Elia Pinto <gitter.spiros@gmail.com>\n---\n apply.c                     |  6 +++---\n builtin/blame.c             |  2 +-\n builtin/cat-file.c          |  2 +-\n builtin/checkout.c          |  2 +-\n builtin/fast-export.c       |  4 ++--\n builtin/fetch.c             |  2 +-\n builtin/init-db.c           |  2 +-\n builtin/log.c               |  4 ++--\n builtin/ls-files.c          |  2 +-\n builtin/remote-ext.c        |  2 +-\n builtin/reset.c             |  2 +-\n builtin/submodule--helper.c |  2 +-\n config.c                    |  4 ++--\n connect.c                   |  6 +++---\n convert.c                   |  4 ++--\n diff.c                      |  2 +-\n dir.c                       |  2 +-\n fsck.c                      |  2 +-\n git-compat-util.h           | 17 +++++++++++++++++\n ll-merge.c                  |  2 +-\n mailinfo.c                  |  2 +-\n notes.c                     |  4 ++--\n pretty.c                    |  2 +-\n quote.c                     |  4 ++--\n read-cache.c                |  4 ++--\n refs/files-backend.c        |  2 +-\n revision.c                  |  2 +-\n send-pack.c                 |  2 +-\n strbuf.c                    |  2 +-\n streaming.c                 |  2 +-\n upload-pack.c               |  3 +--\n wildmatch.c                 |  2 +-\n wt-status.c                 |  4 ++--\n 33 files changed, 61 insertions(+), 45 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 321a9fa68..a22fb2881 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -1450,7 +1450,7 @@ static void recount_diff(const char *line, int size, struct fragment *fragment)\n \t\tswitch (*line) {\n \t\tcase ' ': case '\\n':\n \t\t\tnewlines++;\n-\t\t\t/* fall through */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase '-':\n \t\t\toldlines++;\n \t\t\tcontinue;\n@@ -2896,7 +2896,7 @@ static int apply_one_fragment(struct apply_state *state,\n \t\t\tif (plen && (ws_rule & WS_BLANK_AT_EOF) &&\n \t\t\t    ws_blank_line(patch + 1, plen, ws_rule))\n \t\t\t\tis_blank_context = 1;\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase '-':\n \t\t\tmemcpy(old, patch + 1, plen);\n \t\t\tadd_line_info(&preimage, old, plen,\n@@ -2904,7 +2904,7 @@ static int apply_one_fragment(struct apply_state *state,\n \t\t\told += plen;\n \t\t\tif (first == '-')\n \t\t\t\tbreak;\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase '+':\n \t\t\t/* --no-add does not add new lines */\n \t\t\tif (first == '+' && state->no_add)\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex 005f55aaa..acc2c1e93 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -834,7 +834,7 @@ int cmd_blame(int argc, const char **argv, const char *prefix)\n \t\t\targv[1] = argv[3];\n \t\t\targv[3] = argv[2];\n \t\t\targv[2] = \"--\";\n-\t\t\t/* FALLTHROUGH */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase 1: /* (1a) */\n \t\t\tpath = add_prefix(prefix, argv[--argc]);\n \t\t\targv[argc] = NULL;\ndiff --git a/builtin/cat-file.c b/builtin/cat-file.c\nindex cf9ea5c79..eb4fa9247 100644\n--- a/builtin/cat-file.c\n+++ b/builtin/cat-file.c\n@@ -113,7 +113,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name,\n \n \t\tif (textconv_object(path, obj_context.mode, &oid, 1, &buf, &size))\n \t\t\tbreak;\n-\t\t/* else fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \n \tcase 'p':\n \t\ttype = sha1_object_info(oid.hash, NULL);\ndiff --git a/builtin/checkout.c b/builtin/checkout.c\nindex c54c78df5..2db0e16d1 100644\n--- a/builtin/checkout.c\n+++ b/builtin/checkout.c\n@@ -441,7 +441,7 @@ static int reset_tree(struct tree *tree, const struct checkout_opts *o,\n \t\t * update paths in the work tree, and we cannot revert\n \t\t * them.\n \t\t */\n-\t\t/* fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \tcase 0:\n \t\treturn 0;\n \tdefault:\ndiff --git a/builtin/fast-export.c b/builtin/fast-export.c\nindex 796d0cd66..972f0a3f0 100644\n--- a/builtin/fast-export.c\n+++ b/builtin/fast-export.c\n@@ -385,7 +385,7 @@ static void show_filemodify(struct diff_queue_struct *q,\n \t\t\t\t    ospec->mode == spec->mode)\n \t\t\t\t\tbreak;\n \t\t\t}\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n \t\tcase DIFF_STATUS_MODIFIED:\n@@ -733,7 +733,7 @@ static void handle_tag(const char *name, struct tag *tag)\n \t\t\tcase WARN_STRIP:\n \t\t\t\twarning (\"Stripping signature from tag %s\",\n \t\t\t\t\t oid_to_hex(&tag->object.oid));\n-\t\t\t\t/* fallthru */\n+\t\t\t\tGIT_FALLTHROUGH;\n \t\t\tcase STRIP:\n \t\t\t\tmessage_size = signature + 1 - message;\n \t\t\t\tbreak;\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex 8ee998ea2..320fd54de 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -848,7 +848,7 @@ static int store_updated_refs(const char *raw_url, const char *remote_name,\n \t\t\tswitch (rm->fetch_head_status) {\n \t\t\tcase FETCH_HEAD_NOT_FOR_MERGE:\n \t\t\t\tmerge_status_marker = \"not-for-merge\";\n-\t\t\t\t/* fall-through */\n+\t\t\t\tGIT_FALLTHROUGH;\n \t\t\tcase FETCH_HEAD_MERGE:\n \t\t\t\tfprintf(fp, \"%s\\t%s\\t%s\",\n \t\t\t\t\toid_to_hex(&rm->old_oid),\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex c9b7946ba..6554172da 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -511,7 +511,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tbreak;\n \t\t\t\tcase SCLD_EXISTS:\n \t\t\t\t\terrno = EEXIST;\n-\t\t\t\t\t/* fallthru */\n+\t\t\t\t\tGIT_FALLTHROUGH;\n \t\t\t\tdefault:\n \t\t\t\t\tdie_errno(_(\"cannot mkdir %s\"), argv[0]);\n \t\t\t\t\tbreak;\ndiff --git a/builtin/log.c b/builtin/log.c\nindex 46b4ca13e..ae6f2257a 100644\n--- a/builtin/log.c\n+++ b/builtin/log.c\n@@ -1902,10 +1902,10 @@ int cmd_cherry(int argc, const char **argv, const char *prefix)\n \tswitch (argc) {\n \tcase 3:\n \t\tlimit = argv[2];\n-\t\t/* FALLTHROUGH */\n+\t\tGIT_FALLTHROUGH;\n \tcase 2:\n \t\thead = argv[1];\n-\t\t/* FALLTHROUGH */\n+\t\tGIT_FALLTHROUGH;\n \tcase 1:\n \t\tupstream = argv[0];\n \t\tbreak;\ndiff --git a/builtin/ls-files.c b/builtin/ls-files.c\nindex 2fc836e33..57aac6024 100644\n--- a/builtin/ls-files.c\n+++ b/builtin/ls-files.c\n@@ -448,7 +448,7 @@ void overlay_tree_on_index(struct index_state *istate,\n \t\tswitch (ce_stage(ce)) {\n \t\tcase 0:\n \t\t\tlast_stage0 = ce;\n-\t\t\t/* fallthru */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tcontinue;\n \t\tcase 1:\ndiff --git a/builtin/remote-ext.c b/builtin/remote-ext.c\nindex 6a9127a33..909f8995e 100644\n--- a/builtin/remote-ext.c\n+++ b/builtin/remote-ext.c\n@@ -57,7 +57,7 @@ static char *strip_escapes(const char *str, const char *service,\n \t\t\t\tspecial = str[rpos];\n \t\t\t\tif (rpos == 1)\n \t\t\t\t\tbreak;\n-\t\t\t\t/* fallthrough */\n+\t\t\t\tGIT_FALLTHROUGH;\n \t\t\tdefault:\n \t\t\t\tdie(\"Bad remote-ext placeholder '%%%c'.\",\n \t\t\t\t\tstr[rpos]);\ndiff --git a/builtin/reset.c b/builtin/reset.c\nindex e15f59579..00e21958f 100644\n--- a/builtin/reset.c\n+++ b/builtin/reset.c\n@@ -65,7 +65,7 @@ static int reset_index(const struct object_id *oid, int reset_type, int quiet)\n \t\tbreak;\n \tcase HARD:\n \t\topts.update = 1;\n-\t\t/* fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \tdefault:\n \t\topts.reset = 1;\n \t}\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a5c4a8a69..bc2fb2ebd 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1425,7 +1425,7 @@ static int push_check(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\tbreak;\n \t\t\t\t\tdie(\"HEAD does not match the named branch in the superproject\");\n \t\t\t\t}\n-\t\t\t\t/* fallthrough */\n+\t\t\t\tGIT_FALLTHROUGH;\n \t\t\tdefault:\n \t\t\t\tdie(\"src refspec '%s' must name a ref\",\n \t\t\t\t    rs->src);\ndiff --git a/config.c b/config.c\nindex 70ce6e704..6f89f3925 100644\n--- a/config.c\n+++ b/config.c\n@@ -2286,7 +2286,7 @@ static int store_aux(const char *key, const char *value, void *cb)\n \t\tALLOC_GROW(store.offset, store.seen + 1,\n \t\t\t   store.offset_alloc);\n \t\tstore.offset[store.seen] = cf->do_ftell(cf);\n-\t\t/* fallthru */\n+\t\tGIT_FALLTHROUGH;\n \tcase SECTION_END_SEEN:\n \tcase START:\n \t\tif (matches(key, value)) {\n@@ -2387,7 +2387,7 @@ static ssize_t write_pair(int fd, const char *key, const char *value)\n \t\tcase '\"':\n \t\tcase '\\\\':\n \t\t\tstrbuf_addch(&sb, '\\\\');\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tstrbuf_addch(&sb, value[i]);\n \t\t\tbreak;\ndiff --git a/connect.c b/connect.c\nindex c3a014c5b..3b4a30520 100644\n--- a/connect.c\n+++ b/connect.c\n@@ -251,7 +251,7 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tstate = EXPECTING_FIRST_REF;\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase EXPECTING_FIRST_REF:\n \t\t\tprocess_capabilities(&len);\n \t\t\tif (process_dummy_ref()) {\n@@ -259,12 +259,12 @@ struct ref **get_remote_heads(int in, char *src_buf, size_t src_len,\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tstate = EXPECTING_REF;\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase EXPECTING_REF:\n \t\t\tif (process_ref(len, &list, flags, extra_have))\n \t\t\t\tbreak;\n \t\t\tstate = EXPECTING_SHALLOW;\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase EXPECTING_SHALLOW:\n \t\t\tif (process_shallow(len, shallow_points))\n \t\t\t\tbreak;\ndiff --git a/convert.c b/convert.c\nindex 1a41a48e1..81c2098a4 100644\n--- a/convert.c\n+++ b/convert.c\n@@ -72,7 +72,7 @@ static void gather_stats(const char *buf, unsigned long size, struct text_stat *\n \t\t\t\tbreak;\n \t\t\tcase 0:\n \t\t\t\tstats->nul++;\n-\t\t\t\t/* fall through */\n+\t\t\t\tGIT_FALLTHROUGH;\n \t\t\tdefault:\n \t\t\t\tstats->nonprintable++;\n \t\t\t}\n@@ -1554,7 +1554,7 @@ static int ident_filter_fn(struct stream_filter *filter,\n \t\tswitch (ident->state) {\n \t\tdefault:\n \t\t\tstrbuf_add(&ident->left, head, ident->state);\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase IDENT_SKIPPING:\n \t\t\t/* fallthrough */\n \t\tcase IDENT_DRAINING:\ndiff --git a/diff.c b/diff.c\nindex 0a9a0cdf1..b632d6fde 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -3869,7 +3869,7 @@ static void fill_metainfo(struct strbuf *msg,\n \t\t\t\t    set, similarity_index(p), reset);\n \t\t\tbreak;\n \t\t}\n-\t\t/* fallthru */\n+\t\tGIT_FALLTHROUGH;\n \tdefault:\n \t\t*must_show_header = 0;\n \t}\ndiff --git a/dir.c b/dir.c\nindex a605e0169..146b40c96 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -700,7 +700,7 @@ static void trim_trailing_spaces(char *buf)\n \t\t\tp++;\n \t\t\tif (!*p)\n \t\t\t\treturn;\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tlast_space = NULL;\n \t\t}\ndiff --git a/fsck.c b/fsck.c\nindex 032699e9a..3b2d07cbb 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -588,7 +588,7 @@ static int fsck_tree(struct tree *item, struct fsck_options *options)\n \t\tcase S_IFREG | 0664:\n \t\t\tif (!options->strict)\n \t\t\t\tbreak;\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\thas_bad_modes = 1;\n \t\t}\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 68b2ad531..158f5fffb 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -1214,4 +1214,21 @@ extern void unleak_memory(const void *ptr, size_t len);\n #define UNLEAK(var) do {} while (0)\n #endif\n \n+/*\n+ *  Define a generic macro for suppress implicit case fall-through \n+ *  warning for different versions of C/C++ compiler.\n+ *\n+ */\n+#if defined(__cplusplus) && __cplusplus >= 201703L\n+#define GIT_FALLTHROUGH [[fallthrough]]\n+#elif defined(__cplusplus) && __cplusplus >= 201103L && defined(__clang__)\n+#define GIT_FALLTHROUGH [[clang::fallthrough]]\n+#elif defined(__cplusplus) && __cplusplus >= 201103L && __GNUC__ >= 7\n+#define GIT_FALLTHROUGH [[gnu::fallthrough]]\n+#elif  __GNUC__ >= 7 // gcc 7\n+#define GIT_FALLTHROUGH __attribute__ ((fallthrough))\n+#else // C++11 on gcc 6, and all other cases\n+#define GIT_FALLTHROUGH\n+#endif\n+\n #endif\ndiff --git a/ll-merge.c b/ll-merge.c\nindex a6ad2ec12..be7a21975 100644\n--- a/ll-merge.c\n+++ b/ll-merge.c\n@@ -59,7 +59,7 @@ static int ll_binary_merge(const struct ll_merge_driver *drv_unused,\n \t\tdefault:\n \t\t\twarning(\"Cannot merge binary files: %s (%s vs. %s)\",\n \t\t\t\tpath, name1, name2);\n-\t\t\t/* fallthru */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase XDL_MERGE_FAVOR_OURS:\n \t\t\tstolen = src1;\n \t\t\tbreak;\ndiff --git a/mailinfo.c b/mailinfo.c\nindex a89db22ab..4e4fd31c5 100644\n--- a/mailinfo.c\n+++ b/mailinfo.c\n@@ -827,7 +827,7 @@ static void handle_filter(struct mailinfo *mi, struct strbuf *line)\n \t\tif (!handle_commit_msg(mi, line))\n \t\t\tbreak;\n \t\tmi->filter_stage++;\n-\t\t/* fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \tcase 1:\n \t\thandle_patch(mi, line);\n \t\tbreak;\ndiff --git a/notes.c b/notes.c\nindex c7f21fae4..7e4b73086 100644\n--- a/notes.c\n+++ b/notes.c\n@@ -128,7 +128,7 @@ static void **note_tree_search(struct notes_tree *t, struct int_node **tree,\n \t\t\tfree(l);\n \t\t\treturn note_tree_search(t, tree, n, key_sha1);\n \t\t}\n-\t\t/* fall through */\n+\t\tGIT_FALLTHROUGH;\n \tdefault:\n \t\treturn &((*tree)->a[i]);\n \t}\n@@ -326,7 +326,7 @@ static void note_tree_free(struct int_node *tree)\n \t\tswitch (GET_PTR_TYPE(p)) {\n \t\tcase PTR_TYPE_INTERNAL:\n \t\t\tnote_tree_free(CLR_PTR_TYPE(p));\n-\t\t\t/* fall through */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase PTR_TYPE_NOTE:\n \t\tcase PTR_TYPE_SUBTREE:\n \t\t\tfree(CLR_PTR_TYPE(p));\ndiff --git a/pretty.c b/pretty.c\nindex f7ce49023..c942ce8dd 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -274,7 +274,7 @@ static void add_rfc822_quoted(struct strbuf *out, const char *s, int len)\n \t\tcase '\"':\n \t\tcase '\\\\':\n \t\t\tstrbuf_addch(out, '\\\\');\n-\t\t\t/* fall through */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tstrbuf_addch(out, s[i]);\n \t\t}\ndiff --git a/quote.c b/quote.c\nindex de2922ddd..f1733de19 100644\n--- a/quote.c\n+++ b/quote.c\n@@ -99,7 +99,7 @@ static char *sq_dequote_step(char *arg, char **next)\n \t\t\t\t*dst++ = c;\n \t\t\t\tcontinue;\n \t\t\t}\n-\t\t/* Fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tif (!next || !isspace(*src))\n \t\t\t\treturn NULL;\n@@ -431,7 +431,7 @@ void tcl_quote_buf(struct strbuf *sb, const char *src)\n \t\tcase '{': case '}':\n \t\tcase '$': case '\\\\': case '\"':\n \t\t\tstrbuf_addch(sb, '\\\\');\n-\t\t\t/* fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tstrbuf_addch(sb, c);\n \t\t\tbreak;\ndiff --git a/read-cache.c b/read-cache.c\nindex 2eb81a66b..abe655024 100644\n--- a/read-cache.c\n+++ b/read-cache.c\n@@ -225,7 +225,7 @@ static int ce_modified_check_fs(const struct cache_entry *ce, struct stat *st)\n \tcase S_IFDIR:\n \t\tif (S_ISGITLINK(ce->ce_mode))\n \t\t\treturn ce_compare_gitlink(ce) ? DATA_CHANGED : 0;\n-\t\t/* else fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \tdefault:\n \t\treturn TYPE_CHANGED;\n \t}\n@@ -840,7 +840,7 @@ static int verify_dotfile(const char *rest)\n \t\tif (rest[2] != 't' && rest[2] != 'T')\n \t\t\tbreak;\n \t\trest += 2;\n-\t/* fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \tcase '.':\n \t\tif (rest[1] == '\\0' || is_dir_sep(rest[1]))\n \t\t\treturn 0;\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex f75d960e1..d6a2f8129 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -562,7 +562,7 @@ static int lock_raw_ref(struct files_ref_store *refs,\n \t\t/* Maybe another process was tidying up. Try again. */\n \t\tif (--attempts_remaining > 0)\n \t\t\tgoto retry;\n-\t\t/* fall through */\n+\t\tGIT_FALLTHROUGH;\n \tdefault:\n \t\tstrbuf_addf(err, \"unable to create directory for %s\",\n \t\t\t    ref_file.buf);\ndiff --git a/revision.c b/revision.c\nindex 236d0d579..e34c2dfe1 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -702,7 +702,7 @@ static void try_to_simplify_commit(struct rev_info *revs, struct commit *commit)\n \t\t\t\t\t    oid_to_hex(&p->object.oid));\n \t\t\t\tp->parents = NULL;\n \t\t\t}\n-\t\t/* fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \t\tcase REV_TREE_OLD:\n \t\tcase REV_TREE_DIFFERENT:\n \t\t\tif (relevant_commit(p))\ndiff --git a/send-pack.c b/send-pack.c\nindex 2112d3b27..58f1dcbea 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -487,7 +487,7 @@ int send_pack(struct send_pack_args *args,\n \t\t\t\tstrbuf_release(&cap_buf);\n \t\t\t\treturn atomic_push_failure(args, remote_refs, ref);\n \t\t\t}\n-\t\t\t/* else fallthrough */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tcontinue;\n \t\t}\ndiff --git a/strbuf.c b/strbuf.c\nindex 1df674e91..281f6e254 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -838,7 +838,7 @@ void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm,\n \t\t\t\tfmt++;\n \t\t\t\tbreak;\n \t\t\t}\n-\t\t\t/* FALLTHROUGH */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tstrbuf_addch(&munged_fmt, '%');\n \t\t}\ndiff --git a/streaming.c b/streaming.c\nindex 5892b50bd..ac46c9079 100644\n--- a/streaming.c\n+++ b/streaming.c\n@@ -124,7 +124,7 @@ static enum input_source istream_source(const unsigned char *sha1,\n \tcase OI_PACKED:\n \t\tif (!oi->u.packed.is_delta && big_file_threshold < size)\n \t\t\treturn pack_non_delta;\n-\t\t/* fallthru */\n+\t\tGIT_FALLTHROUGH;\n \tdefault:\n \t\treturn incore;\n \t}\ndiff --git a/upload-pack.c b/upload-pack.c\nindex f51b6cfca..3ff2a0f06 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -1105,8 +1105,7 @@ int cmd_main(int argc, const char **argv)\n \t\t */\n \t\tif (advertise_refs || !stateless_rpc)\n \t\t\tpacket_write_fmt(1, \"version 1\\n\");\n-\n-\t\t/* fallthrough */\n+\t\tGIT_FALLTHROUGH;\n \tcase protocol_v0:\n \t\tupload_pack();\n \t\tbreak;\ndiff --git a/wildmatch.c b/wildmatch.c\nindex d074c1be1..594f7585c 100644\n--- a/wildmatch.c\n+++ b/wildmatch.c\n@@ -71,7 +71,7 @@ static int dowild(const uchar *p, const uchar *text, unsigned int flags)\n \t\t\t/* Literal match with following character.  Note that the test\n \t\t\t * in \"default\" handles the p[1] == '\\0' failure case. */\n \t\t\tp_ch = *++p;\n-\t\t\t/* FALLTHROUGH */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tdefault:\n \t\t\tif (t_ch != p_ch)\n \t\t\t\treturn WM_NOMATCH;\ndiff --git a/wt-status.c b/wt-status.c\nindex f5debcd2b..f35452119 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -473,7 +473,7 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\td->rename_source = xstrdup(p->one->path);\n \t\t\td->rename_score = p->score * 100 / MAX_SCORE;\n \t\t\td->rename_status = p->status;\n-\t\t\t/* fallthru */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase DIFF_STATUS_MODIFIED:\n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n \t\tcase DIFF_STATUS_UNMERGED:\n@@ -550,7 +550,7 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \t\t\td->rename_source = xstrdup(p->one->path);\n \t\t\td->rename_score = p->score * 100 / MAX_SCORE;\n \t\t\td->rename_status = p->status;\n-\t\t\t/* fallthru */\n+\t\t\tGIT_FALLTHROUGH;\n \t\tcase DIFF_STATUS_MODIFIED:\n \t\tcase DIFF_STATUS_TYPE_CHANGED:\n \t\t\td->mode_head = p->one->mode;\n-- \n2.16.0.312.g896df04.dirty\n\n"},{"id":"337129","messageId":"20180123000727.GB26357@sigill.intra.peff.net","threadId":"47670","inReplyTo":"1516665078-107974-1-git-send-email-gitter.spiros@gmail.com","subject":"Re: [PATCH] Fixes compile warning with -Wimplicit-fallthrough CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-23T00:07:28Z","receivedAt":"2018-01-23T00:07:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 22, 2018 at 11:51:18PM +0000, Elia Pinto wrote:\n\n> This patch add explicit fallthrough compiler attribute\n> when needed on switch case statement eliminating\n> the compile warning [-Werror=implicit-fallthrough=].\n> It does this by means of a macro that takes into account\n> the versions of the compilers that include that attribute.\n> \n> The fallthrough (or clang::fallthrough) attribute is used to annotate\n> intentional fall-through between switch labels.\n> Traditionally these are marked with a specific comment, but\n> this attribute is meant to replace comments with a more strict\n> annotation, which can be checked by the compiler (gcc-7 or clang).\n> The flags in question were introduced in gcc 7 and are also enabled\n> with -Wextra.\n\nHrm. Your subject says \"fixes compile warnings\", but don't we already\ncompile cleanly with -Wimplicit-fallthrough after my 1cf01a34ea\n(consistently use \"fallthrough\" comments in switches, 2017-09-21)?\n\nCertainly the tip of \"master\" seems to pass for me on both gcc 7 and\nclang 4. You can pump the warning up to level 5 on gcc to insist on the\nattribute, but I think the comments are more readable (and it is not\nlike we have a problem with false positive comments).\n\n> It would also have been possible to introduce a specific comment\n> accepted by gcc 7 instead of the fallthrough attribute for this warning,\n> but clang does not have a similar implementation. The macro replaces\n> the previous, not uniform, comments and can acts as a comment itself.\n\nInterestingly clang seems to accept -Wimplicit-fallthrough, but I could\nnot get it to actually trigger a warning, even after removing some of\nthe existing comments.\n\nWhat version of clang are you using? I'm certainly puzzled by the\nbehavior I'm seeing.\n\n> diff --git a/apply.c b/apply.c\n> index 321a9fa68..a22fb2881 100644\n> --- a/apply.c\n> +++ b/apply.c\n> @@ -1450,7 +1450,7 @@ static void recount_diff(const char *line, int size, struct fragment *fragment)\n>  \t\tswitch (*line) {\n>  \t\tcase ' ': case '\\n':\n>  \t\t\tnewlines++;\n> -\t\t\t/* fall through */\n> +\t\t\tGIT_FALLTHROUGH;\n\nUgh, the semi-colon there makes it look like it's actual code. If we go\nthis route, I wonder if it's worth hiding it inside the macro.\n\n-Peff\n"},{"id":"337139","messageId":"CAPig+cSuBRcM7BBw1MKOOVSZv+CVVrY-dWfSGj+31DA4QMrruA@mail.gmail.com","threadId":"47670","inReplyTo":"1516665078-107974-1-git-send-email-gitter.spiros@gmail.com","subject":"Re: [PATCH] Fixes compile warning with -Wimplicit-fallthrough CFLAGS","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-01-23T00:54:18Z","receivedAt":"2018-01-23T00:54:23Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Jan 22, 2018 at 6:51 PM, Elia Pinto <gitter.spiros@gmail.com> wrote:\n> This patch add explicit fallthrough compiler attribute\n> when needed on switch case statement eliminating\n> the compile warning [-Werror=implicit-fallthrough=].\n> It does this by means of a macro that takes into account\n> the versions of the compilers that include that attribute.\n> [...]\n> Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>\n> ---\n> diff --git a/convert.c b/convert.c\n> @@ -1554,7 +1554,7 @@ static int ident_filter_fn(struct stream_filter *filter,\n>                 switch (ident->state) {\n>                 default:\n>                         strbuf_add(&ident->left, head, ident->state);\n> -                       /* fallthrough */\n> +                       GIT_FALLTHROUGH;\n>                 case IDENT_SKIPPING:\n>                         /* fallthrough */\n\nWhy doesn't this /* fallthrough */ deserve the same treatment?\n\n>                 case IDENT_DRAINING:\n"},{"id":"337140","messageId":"20180123005950.GH26357@sigill.intra.peff.net","threadId":"47670","inReplyTo":"CAPig+cSuBRcM7BBw1MKOOVSZv+CVVrY-dWfSGj+31DA4QMrruA@mail.gmail.com","subject":"Re: [PATCH] Fixes compile warning with -Wimplicit-fallthrough CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-23T00:59:50Z","receivedAt":"2018-01-23T00:59:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jan 22, 2018 at 07:54:18PM -0500, Eric Sunshine wrote:\n\n> On Mon, Jan 22, 2018 at 6:51 PM, Elia Pinto <gitter.spiros@gmail.com> wrote:\n> > This patch add explicit fallthrough compiler attribute\n> > when needed on switch case statement eliminating\n> > the compile warning [-Werror=implicit-fallthrough=].\n> > It does this by means of a macro that takes into account\n> > the versions of the compilers that include that attribute.\n> > [...]\n> > Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>\n> > ---\n> > diff --git a/convert.c b/convert.c\n> > @@ -1554,7 +1554,7 @@ static int ident_filter_fn(struct stream_filter *filter,\n> >                 switch (ident->state) {\n> >                 default:\n> >                         strbuf_add(&ident->left, head, ident->state);\n> > -                       /* fallthrough */\n> > +                       GIT_FALLTHROUGH;\n> >                 case IDENT_SKIPPING:\n> >                         /* fallthrough */\n> \n> Why doesn't this /* fallthrough */ deserve the same treatment?\n> \n> >                 case IDENT_DRAINING:\n\nI can't answer that philosophically, but I can tell you why the compiler\ndoes not complain. :)\n\nUsually case arms with no statements between them are exempt from\nfallthrough warnings. So:\n\n  switch (foo)\n  case 1:\n  case 2:\n  case 3:\n     /* do one thing */\n     break;\n  case 4:\n     /* do another thing */\n     break;\n  }\n\ndoes not need any annotations for cases 1 and 2 to fallthrough. Which\nmeans that the original comment was not actually necessary for the\ncompiler, though the original author considered it a useful comment.\n\nSo there you get into philosophy. Should it be converted to a\ncompiler-visible annotation, or is it better left as a comment?\n\n-Peff\n"},{"id":"337146","messageId":"CA+P7+xqfHnsoOq8EPb-4feu8Q6gdRTCeOGRVkSyjNJNtmtiw=Q@mail.gmail.com","threadId":"47670","inReplyTo":"20180123005950.GH26357@sigill.intra.peff.net","subject":"Re: [PATCH] Fixes compile warning with -Wimplicit-fallthrough CFLAGS","fromName":"Jacob Keller","fromEmail":"jacob.keller@gmail.com","sentAt":"2018-01-23T06:31:30Z","receivedAt":"2018-01-23T06:31:56Z","isPatch":true,"sender":{"key":"jacob.keller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/874719?v=4"},"body":"On Mon, Jan 22, 2018 at 4:59 PM, Jeff King <peff@peff.net> wrote:\n> On Mon, Jan 22, 2018 at 07:54:18PM -0500, Eric Sunshine wrote:\n>\n>> On Mon, Jan 22, 2018 at 6:51 PM, Elia Pinto <gitter.spiros@gmail.com> wrote:\n>> > This patch add explicit fallthrough compiler attribute\n>> > when needed on switch case statement eliminating\n>> > the compile warning [-Werror=implicit-fallthrough=].\n>> > It does this by means of a macro that takes into account\n>> > the versions of the compilers that include that attribute.\n>> > [...]\n>> > Signed-off-by: Elia Pinto <gitter.spiros@gmail.com>\n>> > ---\n>> > diff --git a/convert.c b/convert.c\n>> > @@ -1554,7 +1554,7 @@ static int ident_filter_fn(struct stream_filter *filter,\n>> >                 switch (ident->state) {\n>> >                 default:\n>> >                         strbuf_add(&ident->left, head, ident->state);\n>> > -                       /* fallthrough */\n>> > +                       GIT_FALLTHROUGH;\n>> >                 case IDENT_SKIPPING:\n>> >                         /* fallthrough */\n>>\n>> Why doesn't this /* fallthrough */ deserve the same treatment?\n>>\n>> >                 case IDENT_DRAINING:\n>\n> I can't answer that philosophically, but I can tell you why the compiler\n> does not complain. :)\n>\n> Usually case arms with no statements between them are exempt from\n> fallthrough warnings. So:\n>\n>   switch (foo)\n>   case 1:\n>   case 2:\n>   case 3:\n>      /* do one thing */\n>      break;\n>   case 4:\n>      /* do another thing */\n>      break;\n>   }\n>\n> does not need any annotations for cases 1 and 2 to fallthrough. Which\n> means that the original comment was not actually necessary for the\n> compiler, though the original author considered it a useful comment.\n>\n> So there you get into philosophy. Should it be converted to a\n> compiler-visible annotation, or is it better left as a comment?\n>\n> -Peff\n\nI'd personally rather stick to the comment if we can, or use something\nlike \"fallthrough;\" to make it appear like a keyword, instead of an\nall caps macro, since at least to my sensibility, the all caps is a\nbit too crazy.\n\nAlso, I would not put one inside an empty case statement that just\nfalls through to the next branch and does nothing special itself.\n\nThanks,\nJake\n"},{"id":"337169","messageId":"xmqqd120ifvu.fsf@gitster.mtv.corp.google.com","threadId":"47670","inReplyTo":"20180123000727.GB26357@sigill.intra.peff.net","subject":"Re: [PATCH] Fixes compile warning with -Wimplicit-fallthrough CFLAGS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-01-23T18:33:57Z","receivedAt":"2018-01-23T18:34:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n>> diff --git a/apply.c b/apply.c\n>> index 321a9fa68..a22fb2881 100644\n>> --- a/apply.c\n>> +++ b/apply.c\n>> @@ -1450,7 +1450,7 @@ static void recount_diff(const char *line, int size, struct fragment *fragment)\n>>  \t\tswitch (*line) {\n>>  \t\tcase ' ': case '\\n':\n>>  \t\t\tnewlines++;\n>> -\t\t\t/* fall through */\n>> +\t\t\tGIT_FALLTHROUGH;\n>\n> Ugh, the semi-colon there makes it look like it's actual code. If we go\n> this route, I wonder if it's worth hiding it inside the macro.\n\nWhat?  You mean to shout in all caps without even terminating the\nline with any punctuation?  Please don't---I am sure it will break\nauto indentation people rely on from their editors.\n\n"},{"id":"337170","messageId":"20180123183731.GA18333@sigill.intra.peff.net","threadId":"47670","inReplyTo":"xmqqd120ifvu.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Fixes compile warning with -Wimplicit-fallthrough CFLAGS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-01-23T18:37:31Z","receivedAt":"2018-01-23T18:37:37Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jan 23, 2018 at 10:33:57AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> >> diff --git a/apply.c b/apply.c\n> >> index 321a9fa68..a22fb2881 100644\n> >> --- a/apply.c\n> >> +++ b/apply.c\n> >> @@ -1450,7 +1450,7 @@ static void recount_diff(const char *line, int size, struct fragment *fragment)\n> >>  \t\tswitch (*line) {\n> >>  \t\tcase ' ': case '\\n':\n> >>  \t\t\tnewlines++;\n> >> -\t\t\t/* fall through */\n> >> +\t\t\tGIT_FALLTHROUGH;\n> >\n> > Ugh, the semi-colon there makes it look like it's actual code. If we go\n> > this route, I wonder if it's worth hiding it inside the macro.\n> \n> What?  You mean to shout in all caps without even terminating the\n> line with any punctuation?  Please don't---I am sure it will break\n> auto indentation people rely on from their editors.\n\nTrue, that may be even worse. I just wonder if we can do something to\nmake it look more obviously like a non-code attribute. The actual syntax\nis something like:\n\n  [[fallthrough]];\n\nwhich is pretty horrid, but at least a bit easier to see. gcc also\nprovides \"__attribute__((fallthrough))\", but I don't think it works with\nclang.\n\nI vastly prefer the comment approach if we can use it. Apparently clang\ndoesn't support it, but I have also not managed to get clang (either\nversion 4, 6, or the upcoming 7) to actually report anything via\n-Wimplicit-fallthrough, either. Maybe I'm holding it wrong.\n\n-Peff\n"}]}