[PATCH] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[PATCH] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Michael Barker]

Signed-off-by: Michael Barker <mikeb01@gmail.com>
---
 lib/eal/include/rte_compat.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index 2718612cce..9556bbf4d0 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -33,8 +33,11 @@ section(".text.internal")))
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
 #define __rte_internal \
+_Pragma("GCC diagnostic push") \
+_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
-section(".text.internal")))
+section(".text.internal"))) \
+_Pragma("GCC diagnostic pop")
 
 #else
 
-- 
2.25.1

[PATCH v2] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Michael Barker]

When using clang with -Wall the use of diagnose_if kicks up a warning,
requiring all dpdk includes to be wrapped with the pragma.  This change
isolates the ignore just the appropriate location and makes it easier
for users to apply -Wall,-Werror

Signed-off-by: Michael Barker <mikeb01@gmail.com>
---
 lib/eal/include/rte_compat.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index 2718612cce..9556bbf4d0 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -33,8 +33,11 @@ section(".text.internal")))
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
 #define __rte_internal \
+_Pragma("GCC diagnostic push") \
+_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
-section(".text.internal")))
+section(".text.internal"))) \
+_Pragma("GCC diagnostic pop")
 
 #else
 
-- 
2.25.1

Re: [PATCH v2] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Thomas Monjalon]

18/01/2022 00:23, Michael Barker:
> When using clang with -Wall the use of diagnose_if kicks up a warning,

Please could you copy the warning in the commit log?

> requiring all dpdk includes to be wrapped with the pragma.  This change
> isolates the ignore just the appropriate location and makes it easier
> for users to apply -Wall,-Werror

Please could you explain how it is related to -Wgcc-compat?

[...]
>  #define __rte_internal \
> +_Pragma("GCC diagnostic push") \
> +_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
>  __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
> -section(".text.internal")))
> +section(".text.internal"))) \
> +_Pragma("GCC diagnostic pop")

[PATCH v3] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Michael Barker]

When compiling with clang using -Wall (or -Wgcc-compat) the use of diagnose_if kicks up a warning:

.../include/rte_interrupts.h:623:1: error: 'diagnose_if' is a clang extension [-Werror,-Wgcc-compat]
__rte_internal
^
.../include/rte_compat.h:36:16: note: expanded from macro '__rte_internal'
__attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \

This change ignores the '-Wgcc-compat' warning in the specific location
where the warning occurs.  It is safe to do in this circumstance as the
specific macro is only defined when using the clang compiler.

Signed-off-by: Michael Barker <mikeb01@gmail.com>
---
 lib/eal/include/rte_compat.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index 2718612cce..9556bbf4d0 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -33,8 +33,11 @@ section(".text.internal")))
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
 #define __rte_internal \
+_Pragma("GCC diagnostic push") \
+_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
-section(".text.internal")))
+section(".text.internal"))) \
+_Pragma("GCC diagnostic pop")
 
 #else
 
-- 
2.25.1

Re: [PATCH v2] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Michael Barker]

[-- Attachment #1: Type: text/plain, Size: 1673 bytes --]

On Fri, 21 Jan 2022 at 03:16, Thomas Monjalon <thomas@monjalon.net> wrote:

> 18/01/2022 00:23, Michael Barker:
> > When using clang with -Wall the use of diagnose_if kicks up a warning,
>
> Please could you copy the warning in the commit log?
>

I've updated the commit log to be more descriptive (and included the
associated warning).

> requiring all dpdk includes to be wrapped with the pragma.  This change
> > isolates the ignore just the appropriate location and makes it easier
> > for users to apply -Wall,-Werror
>
> Please could you explain how it is related to -Wgcc-compat?
>

I'm currently working on some code that makes use of DPDK, which is built
with '-Wall,-Werror' enabled.  When using the clang toolchain the build
fails as a result of this macro that this patch updates.  The workaround
from my application is to wrap all of the DPDK header includes in pragma to
disable the warnings (see below).  This has the unfortunate side effect of
disabling this warning across all of the included DPDK headers, which is
not ideal.  Hence the reason to submit the patch which disables the warning
just in the location where it occurs.

#if defined(__clang__)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wgcc-compat"
#endif
#include <rte_ethdev.h>
#if defined(__clang__)
#pragma GCC diagnostic pop "-Wgcc-compat"
#endif



