Avoid warnings in threads.c when building without threads

Avoid warnings in threads.c when building without threads

[Eli Zaretskii]

     CC       libguile_2.0_la-threads.lo
   In file included from ../libguile/threads.h:40:0,
		    from ../libguile/gc.h:30,
		    from ../libguile/_scm.h:76,
		    from threads.c:28:
   threads.c: In function 'scm_call_with_new_thread':
   ../libguile/null-threads.h:74:53: warning: right-hand operand of comma expression has no effect [-Wunused-value]
    #define scm_i_pthread_cond_wait(c,m)        (abort(), 0)
							^
   ../libguile/null-threads.h:102:45: note: in expansion of macro 'scm_i_pthread_cond_wait'
    #define scm_i_scm_pthread_cond_wait         scm_i_pthread_cond_wait
						^
   threads.c:1061:5: note: in expansion of macro 'scm_i_scm_pthread_cond_wait'
	scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
	^

The patch to shut up these warnings is below.  OK to commit?

--- libguile/null-threads.h~0	2016-01-02 13:32:40.000000000 +0200
+++ libguile/null-threads.h	2016-07-15 17:47:37.101375000 +0300
@@ -43,7 +43,7 @@
 #define scm_i_pthread_create(t,a,f,d)       (*(t)=0, (void)(f), ENOSYS)
 #define scm_i_pthread_detach(t)             do { } while (0)
 #define scm_i_pthread_exit(v)               exit (EXIT_SUCCESS)
-#define scm_i_pthread_cancel(t)             0
+#define scm_i_pthread_cancel(t)             (void)0
 #define scm_i_pthread_cleanup_push(t,v)     0
 #define scm_i_pthread_cleanup_pop(e)        0
 #define scm_i_sched_yield()                 0


--- libguile/threads.c~	2016-06-20 23:35:06.000000000 +0300
+++ libguile/threads.c	2016-07-15 17:48:20.757625000 +0300
@@ -1058,7 +1058,7 @@ SCM_DEFINE (scm_call_with_new_thread, "c
     }
 
   while (scm_is_false (data.thread))
-    scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
+    (void)scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
 
   scm_i_pthread_mutex_unlock (&data.mutex);
 
@@ -1138,7 +1138,7 @@ scm_spawn_thread (scm_t_catch_body body,
     }
 
   while (scm_is_false (data.thread))
-    scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
+    (void)scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
 
   scm_i_pthread_mutex_unlock (&data.mutex);
 

Re: Avoid warnings in threads.c when building without threads

[Eli Zaretskii]

Ping!

> Date: Sat, 16 Jul 2016 20:12:22 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
>      CC       libguile_2.0_la-threads.lo
>    In file included from ../libguile/threads.h:40:0,
> 		    from ../libguile/gc.h:30,
> 		    from ../libguile/_scm.h:76,
> 		    from threads.c:28:
>    threads.c: In function 'scm_call_with_new_thread':
>    ../libguile/null-threads.h:74:53: warning: right-hand operand of comma expression has no effect [-Wunused-value]
>     #define scm_i_pthread_cond_wait(c,m)        (abort(), 0)
> 							^
>    ../libguile/null-threads.h:102:45: note: in expansion of macro 'scm_i_pthread_cond_wait'
>     #define scm_i_scm_pthread_cond_wait         scm_i_pthread_cond_wait
> 						^
>    threads.c:1061:5: note: in expansion of macro 'scm_i_scm_pthread_cond_wait'
> 	scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
> 	^
> 
> The patch to shut up these warnings is below.  OK to commit?
> 
> --- libguile/null-threads.h~0	2016-01-02 13:32:40.000000000 +0200
> +++ libguile/null-threads.h	2016-07-15 17:47:37.101375000 +0300
> @@ -43,7 +43,7 @@
>  #define scm_i_pthread_create(t,a,f,d)       (*(t)=0, (void)(f), ENOSYS)
>  #define scm_i_pthread_detach(t)             do { } while (0)
>  #define scm_i_pthread_exit(v)               exit (EXIT_SUCCESS)
> -#define scm_i_pthread_cancel(t)             0
> +#define scm_i_pthread_cancel(t)             (void)0
>  #define scm_i_pthread_cleanup_push(t,v)     0
>  #define scm_i_pthread_cleanup_pop(e)        0
>  #define scm_i_sched_yield()                 0
> 
> 
> --- libguile/threads.c~	2016-06-20 23:35:06.000000000 +0300
> +++ libguile/threads.c	2016-07-15 17:48:20.757625000 +0300
> @@ -1058,7 +1058,7 @@ SCM_DEFINE (scm_call_with_new_thread, "c
>      }
>  
>    while (scm_is_false (data.thread))
> -    scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
> +    (void)scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
>  
>    scm_i_pthread_mutex_unlock (&data.mutex);
>  
> @@ -1138,7 +1138,7 @@ scm_spawn_thread (scm_t_catch_body body,
>      }
>  
>    while (scm_is_false (data.thread))
> -    scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
> +    (void)scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
>  
>    scm_i_pthread_mutex_unlock (&data.mutex);
>  
> 
> 

Re: Avoid warnings in threads.c when building without threads

[Andy Wingo]

On Sat 16 Jul 2016 19:12, Eli Zaretskii <eliz@gnu.org> writes:

