[v2,3/3] clang: don't define nocall
diff mbox series

Message ID 20200528144023.10814-4-roger.pau@citrix.com
State New
Headers show
Series
  • Clang/LLVM build fixes
Related show

Commit Message

Roger Pau Monné May 28, 2020, 2:40 p.m. UTC
Clang doesn't support attribute error, and the possible equivalents
like diagnose_if don't seem to work well in this case as they trigger
when when the function is not called (just by being used by the
APPEND_CALL macro).

Define nocall to a noop on clang until a proper solution can be found.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/include/xen/compiler.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Comments

Julien Grall May 29, 2020, 7:25 a.m. UTC | #1
Hi Roger,

On 28/05/2020 15:40, Roger Pau Monne wrote:
> Clang doesn't support attribute error, and the possible equivalents
> like diagnose_if don't seem to work well in this case as they trigger
> when when the function is not called (just by being used by the
> APPEND_CALL macro).

OOI, could you share the diagnose_if change you tried?

> 
> Define nocall to a noop on clang until a proper solution can be found.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Acked-by: Julien Grall <jgrall@amazon.com>

> ---
>   xen/include/xen/compiler.h | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
> index c22439b7a4..225e09e5f7 100644
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -20,7 +20,11 @@
>   
>   #define __weak        __attribute__((__weak__))
>   
> -#define nocall        __attribute__((error("Nonstandard ABI")))
> +#if !defined(__clang__)
> +# define nocall        __attribute__((error("Nonstandard ABI")))
> +#else
> +# define nocall
> +#endif
>   
>   #if (!defined(__clang__) && (__GNUC__ == 4) && (__GNUC_MINOR__ < 5))
>   #define unreachable() do {} while (1)
> 

Cheers,
Roger Pau Monné May 29, 2020, 8:31 a.m. UTC | #2
On Fri, May 29, 2020 at 08:25:44AM +0100, Julien Grall wrote:
> Hi Roger,
> 
> On 28/05/2020 15:40, Roger Pau Monne wrote:
> > Clang doesn't support attribute error, and the possible equivalents
> > like diagnose_if don't seem to work well in this case as they trigger
> > when when the function is not called (just by being used by the
> > APPEND_CALL macro).
> 
> OOI, could you share the diagnose_if change you tried?

I've sent a reduced version to the llvm-dev mailing list, because I
think the documentation should be fixed for diagnose_if. The email
with the example is at:

http://lists.llvm.org/pipermail/llvm-dev/2020-May/141908.html

FWIW, using the deprecated attribute will also trigger the same
error/warning even when not calling the function from C.

> > 
> > Define nocall to a noop on clang until a proper solution can be found.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Acked-by: Julien Grall <jgrall@amazon.com>

Thanks!
Julien Grall May 29, 2020, 12:36 p.m. UTC | #3
Hi,

On 29/05/2020 09:31, Roger Pau Monné wrote:
> On Fri, May 29, 2020 at 08:25:44AM +0100, Julien Grall wrote:
>> Hi Roger,
>>
>> On 28/05/2020 15:40, Roger Pau Monne wrote:
>>> Clang doesn't support attribute error, and the possible equivalents
>>> like diagnose_if don't seem to work well in this case as they trigger
>>> when when the function is not called (just by being used by the
>>> APPEND_CALL macro).
>>
>> OOI, could you share the diagnose_if change you tried?
> 
> I've sent a reduced version to the llvm-dev mailing list, because I
> think the documentation should be fixed for diagnose_if. The email
> with the example is at:
> 
> http://lists.llvm.org/pipermail/llvm-dev/2020-May/141908.html

Thanks!

> 
> FWIW, using the deprecated attribute will also trigger the same
> error/warning even when not calling the function from C.

I read the documentation before asking the code and I did wonder why 
diagnose_if wouldn't work.

I am guessing the behavior wasn't intended.

Cheers,
Jan Beulich May 29, 2020, 2:49 p.m. UTC | #4
On 28.05.2020 16:40, Roger Pau Monne wrote:
> --- a/xen/include/xen/compiler.h
> +++ b/xen/include/xen/compiler.h
> @@ -20,7 +20,11 @@
>  
>  #define __weak        __attribute__((__weak__))
>  
> -#define nocall        __attribute__((error("Nonstandard ABI")))
> +#if !defined(__clang__)
> +# define nocall        __attribute__((error("Nonstandard ABI")))

I think a blank wants dropping from here. I also should have noticed
already on Andrew's putting in of this construct that in a header
we'd better use __error__. Both can easily be taken care of while
committing.

Jan

Patch
diff mbox series

diff --git a/xen/include/xen/compiler.h b/xen/include/xen/compiler.h
index c22439b7a4..225e09e5f7 100644
--- a/xen/include/xen/compiler.h
+++ b/xen/include/xen/compiler.h
@@ -20,7 +20,11 @@ 
 
 #define __weak        __attribute__((__weak__))
 
-#define nocall        __attribute__((error("Nonstandard ABI")))
+#if !defined(__clang__)
+# define nocall        __attribute__((error("Nonstandard ABI")))
+#else
+# define nocall
+#endif
 
 #if (!defined(__clang__) && (__GNUC__ == 4) && (__GNUC_MINOR__ < 5))
 #define unreachable() do {} while (1)