17/04/2023 16:46, Tyler Retzlaff пишет:
On Sun, Apr 16, 2023 at 10:29:38PM +0100, Konstantin Ananyev wrote:
10/04/2023 21:53, Tyler Retzlaff пишет:
On Mon, Apr 10, 2023 at 08:59:33PM +0100, Konstantin Ananyev wrote:
06/04/2023 01:45, Tyler Retzlaff пишет:
Forward declaration of a typedef is a non-standard extension and is not
supported by msvc. Use an int instead.
Abstract the use of the int/enum rte_cpu_flag_t in function parameter
lists by re-typdefing the enum rte_cpu_flag_t to the rte_cpu_flag_t
identifier.
Remove the use of __extension__ on function prototypes where
rte_cpu_flag_t appeared in parameter lists, it is sufficient to have the
conditionally compiled __extension__ at the non-standard forward
declaration site.
Signed-off-by: Tyler Retzlaff <roret...@linux.microsoft.com>
---
lib/eal/include/generic/rte_cpuflags.h | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/lib/eal/include/generic/rte_cpuflags.h
b/lib/eal/include/generic/rte_cpuflags.h
index d35551e..87ab03c 100644
--- a/lib/eal/include/generic/rte_cpuflags.h
+++ b/lib/eal/include/generic/rte_cpuflags.h
@@ -44,8 +44,12 @@ struct rte_cpu_intrinsics {
/**
* Enumeration of all CPU features supported
*/
+#ifndef RTE_TOOLCHAIN_MSVC
__extension__
-enum rte_cpu_flag_t;
+typedef enum rte_cpu_flag_t rte_cpu_flag_t;
+#else
+typedef int rte_cpu_flag_t;
+#endif
Just curious what exactly MSVC doesn't support here?
Is that construction like:
enum rte_cpu_flag_t {....};
enum rte_cpu_flag_t;
...
Or something else?
Forward declaration of an enum is non standard. It's probably only
allowed by gcc as an extension because gcc will make some kind of
implementation specific promise for it always to be `int` size by
default (assuming no other -foptions).
I understood that part, what is not clear to me from where we are
getting forward declarations?
i investigated and expirmented with this further. i can see now there
were two things that were tripping things up portability wise.
* it's still a forward declaration even if the full enum definition is
visible and thus generates a warning (but not an error).
* the use of __extension__ which is valid in this context because it is
after all an extension (causes compilation error for msvc).
my testing shows i'm still getting correct sizes (because the definition
is visible) so what i'll do is update this patch to remove the use of
__extension__ and allow the warning to be emitted for now. i think this
is probably what you're preference would be.
in a future series i will either suppress the warning for msvc or remove
the declaration entirely which as you point out should not be needed due
to the include via arch/rte_cpuflags.h -> include generic/rte_cpuflags.h.
Agree, let's try to remove this declaration.
I don't see any point to have declaration straight after defintion.
Thanks
Konstantin
thanks!
As I remember the usual organization of arch specific rte_cpuflags.h:
/* type definition */
enum rte_cpu_flag_t {...};
/some other stuff */
#include "generic/rte_cpuflags.h"
Which contains 'enum rte_cpu_flag_t' type declaration.
But it doesn't look like a forward declaration to me.
Is there a place where we do include "generic/rte_cpuflags.h" directly
(not from arch specific header)?
If so. might be we should change it to include arch specific header instead?
If the enum was defined before reference it would probably be accepted
by msvc since it could 'see' the definition and know the integer width
in use.
/**
* Get name of CPU flag
@@ -56,9 +60,8 @@ struct rte_cpu_intrinsics {
* flag name
* NULL if flag ID is invalid
*/
-__extension__
const char *
-rte_cpu_get_flag_name(enum rte_cpu_flag_t feature);
+rte_cpu_get_flag_name(rte_cpu_flag_t feature);
/**
* Function for checking a CPU flag availability
@@ -70,9 +73,8 @@ struct rte_cpu_intrinsics {
* 0 if flag is not available
* -ENOENT if flag is invalid
*/
-__extension__
int
-rte_cpu_get_flag_enabled(enum rte_cpu_flag_t feature);
+rte_cpu_get_flag_enabled(rte_cpu_flag_t feature);
/**
* This function checks that the currently used CPU supports the CPU features
