Re: WINECFG: Eliminate some gcc 4.1 warnings caused by casts in macros.

[Francois Gouget]

On Fri, 3 Mar 2006, Mike McCormack wrote:
> ChangeLog:
> Eliminate some gcc 4.1 warnings caused by casts in macros.

What are these warnings? What is causing them?
> - TreeView_SetImageList(tree, hImageList, TVSIL_STATE);
> - SendMessageW( tree, LVM_SETIMAGELIST, TVSIL_STATE, (LPARAM)hImageList );

Do we really have to give up on the macros and use SendMessage directly 
to avoid the warnings? If we do, then I would say this means the macros 
are broken because it means Winelib users will get the same problem we 
do and will have to modify their code or live with gobs of warnings.

So, is there a way to fix the macros?

WINECFG: Eliminate some gcc 4.1 warnings caused by casts in macros.

[Mike McCormack]

Francois Gouget wrote:
> What are these warnings? What is causing them?

Sorry, should have posted an explanation.  The warning comes from code 
like this:

char foo(x);
#define bar(x) (int) foo(x)

So for the example you gave from my patch:

#define Header_SetImageList(hwnd,himl) \
   (HIMAGELIST)SNDMSGA((hwnd),HDM_SETIMAGELIST,0,(LPARAM)himl)

We get a "computed value is not used" warning when we use 
Header_SetImageList without assigning the returned value to anything.

I reported this in the gcc bugzilla some time ago, but the gcc team 
claim that this is a feature, not a bug:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25955

They would prefer that we use statement expressions, which are gcc 
specific, in our macros.  Something like:

#define Header_SetImageList(hwnd,himl) \
   ({ HIMAGELIST r; \
      r = (HIMAGELIST)SNDMSGA((hwnd),HDM_SETIMAGELIST,0,(LPARAM)himl); \
      r; })

Alexandre suggested that we use inline functions to solve this problem, 
however this requires including winuser.h when including windowsx.h, and 
that we choose SendMessageA or SendMessageW for the macros that deal 
with strings.

Conceivably we could use:

#ifdef HAVE_BORKED_GCC_COMPUTED_VALUE_NOT_USED_WARNING

#define Header_SetImageList(hwnd,himl) \
   ({ HIMAGELIST r; \
     r = (HIMAGELIST)SNDMSGA((hwnd),HDM_SETIMAGELIST,0,(LPARAM)himl); \
     r; })

#else

#define Header_SetImageList(hwnd,himl) \
   (HIMAGELIST)SNDMSGA((hwnd),HDM_SETIMAGELIST,0,(LPARAM)himl)

#endif

This requires use -DHAVE_BORKED_GCC_COMPUTED_VALUE_NOT_USED_WARNING when 
compiling Wine code and Winelib apps.
> Do we really have to give up on the macros and use SendMessage directly 
> to avoid the warnings? If we do, then I would say this means the macros 
> are broken because it means Winelib users will get the same problem we 
> do and will have to modify their code or live with gobs of warnings.

Yes, and it will be broken for WineLib.  I quote Alexandre from IRC:

<julliard> mike_m: hmm right, that's a PITA
<mike_m> truely
<julliard> i'd say get rid of windowsx.h then

I agree this is not an ideal solution... let me know if you have any 
better ideas.

Mike

WINECFG: Eliminate some gcc 4.1 warnings caused by casts in macros.

[Francois Gouget]

On Fri, 3 Mar 2006, Mike McCormack wrote:
[...]
> They would prefer that we use statement expressions, which are gcc specific, 
> in our macros.  Something like:

Thanks for the explanation.
Sure, having an #ifdef/#else/#endif for each macro is way too painful. 
But what about the following:

+#ifndef WINE_CAST
+# if __GNUC__
+#  define WINE_CAST(ctype, val) ({ctype r=(ctype)(val);r;})
+# else
+#  define WINE_CAST(ctype, val) ((ctype)(val))
+# endif
+#endif
+
+ #define Header_SetImageList(hwnd,himl) \
-  (HIMAGELIST)SNDMSGA((hwnd),HDM_SETIMAGELIST,0,(LPARAM)himl)
+    WINE_CAST(HIMAGELIST, SNDMSGA((hwnd),HDM_SETIMAGELIST,0,(LPARAM)himl))
+

This pre-processes to code that uses the statement expression on gcc 
compilers and to the regular cast for others. So there should be no 
warning in either case (I couldn't check with gcc 4.1 but this 
preprocesses to the expected form). I also checked with gcc 2.95 and it 
compiles fine and without warning there which should be enough backwards 
compatibility for everyone (otherwise we can go further and check the 
GCC version).

Of course in a real patch the WINE_CAST macro would be declared 
somewhere in windef.h, and we'd still have to change all the macros that 
cast a function result. But that seems quite feasible and not too ugly 
and I would be willing to submit such a patch. We may also want to find 
a better name. WINE_MACRO_CAST? WINE_MCAST?  __MACRO_CAST? WINE_CAST_RESULT?

Should I send such a patch?