> The patch to shut up these warnings is below.  OK to commit?
>
> --- libguile/null-threads.h~0	2016-01-02 13:32:40.000000000 +0200
> +++ libguile/null-threads.h	2016-07-15 17:47:37.101375000 +0300
> @@ -43,7 +43,7 @@
>  #define scm_i_pthread_create(t,a,f,d)       (*(t)=0, (void)(f), ENOSYS)
>  #define scm_i_pthread_detach(t)             do { } while (0)
>  #define scm_i_pthread_exit(v)               exit (EXIT_SUCCESS)
> -#define scm_i_pthread_cancel(t)             0
> +#define scm_i_pthread_cancel(t)             (void)0
>  #define scm_i_pthread_cleanup_push(t,v)     0
>  #define scm_i_pthread_cleanup_pop(e)        0
>  #define scm_i_sched_yield()                 0

I think not, sorry :/  pthread_cancel returns an int, so
scm_i_pthread_cancel should always return an int.

> --- libguile/threads.c~	2016-06-20 23:35:06.000000000 +0300
> +++ libguile/threads.c	2016-07-15 17:48:20.757625000 +0300
> @@ -1058,7 +1058,7 @@ SCM_DEFINE (scm_call_with_new_thread, "c
>      }
>  
>    while (scm_is_false (data.thread))
> -    scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
> +    (void)scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
>  
>    scm_i_pthread_mutex_unlock (&data.mutex);
>  

Likewise this is not needed -- the problem is in the cond_wait
definition.

null-threads.h is a distressing header file.  I think the right thing to
do is to use typedefs and static inline functions instead of CPP macros.
That way we keep type-safety, and also the compiler will stop
complaining.

Andy

Re: Avoid warnings in threads.c when building without threads

[Eli Zaretskii]

> From: Andy Wingo <wingo@pobox.com>
> Cc: guile-devel@gnu.org
> Date: Sat, 23 Jul 2016 22:49:03 +0200
> 
> On Sat 16 Jul 2016 19:12, Eli Zaretskii <eliz@gnu.org> writes:
> 
> > The patch to shut up these warnings is below.  OK to commit?
> >
> > --- libguile/null-threads.h~0	2016-01-02 13:32:40.000000000 +0200
> > +++ libguile/null-threads.h	2016-07-15 17:47:37.101375000 +0300
> > @@ -43,7 +43,7 @@
> >  #define scm_i_pthread_create(t,a,f,d)       (*(t)=0, (void)(f), ENOSYS)
> >  #define scm_i_pthread_detach(t)             do { } while (0)
> >  #define scm_i_pthread_exit(v)               exit (EXIT_SUCCESS)
> > -#define scm_i_pthread_cancel(t)             0
> > +#define scm_i_pthread_cancel(t)             (void)0
> >  #define scm_i_pthread_cleanup_push(t,v)     0
> >  #define scm_i_pthread_cleanup_pop(e)        0
> >  #define scm_i_sched_yield()                 0
> 
> I think not, sorry :/  pthread_cancel returns an int, so
> scm_i_pthread_cancel should always return an int.

Then code which ignores the value should cast to void.

> > --- libguile/threads.c~	2016-06-20 23:35:06.000000000 +0300
> > +++ libguile/threads.c	2016-07-15 17:48:20.757625000 +0300
> > @@ -1058,7 +1058,7 @@ SCM_DEFINE (scm_call_with_new_thread, "c
> >      }
> >  
> >    while (scm_is_false (data.thread))
> > -    scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
> > +    (void)scm_i_scm_pthread_cond_wait (&data.cond, &data.mutex);
> >  
> >    scm_i_pthread_mutex_unlock (&data.mutex);
> >  
> 
> Likewise this is not needed -- the problem is in the cond_wait
> definition.

But the above is a valid code when the returned value is being
ignored, so why not use it?

> null-threads.h is a distressing header file.  I think the right thing to
> do is to use typedefs and static inline functions instead of CPP macros.
> That way we keep type-safety, and also the compiler will stop
> complaining.

I'm okay with any solution that will shut up the warnings, because
they distract and mask real problems.

TIA

Re: Avoid warnings in threads.c when building without threads

[Andy Wingo]

On Sun 24 Jul 2016 04:38, Eli Zaretskii <eliz@gnu.org> writes:

>> > --- libguile/null-threads.h~0	2016-01-02 13:32:40.000000000 +0200
>> > +++ libguile/null-threads.h	2016-07-15 17:47:37.101375000 +0300
>> > @@ -43,7 +43,7 @@
>> >  #define scm_i_pthread_create(t,a,f,d)       (*(t)=0, (void)(f), ENOSYS)
>> >  #define scm_i_pthread_detach(t)             do { } while (0)
>> >  #define scm_i_pthread_exit(v)               exit (EXIT_SUCCESS)
>> > -#define scm_i_pthread_cancel(t)             0
>> > +#define scm_i_pthread_cancel(t)             (void)0
>> >  #define scm_i_pthread_cleanup_push(t,v)     0
>> >  #define scm_i_pthread_cleanup_pop(e)        0
>> >  #define scm_i_sched_yield()                 0
>> 
>> I think not, sorry :/  pthread_cancel returns an int, so
>> scm_i_pthread_cancel should always return an int.
>
> Then code which ignores the value should cast to void.

That way leads to madness :) There are many function call sites in Guile
that ignore the function return values.  GCC does not warn about them,
and rightly so.  The problem here is that the "null" definition of
scm_i_pthread_cancel is a bare expression and for some reason GCC is
treating this case differently.  The solution AFAIU is to make the null
definition of scm_i_pthread_cancel into a function so that it is more
like the proper definition.  That way not only will GCC not warn, but it
will also do type checking for us.

Applied a fix along these lines; a --without-threads configuration
builds without warnings and passes all tests for me now.

Andy