| Message ID | 20181023215020.18550-2-carenas@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/2] commit-slabs: move MAYBE_UNUSED out | expand |
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
On Tue, Oct 23, 2018 at 3:00 PM Jeff King <peff@peff.net> 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
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"
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 <carenas@gmail.com> --- commit-slab-impl.h | 4 ++-- git-compat-util.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-)