diff mbox series

[v2,1/2] commit-slabs: move MAYBE_UNUSED out

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

Commit Message

Carlo Arenas Oct. 23, 2018, 9:50 p.m. UTC
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(-)

Comments

Jeff King Oct. 23, 2018, 10 p.m. UTC | #1
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 Oct. 23, 2018, 11:02 p.m. UTC | #2
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 mbox series

Patch

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"