{"thread":{"id":"49650","subject":"[PATCH] khash: silence -Wunused-function","startedAt":"2018-10-23T11:39:16Z","lastAt":"2018-10-23T23:02:39Z","messageCount":9,"participants":["Carlo Marcelo Arenas Belón","Duy Nguyen","René Scharfe","Carlo Arenas","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"361283","messageId":"20181023113416.90958-1-carenas@gmail.com","threadId":"49650","inReplyTo":null,"subject":"[PATCH] khash: silence -Wunused-function","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2018-10-23T11:34:16Z","receivedAt":"2018-10-23T11:39:16Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"after 36da893114 (\"config.mak.dev: enable -Wunused-function\", 2018-10-18)\nmacro generated code should use a similar solution than commit-slab to\nsilence the compiler.\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n khash.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/khash.h b/khash.h\nindex d10caa0c35..39c2833877 100644\n--- a/khash.h\n+++ b/khash.h\n@@ -234,7 +234,7 @@ static const double __ac_HASH_UPPER = 0.77;\n \t__KHASH_IMPL(name, SCOPE, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n \n #define KHASH_INIT(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \\\n-\tKHASH_INIT2(name, static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n+\tKHASH_INIT2(name, __attribute__((__unused__)) static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n \n /* Other convenient macros... */\n \n-- \n2.19.1\n\n"},{"id":"361292","messageId":"CACsJy8AXjhbY9WWjYtMk128TZHLBCf4Ye4fe-ryXhoP9j2jtdw@mail.gmail.com","threadId":"49650","inReplyTo":"20181023113416.90958-1-carenas@gmail.com","subject":"Re: [PATCH] khash: silence -Wunused-function","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-10-23T15:44:41Z","receivedAt":"2018-10-23T15:45:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Oct 23, 2018 at 1:42 PM Carlo Marcelo Arenas Belón\n<carenas@gmail.com> wrote:\n>\n> after 36da893114 (\"config.mak.dev: enable -Wunused-function\", 2018-10-18)\n> macro generated code should use a similar solution than commit-slab to\n> silence the compiler.\n>\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>  khash.h | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/khash.h b/khash.h\n> index d10caa0c35..39c2833877 100644\n> --- a/khash.h\n> +++ b/khash.h\n> @@ -234,7 +234,7 @@ static const double __ac_HASH_UPPER = 0.77;\n>         __KHASH_IMPL(name, SCOPE, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n>\n>  #define KHASH_INIT(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \\\n> -       KHASH_INIT2(name, static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n> +       KHASH_INIT2(name, __attribute__((__unused__)) static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n\nMaybe move MAYBE_UNUSED from commit-slab-impl.h to git-compat-util.h\nand use it here so we have an easier time if we have to use a\ndifferent way to mark unused functions on some exotic platform.\n\n>\n>  /* Other convenient macros... */\n>\n> --\n> 2.19.1\n>\n\n\n-- \nDuy\n"},{"id":"361296","messageId":"2518b1ff-d16d-da95-fdc2-698dddb61a36@web.de","threadId":"49650","inReplyTo":"20181023113416.90958-1-carenas@gmail.com","subject":"Re: [PATCH] khash: silence -Wunused-function","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2018-10-23T16:19:54Z","receivedAt":"2018-10-23T16:20:03Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 23.10.2018 um 13:34 schrieb Carlo Marcelo Arenas Belón:\n> after 36da893114 (\"config.mak.dev: enable -Wunused-function\", 2018-10-18)\n> macro generated code should use a similar solution than commit-slab to\n> silence the compiler.\n\nWith Clang 6 and GCC 8 on Debian I don't get any warnings on master or\n36da893114.\n\nWith Clang 6 on OpenBSD I get warnings about the unused khash functions\nin delta-islands.c on master, though.\n\nWhat do you get, or in other words: why did you send this patch?\n\n> Signed-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n> ---\n>  khash.h | 2 +-\n>  1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/khash.h b/khash.h\n> index d10caa0c35..39c2833877 100644\n> --- a/khash.h\n> +++ b/khash.h\n> @@ -234,7 +234,7 @@ static const double __ac_HASH_UPPER = 0.77;\n>  \t__KHASH_IMPL(name, SCOPE, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n>  \n>  #define KHASH_INIT(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \\\n> -\tKHASH_INIT2(name, static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n> +\tKHASH_INIT2(name, __attribute__((__unused__)) static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n>  \n>  /* Other convenient macros... */\n>  \n\nThis fixes the warning on OpenBSD for me.  So does moving the KHASH_INIT\nline from delta-islands.c to a header file (e.g. khash.h or\ndelta-islands.h).\n\nRené\n"},{"id":"361299","messageId":"CAPUEspho2MB=AruF565XaCPL7e+RmfT9iJtbzDAMfFpAkrzoNg@mail.gmail.com","threadId":"49650","inReplyTo":"2518b1ff-d16d-da95-fdc2-698dddb61a36@web.de","subject":"Re: [PATCH] khash: silence -Wunused-function","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2018-10-23T16:52:08Z","receivedAt":"2018-10-23T16:52:25Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Tue, Oct 23, 2018 at 9:19 AM René Scharfe <l.s.r@web.de> wrote:\n\n> With Clang 6 and GCC 8 on Debian I don't get any warnings on master or\n> 36da893114.\n\nI see it with Clang 7 on Fedora (at least Rawhide but suspect also to\naffect the next release, now in beta: 29)\n\n> With Clang 6 on OpenBSD I get warnings about the unused khash functions\n> in delta-islands.c on master, though.\n\nApple LLVM 9 and 10 will also trigger similar warnings to what you see\nin OpenBSD and as Junio mentioned recently in a different thread about\nsemantic patches, this currently affects the CI for \"Apple\":\n\n  https://travis-ci.org/git/git/builds/444969342\n\nCarlo\n"},{"id":"361329","messageId":"20181023215020.18550-1-carenas@gmail.com","threadId":"49650","inReplyTo":"CACsJy8AXjhbY9WWjYtMk128TZHLBCf4Ye4fe-ryXhoP9j2jtdw@mail.gmail.com","subject":"[PATCH v2 0/2] delta-islands: avoid unused function messages","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2018-10-23T21:50:18Z","receivedAt":"2018-10-23T21:50:46Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"the macro generated code from delta-islands (using khash) triggers\nsome unused function warnings in macOS, OpenBSD and some linux with a\nnewer version of clang\n\nCarlo Marcelo Arenas Belón (2):\n  commit-slabs: move MAYBE_UNUSED out\n  khash: silence -Wunused-function for delta-islands\n\n commit-slab-impl.h | 4 ++--\n git-compat-util.h  | 2 ++\n khash.h            | 2 +-\n 3 files changed, 5 insertions(+), 3 deletions(-)\n\n-- \n2.19.1\n\n"},{"id":"361330","messageId":"20181023215020.18550-2-carenas@gmail.com","threadId":"49650","inReplyTo":"20181023215020.18550-1-carenas@gmail.com","subject":"[PATCH v2 1/2] commit-slabs: move MAYBE_UNUSED out","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2018-10-23T21:50:19Z","receivedAt":"2018-10-23T21:50:46Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"after 36da893114 (\"config.mak.dev: enable -Wunused-function\", 2018-10-18)\nit is expected to be used to prevent -Wunused-function warnings for code\nthat was macro generated\n\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n commit-slab-impl.h | 4 ++--\n git-compat-util.h  | 2 ++\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/commit-slab-impl.h b/commit-slab-impl.h\nindex ac1e6d409a..5c0eb91a5d 100644\n--- a/commit-slab-impl.h\n+++ b/commit-slab-impl.h\n@@ -1,10 +1,10 @@\n #ifndef COMMIT_SLAB_IMPL_H\n #define COMMIT_SLAB_IMPL_H\n \n-#define MAYBE_UNUSED __attribute__((__unused__))\n+#include \"git-compat-util.h\"\n \n #define implement_static_commit_slab(slabname, elemtype) \\\n-\timplement_commit_slab(slabname, elemtype, static MAYBE_UNUSED)\n+\timplement_commit_slab(slabname, elemtype, MAYBE_UNUSED static)\n \n #define implement_shared_commit_slab(slabname, elemtype) \\\n \timplement_commit_slab(slabname, elemtype, )\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 9a64998b24..e4d3967a23 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -408,6 +408,8 @@ static inline char *git_find_last_dir_sep(const char *path)\n #define LAST_ARG_MUST_BE_NULL\n #endif\n \n+#define MAYBE_UNUSED __attribute__((__unused__))\n+\n #include \"compat/bswap.h\"\n \n #include \"wildmatch.h\"\n-- \n2.19.1\n\n"},{"id":"361331","messageId":"20181023215020.18550-3-carenas@gmail.com","threadId":"49650","inReplyTo":"20181023215020.18550-1-carenas@gmail.com","subject":"[PATCH v2 2/2] khash: silence -Wunused-function for delta-islands","fromName":"Carlo Marcelo Arenas Belón","fromEmail":"carenas@gmail.com","sentAt":"2018-10-23T21:50:20Z","receivedAt":"2018-10-23T21:51:01Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"showing the following when compiled with latest clang (OpenBSD, Fedors\nand macOS):\n\ndelta-islands.c:23:1: warning: unused function 'kh_destroy_str'\n      [-Wunused-function]\ndelta-islands.c:23:1: warning: unused function 'kh_clear_str'\n      [-Wunused-function]\ndelta-islands.c:23:1: warning: unused function 'kh_del_str' [-Wunused-function]\n\nReported-by: René Scharfe <l.s.r@web.de>\nSuggested-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>\n---\n khash.h | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/khash.h b/khash.h\nindex c0da40daa7..fb71c92547 100644\n--- a/khash.h\n+++ b/khash.h\n@@ -226,7 +226,7 @@ static const double __ac_HASH_UPPER = 0.77;\n \t__KHASH_IMPL(name, SCOPE, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n \n #define KHASH_INIT(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \\\n-\tKHASH_INIT2(name, static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n+\tKHASH_INIT2(name, MAYBE_UNUSED static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)\n \n /* Other convenient macros... */\n \n-- \n2.19.1\n\n"},{"id":"361334","messageId":"20181023220019.GA31613@sigill.intra.peff.net","threadId":"49650","inReplyTo":"20181023215020.18550-2-carenas@gmail.com","subject":"Re: [PATCH v2 1/2] commit-slabs: move MAYBE_UNUSED out","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-10-23T22:00:19Z","receivedAt":"2018-10-23T22:00:23Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Oct 23, 2018 at 02:50:19PM -0700, Carlo Marcelo Arenas Belón wrote:\n\n> after 36da893114 (\"config.mak.dev: enable -Wunused-function\", 2018-10-18)\n> it is expected to be used to prevent -Wunused-function warnings for code\n> that was macro generated\n\nMakes sense.\n\n> diff --git a/commit-slab-impl.h b/commit-slab-impl.h\n> index ac1e6d409a..5c0eb91a5d 100644\n> --- a/commit-slab-impl.h\n> +++ b/commit-slab-impl.h\n> @@ -1,10 +1,10 @@\n>  #ifndef COMMIT_SLAB_IMPL_H\n>  #define COMMIT_SLAB_IMPL_H\n>  \n> -#define MAYBE_UNUSED __attribute__((__unused__))\n> +#include \"git-compat-util.h\"\n\nWe shouldn't need to include git-compat-util.h, as the C files would\nalready do so, via this rule in CodingGuidelines:\n\n   - The first #include in C files, except in platform specific compat/\n   implementations, must be either \"git-compat-util.h\", \"cache.h\" or\n   \"builtin.h\".  You do not have to include more than one of these.\n\n(and if there is a case that does not, we should fix that).\n\n>  #define implement_static_commit_slab(slabname, elemtype) \\\n> -\timplement_commit_slab(slabname, elemtype, static MAYBE_UNUSED)\n> +\timplement_commit_slab(slabname, elemtype, MAYBE_UNUSED static)\n\nIs this hunk necessary?\n\n> diff --git a/git-compat-util.h b/git-compat-util.h\n> index 9a64998b24..e4d3967a23 100644\n> --- a/git-compat-util.h\n> +++ b/git-compat-util.h\n> @@ -408,6 +408,8 @@ static inline char *git_find_last_dir_sep(const char *path)\n>  #define LAST_ARG_MUST_BE_NULL\n>  #endif\n>  \n> +#define MAYBE_UNUSED __attribute__((__unused__))\n> +\n>  #include \"compat/bswap.h\"\n\nLooks good.\n\n-Peff\n"},{"id":"361339","messageId":"CAPUEspiAhmJ5+qTUcA6x+eSPc9jmOLdF7QsSsSfwJSy5r=06PA@mail.gmail.com","threadId":"49650","inReplyTo":"20181023220019.GA31613@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/2] commit-slabs: move MAYBE_UNUSED out","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2018-10-23T23:02:22Z","receivedAt":"2018-10-23T23:02:39Z","isPatch":true,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Tue, Oct 23, 2018 at 3:00 PM Jeff King <peff@peff.net> wrote:\n> On Tue, Oct 23, 2018 at 02:50:19PM -0700, Carlo Marcelo Arenas Belón wrote:\n> >  #define implement_static_commit_slab(slabname, elemtype) \\\n> > -     implement_commit_slab(slabname, elemtype, static MAYBE_UNUSED)\n> > +     implement_commit_slab(slabname, elemtype, MAYBE_UNUSED static)\n>\n> Is this hunk necessary?\n\nit works eitherway but the proposed syntax is IMHO better aligned\n(when used in a function definition) as it matches better the syntax\nfrom C++ attribute: maybe_unused (since C++17) [1]\n\n__attribute__(unused) (the GCC extension) is meant to be applied to\nfunction declarations, but we are not generating those.\n\nCarlo\n\n[1] https://en.cppreference.com/w/cpp/language/attributes/maybe_unused\n"}]}