delta-islands: avoid unused function messages
v3 cover delta-islands: avoid unused function messages
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 because of its use of static functions.
Changes from v2:
* Relay in the C code including git-compat-util as suggested by Jeff
* Commit message cleanup
* Make changes hdr-check clean
Changes from v1:
* Use MAYBE_UNUSED for all cases as suggested by Duy
Carlo Marcelo Arenas Belón (3):
commit-slab: move MAYBE_UNUSED into git-compat-util
khash: silence -Wunused-function in delta-islands from khash
commit-slab: missing definitions and forward declarations (hdr-check)
commit-slab-impl.h | 4 ++-- commit-slab.h | 2 ++ git-compat-util.h | 2 ++ khash.h | 2 +- 4 files changed, 7 insertions(+), 3 deletions(-)
Signature · 1 line
2.19.1
PATCH v3 1/3 commit-slab: move MAYBE_UNUSED into git-compat-util
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
--- commit-slab-impl.h | 4 +--- git-compat-util.h | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-)
commit-slab-impl.h +1 -3
@@ -1,10 +1,8 @@#ifndef COMMIT_SLAB_IMPL_H #define COMMIT_SLAB_IMPL_H -#define MAYBE_UNUSED __attribute__((__unused__)) -#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, )
git-compat-util.h +2 -0
@@ -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"
Signature · 1 line
2.19.1
PATCH v3 2/3 khash: silence -Wunused-function in delta-islands from khash
In reply to Carlo Marcelo Arenas Belónshowing the following when compiled with latest clang (OpenBSD, Fedora 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]
--- khash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
khash.h +1 -1
@@ -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, MAYBE_UNUSED static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)/* Other convenient macros... */
Signature · 1 line
2.19.1
Re: [PATCH v3 2/3] khash: silence -Wunused-function in delta-islands from khash
Quoted 2 lines · Carlo Marcelo Arenas Belón
showing the following when compiled with latest clang (OpenBSD, Fedora and macOS):
s/^s/S/
This applies to your other commit messages as well.
But more importantly, please be explicit about the compiler version that emits the warning, so others won't have to guess when stumbling upon this commit in a few months or years time.
Quoted 30 lines
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 <[email protected]>Suggested-by Nguyễn Thái Ngọc Duy <[email protected]>Signed-off-by Carlo Marcelo Arenas Belón <[email protected]>Signed-off-by Junio C Hamano <[email protected]>--- khash.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)khash.h +1
-1@@ -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, MAYBE_UNUSED static inline, khkey_t, khval_t, kh_is_map, __hash_func, __hash_equal)/* Other convenient macros... */ Signature · 1 line
2.19.1
PATCH v3 3/3 commit-slab: missing definitions and forward declarations (hdr-check)
In reply to Carlo Marcelo Arenas Belónstruct commmit needs to be defined before commit-slab can generate working code, object_id should be at least known through a forward declaration
--- commit-slab-impl.h | 2 ++ commit-slab.h | 2 ++ 2 files changed, 4 insertions(+)
commit-slab-impl.h +2 -0
@@ -1,6 +1,8 @@#ifndef COMMIT_SLAB_IMPL_H #define COMMIT_SLAB_IMPL_H +#include "commit.h" +#define implement_static_commit_slab (slabname, elemtype) \ implement_commit_slab(slabname, elemtype, MAYBE_UNUSED static)
commit-slab.h +2 -0
@@ -1,6 +1,8 @@#ifndef COMMIT_SLAB_H #define COMMIT_SLAB_H +struct object_id ; +#include "commit-slab-decl.h" #include "commit-slab-impl.h"
Signature · 1 line
2.19.1
Re: [PATCH v3 3/3] commit-slab: missing definitions and forward declarations (hdr-check)
Quoted 29 lines · Carlo Marcelo Arenas Belón
struct commmit needs to be defined before commit-slab can generate working code,
object_idshould be at least known through a forward declarationSigned-off-by Carlo Marcelo Arenas Belón <[email protected]>--- commit-slab-impl.h | 2 ++ commit-slab.h | 2 ++ 2 files changed, 4 insertions(+)commit-slab-impl.h +2
-0@@ -1,6 +1,8 @@#ifndef COMMIT_SLAB_IMPL_H #define COMMIT_SLAB_IMPL_H +#include "commit.h" +#define implement_static_commit_slab (slabname, elemtype) \ implement_commit_slab(slabname, elemtype, MAYBE_UNUSED static)commit-slab.h +0
-0
commit-slab.h line 8
Quoted 8 lines
commit-slab.h +2
-0@@ -1,6 +1,8 @@#ifndef COMMIT_SLAB_H #define COMMIT_SLAB_H +struct object_id ; +#include "commit-slab-decl.h" #include "commit-slab-impl.h"
Hmm, sorry, I don't see how this patch has anything to do with the other two patches! ;-)
Also, I have a patch to fix up the 'commit-reach.h' header (it was part of my original series, just had to update the commit message), which adds these very #includes and forward declarations when _using_ the commit-slab.
I haven't tried applying your patches yet, which may answer my questions, so I am a little puzzled.
ATB,
Ramsay Jones
Re: [PATCH v3 3/3] commit-slab: missing definitions and forward declarations (hdr-check)
Quoted 44 lines · Ramsay Jones
Quoted 29 lines · Carlo Marcelo Arenas Belón
struct commmit needs to be defined before commit-slab can generate working code,
object_idshould be at least known through a forward declarationSigned-off-by Carlo Marcelo Arenas Belón <[email protected]>--- commit-slab-impl.h | 2 ++ commit-slab.h | 2 ++ 2 files changed, 4 insertions(+)commit-slab-impl.h +2
-0@@ -1,6 +1,8 @@#ifndef COMMIT_SLAB_IMPL_H #define COMMIT_SLAB_IMPL_H +#include "commit.h" +#define implement_static_commit_slab (slabname, elemtype) \ implement_commit_slab(slabname, elemtype, MAYBE_UNUSED static)commit-slab.h +0
-0commit-slab.h line 8
Quoted 8 lines
commit-slab.h +2
-0@@ -1,6 +1,8 @@#ifndef COMMIT_SLAB_H #define COMMIT_SLAB_H +struct object_id ; +#include "commit-slab-decl.h" #include "commit-slab-impl.h" Hmm, sorry, I don't see how this patch has anything to do with the other two patches! ;-)
Quoted 7 lines
Also, I have a patch to fix up the '
commit-reach.h' header (it was part of my original series, just had to update the commit message), which adds these very #includes and forward declarations when _using_ the commit-slab.I haven't tried applying your patches yet, which may answer my questions, so I am a little puzzled.
So, having now applied your patches, I still don't see what this patch has to do with the others! I suppose it is dependent on the compiler/version? (the most up-to-date version of clang available to me is 5.0.1).
Yes, this will 'fix' the 'commit-reach.h' header (not surprising), but I prefer my patch. ;-)
Still puzzled.
ATB,
Ramsay Jones
Re: [PATCH v3 3/3] commit-slab: missing definitions and forward declarations (hdr-check)
Quoted 2 lines · Ramsay Jones
Yes, this will 'fix' the '
commit-reach.h' header (not surprising), but I prefer my patch. ;-)
I apologize, I joined the list recently and so might had missed a reroll; the merged series in pu doesn't seem to include it and the error was around the code I changed, so wanted to make sure it would be addressed sooner rather than later.
eitherway, I agree with you my patch (or something better) would fit better in your topic branch than on mine and while I haven't seen your patch I am sure is most likely better.
Quoted 1 line
Still puzzled.
this are the last lines of a make hdr-check in Fedora Rawhide, it should behave the same regardless of OS or compiler used IMHO
HDR commit-reach.h
commit-reach.h:45:28: warning: ‘struct object_id’ declared inside parameter list will not be visible outside of this definition or declaration
int ref_newer(const struct object_id *new_oid, const struct object_id
*old_oid);
^~~~~~~~~In file included from commit-slab.h:5,
from commit-reach.h:4:commit-reach.h: In function ‘contains_cache_at_peek’:
commit-slab-impl.h:47:14: error: dereferencing pointer to incomplete type ‘const struct commit’
nth_slab = c->index / s->slab_size; \
^~commit-slab-impl.h:7:2: note: in expansion of macro ‘implement_commit_slab’
implement_commit_slab(slabname, elemtype, static MAYBE_UNUSED) ^~~~~~~~~~~~~~~~~~~~~
commit-slab.h:49:2: note: in expansion of macro ‘implement_static_commit_slab’
implement_static_commit_slab(slabname, elemtype) ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
commit-reach.h:57:1: note: in expansion of macro ‘define_commit_slab’
define_commit_slab(contains_cache, enum contains_result); ^~~~~~~~~~~~~~~~~~commit-reach.h: At top level:
commit-reach.h:69:41: warning: ‘struct object_array’ declared inside parameter list will not be visible outside of this definition or declaration
int can_all_from_reach_with_flag(struct object_array *from,
^~~~~~~~~~~~
make: *** [Makefile:2685: commit-reach.hco] Error 1
Carlo
Re: [PATCH v3 3/3] commit-slab: missing definitions and forward declarations (hdr-check)
Quoted 5 lines · Carlo Arenas
Quoted 2 lines · Ramsay Jones
Yes, this will 'fix' the '
commit-reach.h' header (not surprising), but I prefer my patch. ;-)
Quoted 8 lines
I apologize, I joined the list recently and so might had missed a reroll; the merged series in pu doesn't seem to include it and the error was around the code I changed, so wanted to make sure it would be addressed sooner rather than later.
eitherway, I agree with you my patch (or something better) would fit better in your topic branch than on mine and while I haven't seen your patch I am sure is most likely better.
Hmm, I don't know about that!
Since the original series has progressed, any additions will now result in a new set of patches, rather than a re-roll.
The original 'commit-reach.h' patch was not applied as part of the last series, since the commit message was felt to be lacking (well, it was actually non-existent!). ;-)
I have been making some additional changes to the 'hdr-check' target in the Makefile, but I haven't quite finished. I will send the other (non-Makefile) changes soon. [These patches will make the 'master' and 'next' branches 'hdr-check' clean for me].
ATB,
Ramsay Jones