> From: Thomas Monjalon [mailto:tho...@monjalon.net] > Sent: Tuesday, 15 October 2024 16.13 > > 15/10/2024 14:53, Morten Brørup: > > > From: David Marchand [mailto:david.march...@redhat.com] > > > @@ -255,7 +255,13 @@ __rte_experimental > > > static inline bool > > > rte_bitset_test(const uint64_t *bitset, size_t bit_num) > > > { > > > +#ifdef ALLOW_EXPERIMENTAL_API > > > return __RTE_BITSET_DELEGATE(rte_bit_test, bitset, bit_num); > > > +#else > > > + RTE_SET_USED(bitset); > > > + RTE_SET_USED(bit_num); > > > + return false; > > > +#endif > > > } > > > > This looks wrong! The API is exposed, but does nothing. > > Yes, this is what we did already in the past for other experimental > functions > called from inline functions. > There is no choice, the compiler is hitting the warning. > > > It is possible to build without ALLOW_EXPERIMENTAL_API; the compiler > will emit warnings when using experimental APIs. > > Yes it is possible to build, > but it is not said it should work the same. > > > If those compiler warnings are not handled as errors, the compiled > application will be full of bugs. > > Yes, that's why there are warnings. > We may document it better but that's the behavior we have for years. > There is no easy solution, and making experimental functions work > without defining ALLOW_EXPERIMENTAL_API is not a really interesting > goal. > I think the word "allow" suggests it is not supposed to work if not > allowed.
There's a world of difference between "experimental, might have bugs" - which is what I (and possibly other DPDK consumers) expect - and "experimental, we know for a fact that it doesn't work" - which is quite a surprise to me. > > It would be more interesting to make sure the users understand > why we have this flag and how to enable it. > I propose adding some docs, and mentioning ALLOW_EXPERIMENTAL_API > in the the __rte_experimental message in rte_compat.h. > If we know that some of these warnings cause bugs in DPDK, we should elevate these specific instances to error level. Regarding this specific patch: Would it be possible to change it to behave like patch 1/3, i.e. completely omit the experimental APIs if ALLOW_EXPERIMENTAL_API is not defined?
