Re: [dpdk-dev] [dpdk-techboard] [PATCH v6 2/8] eal: fix error attribute use for clang

[Thomas Monjalon <thomas@monjalon.net>]

28/01/2021 16:16, Bruce Richardson:
> On Thu, Jan 28, 2021 at 02:16:10PM +0000, Bruce Richardson wrote:
> > On Thu, Jan 28, 2021 at 02:36:25PM +0100, David Marchand wrote:
> > > On Thu, Jan 28, 2021 at 12:20 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > > > If the compiler has neither error or diagnose_if support, an internal
> > > > > API can be called without ALLOW_INTERNAL_API.
> > > > > I prefer a build error complaining on an unknown attribute rather than
> > > > > silence a check.
> > > > >
> > > > > I.e.
> > > > >
> > > > > #ifndef ALLOW_INTERNAL_API
> > > > >
> > > > > #if __has_attribute(diagnose_if) /* For clang */
> > > > > #define __rte_internal \
> > > > > __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
> > > > > section(".text.internal")))
> > > > >
> > > > > #else
> > > > > #define __rte_internal \
> > > > > __attribute__((error("Symbol is not public ABI"), \
> > > > > section(".text.internal")))
> > > > >
> > > > > #endif
> > > > >
> > > > > #else /* ALLOW_INTERNAL_API */
> > > > >
> > > > > #define __rte_internal \
> > > > > section(".text.internal")))
> > > > >
> > > > > #endif
> > > > >
> > > >
> > > > Would this not mean that if someone was using a compiler that supported
> > > > neither that they could never include any header file which contained any
> > > > internal functions? I'd rather err on the side of allowing this, on the
> > > > basis that the symbol should be already documented as internal and this is
> > > > only an additional sanity check.
> > > 
> > > - Still not a fan.
> > > We will never know about those compilers behavior, like how we caught
> > > the clang issue and found an alternative.
> > > 
> > 
> > So I understand, but I'm still concerned about breaking something that was
> > previously working. It's one thing a DPDK developer catching issues with
> > clang, quite another a user catching it when trying to build their own
> > application.
> > 
> > We probably need some other opinions on this one.
> > 
> Adding Tech-board to see if we can get some more thoughts on this before I do
> another revision of this set.

What are the alternatives?