>
> [...]
> >  #define __rte_internal \
> > +_Pragma("GCC diagnostic push") \
> > +_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
> >  __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
> > -section(".text.internal")))
> > +section(".text.internal"))) \
> > +_Pragma("GCC diagnostic pop")
>
>
>
>

[-- Attachment #2: Type: text/html, Size: 3549 bytes --]

[PATCH v4] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Michael Barker]

When compiling with clang using -Wall (or -Wgcc-compat) the use of
diagnose_if kicks up a warning:

.../include/rte_interrupts.h:623:1: error: 'diagnose_if' is a clang
extension [-Werror,-Wgcc-compat]
__rte_internal
^
.../include/rte_compat.h:36:16: note: expanded from macro '__rte_internal'
__attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \

This change ignores the '-Wgcc-compat' warning in the specific location
where the warning occurs.  It is safe to do in this circumstance as the
specific macro is only defined when using the clang compiler.

Signed-off-by: Michael Barker <mikeb01@gmail.com>
---
 lib/eal/include/rte_compat.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index 2718612cce..9556bbf4d0 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -33,8 +33,11 @@ section(".text.internal")))
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
 #define __rte_internal \
+_Pragma("GCC diagnostic push") \
+_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
-section(".text.internal")))
+section(".text.internal"))) \
+_Pragma("GCC diagnostic pop")
 
 #else
 
-- 
2.25.1

Re: [PATCH v2] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Stephen Hemminger]

On Mon, 24 Jan 2022 10:17:37 +1300
Michael Barker <mikeb01@gmail.com> wrote:

> On Fri, 21 Jan 2022 at 03:16, Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > 18/01/2022 00:23, Michael Barker:  
> > > When using clang with -Wall the use of diagnose_if kicks up a warning,  
> >
> > Please could you copy the warning in the commit log?
> >  
> 
> I've updated the commit log to be more descriptive (and included the
> associated warning).
> 
> > requiring all dpdk includes to be wrapped with the pragma.  This change  
> > > isolates the ignore just the appropriate location and makes it easier
> > > for users to apply -Wall,-Werror  
> >
> > Please could you explain how it is related to -Wgcc-compat?
> >  
> 
> I'm currently working on some code that makes use of DPDK, which is built
> with '-Wall,-Werror' enabled.  When using the clang toolchain the build
> fails as a result of this macro that this patch updates.  The workaround
> from my application is to wrap all of the DPDK header includes in pragma to
> disable the warnings (see below).  This has the unfortunate side effect of
> disabling this warning across all of the included DPDK headers, which is
> not ideal.  Hence the reason to submit the patch which disables the warning
> just in the location where it occurs.
> 

Fix the issue please, don't suppress it.

Re: [PATCH v4] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Stephen Hemminger]

On Mon, 24 Jan 2022 10:20:24 +1300
Michael Barker <mikeb01@gmail.com> wrote:

> When compiling with clang using -Wall (or -Wgcc-compat) the use of
> diagnose_if kicks up a warning:
> 
> .../include/rte_interrupts.h:623:1: error: 'diagnose_if' is a clang
> extension [-Werror,-Wgcc-compat]
> __rte_internal
> ^

Which clang version is this?
Perhaps the allow internal API could use a different attribute that
could work in both cases?

Re: [PATCH v4] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Ray Kinsella]

Michael Barker <mikeb01@gmail.com> writes:

> When compiling with clang using -Wall (or -Wgcc-compat) the use of
> diagnose_if kicks up a warning:
>
> .../include/rte_interrupts.h:623:1: error: 'diagnose_if' is a clang
> extension [-Werror,-Wgcc-compat]
> __rte_internal
> ^
> .../include/rte_compat.h:36:16: note: expanded from macro '__rte_internal'
> __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
>
> This change ignores the '-Wgcc-compat' warning in the specific location
> where the warning occurs.  It is safe to do in this circumstance as the
> specific macro is only defined when using the clang compiler.
>
> Signed-off-by: Michael Barker <mikeb01@gmail.com>
> ---
>  lib/eal/include/rte_compat.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
> index 2718612cce..9556bbf4d0 100644
> --- a/lib/eal/include/rte_compat.h
> +++ b/lib/eal/include/rte_compat.h
> @@ -33,8 +33,11 @@ section(".text.internal")))
>  #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /*
> For clang */

