Re: [Emacs-diffs] master 74a5a33: Support native image transforms on MS-Windows

Re: [Emacs-diffs] master 74a5a33: Support native image transforms on MS-Windows

[Ken Brown]

On 6/29/2019 7:55 AM, Eli Zaretskii wrote:
> branch: master
> commit 74a5a332fee8a346cf65ed6656c1b08dc1df5fde
> Author: Eli Zaretskii <eliz@gnu.org>
> Commit: Eli Zaretskii <eliz@gnu.org>
> 
>      Support native image transforms on MS-Windows

I'm seeing a few compiler warnings after this commit with GCC-7.4.0:

In file included from ../../master/src/image.c:37:0:
../../master/src/image.c: In function ‘image_set_transform’:
../../master/src/lisp.h:61:24: warning: implicit conversion from ‘float’ to 
‘double’ to match other operand of binary expression [-Wdouble-promotion]
  #define max(a, b) ((a) > (b) ? (a) : (b))
                         ^
../../master/src/image.c:2123:12: note: in expansion of macro ‘max’
    xscale = max (xscale, FLT_MIN);
             ^
../../master/src/lisp.h:61:36: warning: implicit conversion from ‘float’ to 
‘double’ to match other result of conditional [-Wdouble-promotion]
  #define max(a, b) ((a) > (b) ? (a) : (b))
                                     ^
../../master/src/image.c:2123:12: note: in expansion of macro ‘max’
    xscale = max (xscale, FLT_MIN);
             ^
../../master/src/image.c: In function ‘image_set_transform.isra.21’:
../../master/src/image.c:2136:14: warning: ‘sin_r’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
    int cos_r, sin_r;
               ^~~~~
../../master/src/image.c:2175:17: warning: ‘cos_r’ may be used uninitialized in 
this function [-Wmaybe-uninitialized]
        matrix3x3 rot = { [0][0] = cos_r, [0][1] = -sin_r,

The double-promotion warning is fixed by using DBL_MIN instead of FLT_MIN.

The maybe-uninitialized warnings seem bogus to me.

Ken

Re: [Emacs-diffs] master 74a5a33: Support native image transforms on MS-Windows

[Paul Eggert]

Ken Brown wrote:
> I'm seeing a few compiler warnings after this commit with GCC-7.4.0:

I installed a fix for that on master.

Re: [Emacs-diffs] master 74a5a33: Support native image transforms on MS-Windows

[Eli Zaretskii]

> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 29 Jun 2019 17:00:59 -0700
> 
> Ken Brown wrote:
> > I'm seeing a few compiler warnings after this commit with GCC-7.4.0:
> 
> I installed a fix for that on master.

Thanks, but I think divide_double part obfuscates the code for no good
reason.

Re: [Emacs-diffs] master 74a5a33: Support native image transforms on MS-Windows

[Paul Eggert]

Eli Zaretskii wrote:
> divide_double part obfuscates the code for no good reason.

I'm sure we can clear it up. It replaces the check against FLT_MIN, which to my 
mind was even more obscure as there are no 'float' values in sight.

What is the failure scenario here? Is it not floating-point division by zero?

Re: [Emacs-diffs] master 74a5a33: Support native image transforms on MS-Windows

[Eli Zaretskii]

> Cc: kbrown@cornell.edu, emacs-devel@gnu.org
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Sat, 29 Jun 2019 21:08:46 -0700
> 
> Eli Zaretskii wrote:
> > divide_double part obfuscates the code for no good reason.
> 
> I'm sure we can clear it up. It replaces the check against FLT_MIN, which to my 
> mind was even more obscure as there are no 'float' values in sight.

I mean that an explicit division together with a test is more clear
than a call to a function that one needs to look up.

> What is the failure scenario here? Is it not floating-point division by zero?

Yes.  But in one of the branches that is extremely unlikely, since the
denominator comes from the size of an image that we loaded.