From: Andi Shyti <andi.shyti@linux.intel.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: thomas.hellstrom@linux.intel.com, chris@chris-wilson.co.uk,
daniel@ffwll.ch, keescook@chromium.org, jani.nikula@intel.com,
intel-gfx@lists.freedesktop.org, ndesaulniers@google.com,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org,
andrzej.hajda@intel.com, dlatypov@google.com,
matthew.auld@intel.com, airlied@redhat.com, mchehab@kernel.org,
vitor@massaru.org, nirmoy.das@intel.com
Subject: Re: [Intel-gfx] [PATCH v10 3/9] compiler_types.h: Add assert_type to catch type mis-match while compiling
Date: Mon, 12 Sep 2022 12:21:42 +0200 [thread overview]
Message-ID: <Yx8INguMNIU4RIGY@alfio.lan> (raw)
In-Reply-To: <579b8532-7687-4620-d146-c8ffbbc14097@rasmusvillemoes.dk>
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
WARNING: multiple messages have this Message-ID (diff)
From: Andi Shyti <andi.shyti@linux.intel.com>
To: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: thomas.hellstrom@linux.intel.com, mauro.chehab@linux.intel.com,
chris@chris-wilson.co.uk, keescook@chromium.org,
jani.nikula@intel.com, intel-gfx@lists.freedesktop.org,
ndesaulniers@google.com, linux-kernel@vger.kernel.org,
dri-devel@lists.freedesktop.org,
Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>,
andrzej.hajda@intel.com, dlatypov@google.com,
matthew.auld@intel.com, Andi Shyti <andi.shyti@linux.intel.com>,
airlied@redhat.com, mchehab@kernel.org, vitor@massaru.org,
nirmoy.das@intel.com
Subject: Re: [PATCH v10 3/9] compiler_types.h: Add assert_type to catch type mis-match while compiling
Date: Mon, 12 Sep 2022 12:21:42 +0200 [thread overview]
Message-ID: <Yx8INguMNIU4RIGY@alfio.lan> (raw)
In-Reply-To: <579b8532-7687-4620-d146-c8ffbbc14097@rasmusvillemoes.dk>
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
WARNING: multiple messages have this Message-ID (diff)
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
Date: Mon, 12 Sep 2022 12:21:42 +0200 [thread overview]
Message-ID: <Yx8INguMNIU4RIGY@alfio.lan> (raw)
In-Reply-To: <579b8532-7687-4620-d146-c8ffbbc14097@rasmusvillemoes.dk>
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
next prev parent reply other threads:[~2022-09-12 10:21 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-09 10:59 [Intel-gfx] [PATCH v10 0/9] Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 1/9] overflow: Allow mixed type arguments Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-11 10:31 ` [Intel-gfx] " Andi Shyti
2022-09-11 10:31 ` Andi Shyti
2022-09-11 10:31 ` Andi Shyti
2022-09-12 8:38 ` [Intel-gfx] " Andrzej Hajda
2022-09-12 8:38 ` Andrzej Hajda
2022-09-12 8:38 ` Andrzej Hajda
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 2/9] overflow: Move and add few utility macros into overflow Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-13 11:53 ` [Intel-gfx] " Kees Cook
2022-09-13 11:53 ` Kees Cook
2022-09-13 11:53 ` Kees Cook
2022-09-17 23:31 ` [Intel-gfx] " kernel test robot
2022-09-17 23:31 ` kernel test robot
2022-09-17 23:31 ` kernel test robot
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 3/9] compiler_types.h: Add assert_type to catch type mis-match while compiling Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-11 11:04 ` [Intel-gfx] " Andi Shyti
2022-09-11 11:04 ` Andi Shyti
2022-09-11 11:04 ` Andi Shyti
2022-09-12 9:52 ` [Intel-gfx] " Rasmus Villemoes
2022-09-12 9:52 ` Rasmus Villemoes
2022-09-12 9:52 ` Rasmus Villemoes
2022-09-12 10:21 ` Andi Shyti [this message]
2022-09-12 10:21 ` Andi Shyti
2022-09-12 10:21 ` Andi Shyti
2022-09-13 12:01 ` [Intel-gfx] " Kees Cook
2022-09-13 12:01 ` Kees Cook
2022-09-13 12:01 ` Kees Cook
2022-09-21 14:10 ` [Intel-gfx] " Gwan-gyeong Mun
2022-09-21 14:10 ` Gwan-gyeong Mun
2022-09-21 14:10 ` Gwan-gyeong Mun
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 4/9] drm/i915/gem: Typecheck page lookups Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-10 6:53 ` kernel test robot
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 5/9] drm/i915: Check for integer truncation on scatterlist creation Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 6/9] drm/i915: Check for integer truncation on the configuration of ttm place Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 7/9] drm/i915: Check if the size is too big while creating shmem file Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 8/9] drm/i915: Use error code as -E2BIG when the size of gem ttm object is too large Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` [Intel-gfx] [PATCH v10 9/9] drm/i915: Remove truncation warning for large objects Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 10:59 ` Gwan-gyeong Mun
2022-09-09 11:24 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation Patchwork
2022-09-11 11:22 ` [Intel-gfx] ✗ Fi.CI.BUILD: failure for Fixes integer overflow or integer truncation issues in page lookups, ttm place configuration and scatterlist creation (rev2) Patchwork
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Yx8INguMNIU4RIGY@alfio.lan \
--to=andi.shyti@linux.intel.com \
--cc=airlied@redhat.com \
--cc=andrzej.hajda@intel.com \
--cc=chris@chris-wilson.co.uk \
--cc=daniel@ffwll.ch \
--cc=dlatypov@google.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@intel.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=matthew.auld@intel.com \
--cc=mchehab@kernel.org \
--cc=ndesaulniers@google.com \
--cc=nirmoy.das@intel.com \
--cc=thomas.hellstrom@linux.intel.com \
--cc=vitor@massaru.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.