Re: [PATCH 1/3] do full type check in COPY_ARRAY and MOVE_ARRAY

From: René Scharfe <l.s.r@web.de>
Date: Jan 1, 2023, 07:41 UTC
Message-ID: <220515b3-3924-c8d2-ff20-7017caa7dead@web.de>
In-Reply-To: <xmqqsfguhplc.fsf@gitster.g>

Am 01.01.23 um 04:59 schrieb Junio C Hamano:

Show 5 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>> ...  I think what you ideally want to enforce is that
>> typeof(dst) is exactly typeof(src), or typeof(src) sans constness
>> (i.e. you can populate non-const array from a const template)?

Yes.

Moving the type check shared between COPY_ARRAY and MOVE_ARRAY to a new macro is a good idea.

Using compiler extensions when available, as we already do for other purposes, is a good idea as well. I managed to ignore the existing use somehow.

Show 24 quoted lines

>
> IOW, I am wondering if something like this is an improvement.
>
>  git-compat-util.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git c/git-compat-util.h w/git-compat-util.h
> index a76d0526f7..be435615f0 100644
> --- c/git-compat-util.h
> +++ w/git-compat-util.h
> @@ -97,8 +97,13 @@ struct strbuf;
>  # define BARF_UNLESS_AN_ARRAY(arr)						\
>  	BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(__typeof__(arr), \
>  							   __typeof__(&(arr)[0])))
> +# define ARRAYS_COPYABLE_OR_ZERO(src,dst) \
> +	(BUILD_ASSERT_OR_ZERO(!__builtin_types_compatible_p(__typeof__(src), \
> +							    __typeof__(dst))) + \
> +			      (0 ? *(dst) = *(src) : 0))
>  #else
>  # define BARF_UNLESS_AN_ARRAY(arr) 0
> +# define ARRAYS_COPYABLE_OR_ZERO(src,dst) (0 ? *(dst) = *(src) : 0))
>  #endif
>  /*
>   * ARRAY_SIZE - get the number of elements in a visible array