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:

quoted

On Thu, Jan 28, 2021 at 12:20 PM Bruce Richardson
[off-list ref] wrote:

quoted

quoted

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.