[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Cascaded casts

From: Friedrich Beckmann
Subject: Re: Cascaded casts
Date: Tue, 25 Aug 2020 19:56:39 +0200

Hi John,

yes, I changed the default compiler warning settings with this patch

The patch applies the AM_CFLAGS settings also for the psppire gui code
but with Wunused-parameter disabled. Before that the
gui code was only compiled with Wall.

AM_CFLAGS enables -Wall and -Wextra as default and disables Wsign-compare (-W 
is the old word for -Wextra)

In addition I disabled the Wunused-parameter for the gui because the gui 
callbacks result in
many warnings - for the core code this is not the case. I also had the 
that with the enabled settings some problems showed up as you mention below.

With the previous patches we are down to these warnings on amd64:

For i386 there are two more.

Maybe Ben can have a look at the enum casts:

As far as I understood the code this are intentional „extensions“:

An alternative to the function casts for g_list_foreach that I did in the first 
place is to create a new function „free_tree“ with exactly two parameters which 
then calls gtk_tree_path_free. That free_tree function is then used in 
g_list_foreach. But I found that difficult to read due to the additional 
function call level. Therefore I did this cast which relies on this „undefined“ 
compiler behaviour. Would you prefer this alternative?

When I run configure with CFLAGS=„-Wall -Wextra -Wunused-parameter 
-Wsign-compare“ (= full)
then I get the following warnings:

982     sign-compare
620     unused-parameter
18      deprecated-declarations
15      unused-value
6       unused-variable
2       pointer-arith
2       pointer-sign
1       type-limits
1646    Total

This includes the perl module and the gl functions which are both compiled 
without any warning setup per default. With current default compiler setup we 

18      deprecated-declarations
2       pointer-arith
2       maybe-uninitialized
1       return-local-addr
23      Total

I think this allows us to spot problematic issues and does not flood us with
warnings that we will never fix.


> Am 25.08.2020 um 14:21 schrieb John Darrington <>:
> Ahh! So you are building using -Wextra
> Most of these warnings do not appear (either with gcc8 or gcc10) when passing
> just -Wall.  I don't think it's realistic to expect the code to be free of
> warnings with -Wextra, and I don't think it's a good practice to add casts
> purely for the purpose of shuting the compiler up.
> The warnings I *do* see with gcc10 -Wall include the enum-conversion warnings 
> in
> data-io/placement-parser.c and output/spv/spv.c  but I don't think casts are 
> the
> right way to fix these.  Rethinking the definition of the enums and their 
> members
> would be a better way IMO, but this is Ben's code so I'll leave that decision 
> to
> him.
> The tooltip related changes are good and I'm glad you caught the switch
> fallthrough issues!  The G_DEFINE_TYPE changes are reasonable too.
> J'
> On Tue, Aug 25, 2020 at 01:16:47PM +0200, Friedrich Beckmann wrote:
>     Hi John,
>     the relevant warning without the change is here:
>     ../pspp/src/ui/gui/psppire-acr.c: In function 
> ???on_change_button_clicked???:
>     ../pspp/src/ui/gui/psppire-acr.c:186:22: warning: cast between 
> incompatible function types from ???void (*)(GtkTreePath *)??? {aka ???void 
> (*)(struct _GtkTreePath *)???} to ???void (*)(void *, void *)??? 
> [-Wcast-function-type]
>        g_list_foreach (l, (GFunc) gtk_tree_path_free, NULL);
>     You can see the full build log here: 
>     On debian-buster the compiler is gcc8 while on sid the compiler is gcc10.
>     GFunc is defined with two parameters: 
>     while gtk_tree_path_free has only one:
>     I guess it is the same behaviour that resulted in this macro
>     A discussion about this check is here:
>     resulting in this cast. Root cause seems to be that the behaviour for 
> different number of arguments is undefined.
>     The clang compiler version 9 does not issue this warning.
>     Fritz
>> Am 25.08.2020 um 07:00 schrieb John Darrington 
>> <>:
>> I don't understand this commit, and I think it just makes the code
>> harder to read:
>>   commit ceaed4a17cb3b0a14c89f10b72a636f94af97e7a
>>   Author: Friedrich Beckmann <>
>>   Date:   Mon Aug 24 11:19:33 2020 +0200
>>       Warnings: function type cast for g_list_foreach
>>       I added a cast for the functions used in g_list_foreach. Without
>>       the cast, the warning
>>       warning: cast between incompatible function types
>>       is reported.
>>   diff --git a/src/ui/gui/dialog-common.c b/src/ui/gui/dialog-common.c
>>   index f167034b1..1b29c4a3b 100644
>>   --- a/src/ui/gui/dialog-common.c
>>   +++ b/src/ui/gui/dialog-common.c
>>   @@ -112,7 +112,7 @@ homogeneous_types (GtkWidget *source, GtkWidget *dest)
>>          have_type = true;
>>        }
>>   -  g_list_foreach (list, (GFunc) gtk_tree_path_free, NULL);
>>   +  g_list_foreach (list, (GFunc) (void (*)(void)) gtk_tree_path_free, 
>> NULL);
>> gtk_tree_path_free is already cast to GFunc,  so what good does it do to 
>> first
>> cast it to something else, and _then_ to GFunc without using it?   You say 
>> that
>> something is reporting this as a "cast between incompatible types" - That is
>> what casts are for  -  to make incompatible types compatible.
>> Before, we had:
>> (void (*) (GtkTreePath *)   -> (void (*) (void *, void *))
>> Now we have:
>> (void (*) (GtkTreePath *)  -> (void (*) (void))  -> (void (*) (void *, void 
>> *))
>> What good does the intermediate cast do?  Which tool is issuing a warning?
>> J'

reply via email to

[Prev in Thread] Current Thread [Next in Thread]