Re: drivers/infiniband/hw/qib/qib_sysfs.c:413:1: error: static_assert expression is not an integral constant expression

From: Jason Gunthorpe <jgg@nvidia.com>
To: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>,
	kernel test robot <lkp@intel.com>,
	llvm@lists.linux.dev, kbuild-all@lists.01.org,
	linux-kernel@vger.kernel.org, clang-built-linux@googlegroups.com
Subject: Re: drivers/infiniband/hw/qib/qib_sysfs.c:413:1: error: static_assert expression is not an integral constant expression
Date: Mon, 13 Sep 2021 16:16:29 -0300	[thread overview]
Message-ID: <20210913191629.GC4065468@nvidia.com> (raw)
In-Reply-To: <0158604c-8777-51c8-2cb5-b7f016335786@kernel.org>

On Mon, Sep 13, 2021 at 12:13:45PM -0700, Nathan Chancellor wrote:
> On 9/13/2021 12:02 PM, Jason Gunthorpe wrote:
> > On Mon, Sep 13, 2021 at 11:53:25AM -0700, Nathan Chancellor wrote:
> > > On Mon, Sep 13, 2021 at 03:38:13PM -0300, Jason Gunthorpe wrote:
> > > > On Mon, Sep 13, 2021 at 10:00:02AM -0700, Nick Desaulniers wrote:
> > > > 
> > > > > > This macro would like to know that the passed in member name has a u64
> > > > > > type, all the things I've come up with fail on clang - but many work
> > > > > > fine on gcc. Frankly I think this case is a clang bug myself..
> > > > > 
> > > > > Perhaps, though this assertion looks a bit like offsetof() to me. I
> > > > > wonder if that can help here?
> > > > 
> > > > The assertion would logically like to be this:
> > > > 
> > > >           static_assert(typecheck(((struct qib_port *)0)->N, u64))
> > > 
> > > This works for me with both GCC and clang, if that is acceptable to you?
> > > It fails if you change one of the variables to 'u32'.
> > 
> > Yes, thanks. Can't say I've even heard of __same_type before :\ would
> > be nice if this was in typecheck.h along with the other variations of
> > the same idea. Presumably it is a little bit different from those
> > somehow?
> 
> Good question... commit d2c123c27db8 ("module_param: add __same_type
> convenience wrapper for __builtin_types_compatible_p") introduced it so that
> it could be used in commit fddd52012295 ("module_param: allow 'bool'
> module_params to be bool, not just int."); I am guessing that typecheck()
> could not be used in those cases. Perhaps all instances of typecheck() could
> be converted to __same_type()?
> 
> Do you want me to send a formal patch for that diff?

I think it is a reasonable thing to do if it works, but I do wonder if
they are not quite the same thing considering qualifiers/etc

$ git grep typecheck | wc -l
120
$ #git grep __same_type | wc -l
39

I would delete __same_type if typecheck can be functionally identical
otherwise I'd say this should all be in typecheck.h and the functional
difference clearly documented..

Jason