# [PATCH] khash: silence -Wunused-function 9 messages from 2018-10-23 to 2018-10-23. Participants: Carlo Marcelo Arenas Belón, Duy Nguyen, René Scharfe, Carlo Arenas, Jeff King. Thread: https://gitlist.dev/t/49650 ## Carlo Marcelo Arenas Belón, 2018-10-23 11:34 Subject: [PATCH] khash: silence -Wunused-function Message-ID: <20181023113416.90958-1-carenas@gmail.com> URL: https://gitlist.dev/e/20181023113416.90958-1-carenas%40gmail.com ``` after 36da893114 ("config.mak.dev: enable -Wunused-function", 2018-10-18) macro generated code should use a similar solution than commit-slab to silence the compiler. Signed-off-by: Carlo Marcelo Arenas Belón --- khash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/khash.h b/khash.h index d10caa0c35..39c2833877 100644 --- a/khash.h +++ b/khash.h @@ -234,7 +234,7 @@ static const double __ac_HASH_UPPER = 0.77; __KHASH_IMPL(name, SCOPE, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) #define KHASH_INIT(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \ - KHASH_INIT2(name, static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) + KHASH_INIT2(name, __attribute__((__unused__)) static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) /* Other convenient macros... */ -- 2.19.1 ``` ## Duy Nguyen, 2018-10-23 15:44 Subject: Re: [PATCH] khash: silence -Wunused-function Message-ID: URL: https://gitlist.dev/e/CACsJy8AXjhbY9WWjYtMk128TZHLBCf4Ye4fe-ryXhoP9j2jtdw%40mail.gmail.com In-Reply-To: <20181023113416.90958-1-carenas@gmail.com> ``` On Tue, Oct 23, 2018 at 1:42 PM Carlo Marcelo Arenas Belón wrote: > > after 36da893114 ("config.mak.dev: enable -Wunused-function", 2018-10-18) > macro generated code should use a similar solution than commit-slab to > silence the compiler. > > Signed-off-by: Carlo Marcelo Arenas Belón > --- > khash.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/khash.h b/khash.h > index d10caa0c35..39c2833877 100644 > --- a/khash.h > +++ b/khash.h > @@ -234,7 +234,7 @@ static const double __ac_HASH_UPPER = 0.77; > __KHASH_IMPL(name, SCOPE, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) > > #define KHASH_INIT(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \ > - KHASH_INIT2(name, static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) > + KHASH_INIT2(name, __attribute__((__unused__)) static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) Maybe move MAYBE_UNUSED from commit-slab-impl.h to git-compat-util.h and use it here so we have an easier time if we have to use a different way to mark unused functions on some exotic platform. > > /* Other convenient macros... */ > > -- > 2.19.1 > -- Duy ``` ## René Scharfe, 2018-10-23 16:19 Subject: Re: [PATCH] khash: silence -Wunused-function Message-ID: <2518b1ff-d16d-da95-fdc2-698dddb61a36@web.de> URL: https://gitlist.dev/e/2518b1ff-d16d-da95-fdc2-698dddb61a36%40web.de In-Reply-To: <20181023113416.90958-1-carenas@gmail.com> ``` Am 23.10.2018 um 13:34 schrieb Carlo Marcelo Arenas Belón: > after 36da893114 ("config.mak.dev: enable -Wunused-function", 2018-10-18) > macro generated code should use a similar solution than commit-slab to > silence the compiler. With Clang 6 and GCC 8 on Debian I don't get any warnings on master or 36da893114. With Clang 6 on OpenBSD I get warnings about the unused khash functions in delta-islands.c on master, though. What do you get, or in other words: why did you send this patch? > Signed-off-by: Carlo Marcelo Arenas Belón > --- > khash.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/khash.h b/khash.h > index d10caa0c35..39c2833877 100644 > --- a/khash.h > +++ b/khash.h > @@ -234,7 +234,7 @@ static const double __ac_HASH_UPPER = 0.77; > __KHASH_IMPL(name, SCOPE, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) > > #define KHASH_INIT(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \ > - KHASH_INIT2(name, static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) > + KHASH_INIT2(name, __attribute__((__unused__)) static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) > > /* Other convenient macros... */ > This fixes the warning on OpenBSD for me. So does moving the KHASH_INIT line from delta-islands.c to a header file (e.g. khash.h or delta-islands.h). René ``` ## Carlo Arenas, 2018-10-23 16:52 Subject: Re: [PATCH] khash: silence -Wunused-function Message-ID: URL: https://gitlist.dev/e/CAPUEspho2MB%3DAruF565XaCPL7e%2BRmfT9iJtbzDAMfFpAkrzoNg%40mail.gmail.com In-Reply-To: <2518b1ff-d16d-da95-fdc2-698dddb61a36@web.de> ``` On Tue, Oct 23, 2018 at 9:19 AM René Scharfe wrote: > With Clang 6 and GCC 8 on Debian I don't get any warnings on master or > 36da893114. I see it with Clang 7 on Fedora (at least Rawhide but suspect also to affect the next release, now in beta: 29) > With Clang 6 on OpenBSD I get warnings about the unused khash functions > in delta-islands.c on master, though. Apple LLVM 9 and 10 will also trigger similar warnings to what you see in OpenBSD and as Junio mentioned recently in a different thread about semantic patches, this currently affects the CI for "Apple": https://travis-ci.org/git/git/builds/444969342 Carlo ``` ## Carlo Marcelo Arenas Belón, 2018-10-23 21:50 Subject: [PATCH v2 0/2] delta-islands: avoid unused function messages Message-ID: <20181023215020.18550-1-carenas@gmail.com> URL: https://gitlist.dev/e/20181023215020.18550-1-carenas%40gmail.com In-Reply-To: ``` the macro generated code from delta-islands (using khash) triggers some unused function warnings in macOS, OpenBSD and some linux with a newer version of clang Carlo Marcelo Arenas Belón (2): commit-slabs: move MAYBE_UNUSED out khash: silence -Wunused-function for delta-islands commit-slab-impl.h | 4 ++-- git-compat-util.h | 2 ++ khash.h | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) -- 2.19.1 ``` ## Carlo Marcelo Arenas Belón, 2018-10-23 21:50 Subject: [PATCH v2 1/2] commit-slabs: move MAYBE_UNUSED out Message-ID: <20181023215020.18550-2-carenas@gmail.com> URL: https://gitlist.dev/e/20181023215020.18550-2-carenas%40gmail.com In-Reply-To: <20181023215020.18550-1-carenas@gmail.com> ``` after 36da893114 ("config.mak.dev: enable -Wunused-function", 2018-10-18) it is expected to be used to prevent -Wunused-function warnings for code that was macro generated Signed-off-by: Carlo Marcelo Arenas Belón --- commit-slab-impl.h | 4 ++-- git-compat-util.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/commit-slab-impl.h b/commit-slab-impl.h index ac1e6d409a..5c0eb91a5d 100644 --- a/commit-slab-impl.h +++ b/commit-slab-impl.h @@ -1,10 +1,10 @@ #ifndef COMMIT_SLAB_IMPL_H #define COMMIT_SLAB_IMPL_H -#define MAYBE_UNUSED __attribute__((__unused__)) +#include "git-compat-util.h" #define implement_static_commit_slab(slabname, elemtype) \ - implement_commit_slab(slabname, elemtype, static MAYBE_UNUSED) + implement_commit_slab(slabname, elemtype, MAYBE_UNUSED static) #define implement_shared_commit_slab(slabname, elemtype) \ implement_commit_slab(slabname, elemtype, ) diff --git a/git-compat-util.h b/git-compat-util.h index 9a64998b24..e4d3967a23 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -408,6 +408,8 @@ static inline char *git_find_last_dir_sep(const char *path) #define LAST_ARG_MUST_BE_NULL #endif +#define MAYBE_UNUSED __attribute__((__unused__)) + #include "compat/bswap.h" #include "wildmatch.h" -- 2.19.1 ``` ## Carlo Marcelo Arenas Belón, 2018-10-23 21:50 Subject: [PATCH v2 2/2] khash: silence -Wunused-function for delta-islands Message-ID: <20181023215020.18550-3-carenas@gmail.com> URL: https://gitlist.dev/e/20181023215020.18550-3-carenas%40gmail.com In-Reply-To: <20181023215020.18550-1-carenas@gmail.com> ``` showing the following when compiled with latest clang (OpenBSD, Fedors and macOS): delta-islands.c:23:1: warning: unused function 'kh_destroy_str' [-Wunused-function] delta-islands.c:23:1: warning: unused function 'kh_clear_str' [-Wunused-function] delta-islands.c:23:1: warning: unused function 'kh_del_str' [-Wunused-function] Reported-by: René Scharfe Suggested-by: Nguyễn Thái Ngọc Duy Signed-off-by: Carlo Marcelo Arenas Belón --- khash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/khash.h b/khash.h index c0da40daa7..fb71c92547 100644 --- a/khash.h +++ b/khash.h @@ -226,7 +226,7 @@ static const double __ac_HASH_UPPER = 0.77; __KHASH_IMPL(name, SCOPE, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) #define KHASH_INIT(name, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) \ - KHASH_INIT2(name, static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) + KHASH_INIT2(name, MAYBE_UNUSED static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal) /* Other convenient macros... */ -- 2.19.1 ``` ## Jeff King, 2018-10-23 22:00 Subject: Re: [PATCH v2 1/2] commit-slabs: move MAYBE_UNUSED out Message-ID: <20181023220019.GA31613@sigill.intra.peff.net> URL: https://gitlist.dev/e/20181023220019.GA31613%40sigill.intra.peff.net In-Reply-To: <20181023215020.18550-2-carenas@gmail.com> ``` On Tue, Oct 23, 2018 at 02:50:19PM -0700, Carlo Marcelo Arenas Belón wrote: > after 36da893114 ("config.mak.dev: enable -Wunused-function", 2018-10-18) > it is expected to be used to prevent -Wunused-function warnings for code > that was macro generated Makes sense. > diff --git a/commit-slab-impl.h b/commit-slab-impl.h > index ac1e6d409a..5c0eb91a5d 100644 > --- a/commit-slab-impl.h > +++ b/commit-slab-impl.h > @@ -1,10 +1,10 @@ > #ifndef COMMIT_SLAB_IMPL_H > #define COMMIT_SLAB_IMPL_H > > -#define MAYBE_UNUSED __attribute__((__unused__)) > +#include "git-compat-util.h" We shouldn't need to include git-compat-util.h, as the C files would already do so, via this rule in CodingGuidelines: - The first #include in C files, except in platform specific compat/ implementations, must be either "git-compat-util.h", "cache.h" or "builtin.h". You do not have to include more than one of these. (and if there is a case that does not, we should fix that). > #define implement_static_commit_slab(slabname, elemtype) \ > - implement_commit_slab(slabname, elemtype, static MAYBE_UNUSED) > + implement_commit_slab(slabname, elemtype, MAYBE_UNUSED static) Is this hunk necessary? > diff --git a/git-compat-util.h b/git-compat-util.h > index 9a64998b24..e4d3967a23 100644 > --- a/git-compat-util.h > +++ b/git-compat-util.h > @@ -408,6 +408,8 @@ static inline char *git_find_last_dir_sep(const char *path) > #define LAST_ARG_MUST_BE_NULL > #endif > > +#define MAYBE_UNUSED __attribute__((__unused__)) > + > #include "compat/bswap.h" Looks good. -Peff ``` ## Carlo Arenas, 2018-10-23 23:02 Subject: Re: [PATCH v2 1/2] commit-slabs: move MAYBE_UNUSED out Message-ID: URL: https://gitlist.dev/e/CAPUEspiAhmJ5%2BqTUcA6x%2BeSPc9jmOLdF7QsSsSfwJSy5r%3D06PA%40mail.gmail.com In-Reply-To: <20181023220019.GA31613@sigill.intra.peff.net> ``` On Tue, Oct 23, 2018 at 3:00 PM Jeff King wrote: > On Tue, Oct 23, 2018 at 02:50:19PM -0700, Carlo Marcelo Arenas Belón wrote: > > #define implement_static_commit_slab(slabname, elemtype) \ > > - implement_commit_slab(slabname, elemtype, static MAYBE_UNUSED) > > + implement_commit_slab(slabname, elemtype, MAYBE_UNUSED static) > > Is this hunk necessary? it works eitherway but the proposed syntax is IMHO better aligned (when used in a function definition) as it matches better the syntax from C++ attribute: maybe_unused (since C++17) [1] __attribute__(unused) (the GCC extension) is meant to be applied to function declarations, but we are not generating those. Carlo [1] https://en.cppreference.com/w/cpp/language/attributes/maybe_unused ```