Why doesn't the __has_attribute take care of this?
I would have thought that gcc would check the for the attribute, find it
doesn't support it and ignore the whole thing?

>  
>  #define __rte_internal \
> +_Pragma("GCC diagnostic push") \
> +_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
>  __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
> -section(".text.internal")))
> +section(".text.internal"))) \
> +_Pragma("GCC diagnostic pop")
>  
>  #else


-- 
Regards, Ray K

[PATCH v5] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Michael Barker]

When compiling with clang using -Wpedantic (or -Wgcc-compat) the use of
diagnose_if kicks up a warning:

.../include/rte_interrupts.h:623:1: error: 'diagnose_if' is a clang
extension [-Werror,-Wgcc-compat]
__rte_internal
^
.../include/rte_compat.h:36:16: note: expanded from macro '__rte_internal'
__attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \

This change ignores the '-Wgcc-compat' warning in the specific location
where the warning occurs.  It is safe to do in this circumstance as the
specific macro is only defined when using the clang compiler.

Signed-off-by: Michael Barker <mikeb01@gmail.com>
---
 lib/eal/include/rte_compat.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/eal/include/rte_compat.h b/lib/eal/include/rte_compat.h
index 2718612cce..9556bbf4d0 100644
--- a/lib/eal/include/rte_compat.h
+++ b/lib/eal/include/rte_compat.h
@@ -33,8 +33,11 @@ section(".text.internal")))
 #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /* For clang */
 
 #define __rte_internal \
+_Pragma("GCC diagnostic push") \
+_Pragma("GCC diagnostic ignored \"-Wgcc-compat\"") \
 __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
-section(".text.internal")))
+section(".text.internal"))) \
+_Pragma("GCC diagnostic pop")
 
 #else
 
-- 
2.25.1

Re: [PATCH v4] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Michael Barker]

[-- Attachment #1: Type: text/plain, Size: 703 bytes --]

> > extension [-Werror,-Wgcc-compat]
> > __rte_internal
> > ^
>
> Which clang version is this?
>

Clang 10, 11, 12 and 13.


> Perhaps the allow internal API could use a different attribute that
> could work in both cases?
>

I've realised I've made a small error. It is not -Wall that includes this
particular check, but -Wpendantic.  I'll update the commit message.

However I can't find an attribute that is cross platform.  There is the
#error directive, but it does not work as an attribute.  I've also tried
adding `#if __clang__` around the macro as well, but the warning still gets
triggered by clang.

If there are any other options that I've missed, let me know and I can try
them out.

Mike.

[-- Attachment #2: Type: text/html, Size: 1663 bytes --]

Re: [PATCH v4] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Michael Barker]

[-- Attachment #1: Type: text/plain, Size: 560 bytes --]

>
> > +++ b/lib/eal/include/rte_compat.h
> > @@ -33,8 +33,11 @@ section(".text.internal")))
> >  #elif !defined ALLOW_INTERNAL_API && __has_attribute(diagnose_if) /*
> > For clang */
>
> Why doesn't the __has_attribute take care of this?
> I would have thought that gcc would check the for the attribute, find it
> doesn't support it and ignore the whole thing?
>

It appears that the '-Wgcc-compat' check is too naive and doesn't pick up
the `__has_attribute` or `#if __clang__` and realise that there isn't
really a compatibility issue with the code.

Mike.

[-- Attachment #2: Type: text/html, Size: 1018 bytes --]

Re: [PATCH v5] Add pragma to ignore gcc-compat warnings in clang when used with diagnose_if.

[Thomas Monjalon]

31/01/2022 01:05, Michael Barker:
> When compiling with clang using -Wpedantic (or -Wgcc-compat) the use of
> diagnose_if kicks up a warning:
> 
> .../include/rte_interrupts.h:623:1: error: 'diagnose_if' is a clang
> extension [-Werror,-Wgcc-compat]
> __rte_internal
> ^
> .../include/rte_compat.h:36:16: note: expanded from macro '__rte_internal'
> __attribute__((diagnose_if(1, "Symbol is not public ABI", "error"), \
> 
> This change ignores the '-Wgcc-compat' warning in the specific location
> where the warning occurs.  It is safe to do in this circumstance as the
> specific macro is only defined when using the clang compiler.
> 
> Signed-off-by: Michael Barker <mikeb01@gmail.com>

Applied with following title, thanks:
	eal: ignore gcc-compat warning in clang-only macro