Re: Cascaded casts

[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: 
http://caeis.etech.fh-augsburg.de:8010/#/builders/4/builds/26
     
     On debian-buster the compiler is gcc8 while on sid the compiler is gcc10.
     
     GFunc is defined with two parameters: 
https://developer.gnome.org/glib/stable/glib-Doubly-Linked-Lists.html#GFunc
     
     while gtk_tree_path_free has only one:
     
     
https://developer.gnome.org/gtk3/stable/GtkTreeModel.html#gtk-tree-path-free
     
     I guess it is the same behaviour that resulted in this macro
     
     
https://developer.gnome.org/glib/stable/glib-The-Main-Event-Loop.html#G-SOURCE-FUNC:CAPS
     
     A discussion about this check is here:
     
     https://gitlab.gnome.org/GNOME/gnome-terminal/-/issues/96
     
     resulting in this cast. Root cause seems to be that the behaviour for 
different number of arguments is undefined.
     
     http://c0x.coding-guidelines.com/6.5.2.2.html
     
     The clang compiler version 9 does not issue this warning.
     
     Fritz
     
     > Am 25.08.2020 um 07:00 schrieb John Darrington 
<john@darrington.wattle.id.au>:
     > 
     > I don't understand this commit, and I think it just makes the code
     > harder to read:
     > 
     > 
     >    commit ceaed4a17cb3b0a14c89f10b72a636f94af97e7a
     >    Author: Friedrich Beckmann <friedrich.beckmann@gmx.de>
     >    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'
     > 
     >