On Thu, Jan 28, 2021 at 12:20 PM Bruce Richardson
<bruce.richard...@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.
- I just caught a build error with RHEL7 gcc:
[1/2127] Compiling C object
lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o
FAILED: lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o
ccache cc -Ilib/librte_eal.a.p -Ilib -I../lib -I. -I.. -Iconfig
-I../config -Ilib/librte_eal/include -I../lib/librte_eal/include
-Ilib/librte_eal/linux/include -I../lib/librte_eal/linux/include
-Ilib/librte_eal/x86/include -I../lib/librte_eal/x86/include
-Ilib/librte_eal/common -I../lib/librte_eal/common -Ilib/librte_eal
-I../lib/librte_eal -Ilib/librte_kvargs -I../lib/librte_kvargs
-Ilib/librte_metrics -I../lib/librte_metrics -Ilib/librte_telemetry
-I../lib/librte_telemetry -pipe -D_FILE_OFFSET_BITS=64 -Wall
-Winvalid-pch -O3 -include rte_config.h -Wextra -Wcast-qual
-Wdeprecated -Wformat -Wformat-nonliteral -Wformat-security
-Wmissing-declarations -Wmissing-prototypes -Wnested-externs
-Wold-style-definition -Wpointer-arith -Wsign-compare
-Wstrict-prototypes -Wundef -Wwrite-strings
-Wno-missing-field-initializers -D_GNU_SOURCE -fPIC -march=native
-DALLOW_EXPERIMENTAL_API -DALLOW_INTERNAL_API '-DABI_VERSION="21.1"'
-MD -MQ lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o -MF
lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o.d -o
lib/librte_eal.a.p/librte_eal_common_eal_common_config.c.o -c
../lib/librte_eal/common/eal_common_config.c
In file included from ../lib/librte_eal/include/rte_dev.h:24:0,
from ../lib/librte_eal/common/eal_private.h:12,
from ../lib/librte_eal/common/eal_common_config.c:9:
../lib/librte_eal/include/rte_compat.h:22:51: error: missing binary
operator before token "("
#if !defined ALLOW_INTERNAL_API && __has_attribute(error) /* For GCC */
^
../lib/librte_eal/include/rte_compat.h:28:53: error: missing binary
operator before token "("
#elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /*
For clang */
^
I can see that gcc doc recommends checking for __has_attribute availability.
Pasting from google cache, since the gcc.gnu.org doc website seems unavailable.
"""
4.2.6 __has_attribute
The special operator __has_attribute (operand) may be used in ‘#if’
and ‘#elif’ expressions to test whether the attribute referenced by
its operand is recognized by GCC. Using the operator in other contexts
is not valid. In C code, if compiling for strict conformance to
standards before C2x, operand must be a valid identifier. Otherwise,
operand may be optionally introduced by the attribute-scope:: prefix.
The attribute-scope prefix identifies the “namespace” within which the
attribute is recognized. The scope of GCC attributes is ‘gnu’ or
‘__gnu__’. The __has_attribute operator by itself, without any operand
or parentheses, acts as a predefined macro so that support for it can
be tested in portable code. Thus, the recommended use of the operator
is as follows:
#if defined __has_attribute
# if __has_attribute (nonnull)
# define ATTR_NONNULL __attribute__ ((nonnull))
# endif
#endif
The first ‘#if’ test succeeds only when the operator is supported by
the version of GCC (or another compiler) being used. Only when that
test succeeds is it valid to use __has_attribute as a preprocessor
operator. As a result, combining the two tests into a single
expression as shown below would only be valid with a compiler that
supports the operator but not with others that don’t.
#if defined __has_attribute && __has_attribute (nonnull) /* not portable */
…
#endif
"""
--
David Marchand
