Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 12 Sep 2022 12:21:42 +0200
From:   Andi Shyti <andi.shyti@linux.intel.com>
To:     Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc:     Andi Shyti <andi.shyti@linux.intel.com>,
        Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>,
        intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org, mchehab@kernel.org,
        chris@chris-wilson.co.uk, matthew.auld@intel.com,
        thomas.hellstrom@linux.intel.com, jani.nikula@intel.com,
        nirmoy.das@intel.com, airlied@redhat.com, daniel@ffwll.ch,
        andrzej.hajda@intel.com, keescook@chromium.org,
        mauro.chehab@linux.intel.com, vitor@massaru.org,
        dlatypov@google.com, ndesaulniers@google.com
Subject: Re: [PATCH v10 3/9] compiler_types.h: Add assert_type to catch type
 mis-match while compiling
Message-ID: <Yx8INguMNIU4RIGY@alfio.lan>
References: <20220909105913.752049-1-gwan-gyeong.mun@intel.com>
 <20220909105913.752049-4-gwan-gyeong.mun@intel.com>
 <Yx3A16ZElKOeJr0o@alfio.lan>
 <579b8532-7687-4620-d146-c8ffbbc14097@rasmusvillemoes.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <579b8532-7687-4620-d146-c8ffbbc14097@rasmusvillemoes.dk>
Precedence: bulk

Hi Rasmus,

Thanks for dropping in,

[...]

> >> + * @t1: data type or variable
> >> + * @t2: data type or variable
> >> + *
> >> + * The first and second arguments can be data types or variables or mixed (the
> >> + * first argument is the data type and the second argument is variable or vice
> >> + * versa). It determines whether the first argument's data type and the second
> >> + * argument's data type are the same while compiling, and it breaks compile if
> >> + * the two types are not the same.
> >> + * See also assert_typable().
> >> + */
> >> +#define assert_type(t1, t2) _Static_assert(__same_type(t1, t2))
> > 
> > In C11 _Static_assert is defined as:
> > 
> >   _Static_assert ( constant-expression , string-literal ) ;
> > 
> > While
> > 
> >   _Static_assert ( constant-expression ) ;
> > 
> > is defined in C17 along with the previous. I think you should add
> > the error message as a 'string-literal'.
> 
> See how static_assert() is defined in linux/build_bug.h, and let's avoid
> using _Static_assert directly. So this should IMO just be

yes, our definition of static_assert() is against the C11
specification, which should define it as:

  #define static_assert _Static_assert

this doesn't make me a big fan of it. But, because it's widely
used, I think it should be used here as well, as you are
suggesting.

> #define assert_same_type(t1, t2) static_assert(__same_type(t1, t2))
> 
> (including the naming of the macro; I don't think "assert_type" is a
> good name). No need to add an explicit string literal, the name of the
> macro and the constant expression itself are plenty to explain what is
> being asserted (with the latter being the reason the string was made
> optional).

The string literal would be "__same_type(t1, t2)", right? I would
still use something more explicit... up to Gwan-gyeong.

Thanks,
Andi