(In reply to Massimo from comment #0)
> I've just run
> 
> $make -sC app CC=clang &>out.log
> 
> 
...
> 
> -Wtautological-constant-out-of-range-compare
> comparisons of enum with -1 when -1 is not a valid enum value
> are apparently evaluated at compile-time, possibly making them
> useless, example:
> 
> 
...
> 
> -Wtautological-compare
> enums probably default to unsigned if they do not have any negative values
> example:
> >gradient-editor-commands.c:123:18: warning: comparison of unsigned enum expression >= 0 is always true [-Wtautological-compare]
> >      color_type >= 0 &&
> >      ~~~~~~~~~~ ^  ~
> >gradient-editor-commands.c:277:18: warning: comparison of unsigned enum expression >= 0 is always true [-Wtautological-compare]
> >      color_type >= 0 &&
> >      ~~~~~~~~~~ ^  ~

Investigating -Wtautological-constant-out-of-range-compare
and -Wtautological-compare warnings I compiled the following
program:

#include <stdio.h>

enum Enum { A, B, C };

int
main (void)
{
  enum Enum e = -1;

  printf (e >=  0 ? "-1 >= 0\n" : "-1 < 0\n");
  printf (e != -1 ? "-1 != -1\n" : "-1 == -1\n");

  return 0;
}

with different compiler flags -O0 -O2 -O3 -Os -Ofast.

The first test (-1 >= 0) is always true, the second test (-1 != -1)
is true (as the compiler warning suggests) only using -fshort-enums
(gcc and clang)

'$ info gcc' in section 4.9 says -fshort-enums is the default for
some targets whose ABI requires it.

This means GIMP code is not portable to those target platforms,
assuming that the observed behaviour without -fshort-enums is
the intended behaviour

I don't think it's worth the effort, we use -1 as "none of these values"
all over the place. GIMP would pretty much fall apart if that didn't work.
What platforms are these?

I think I looked at all those warnings now and they all compare
"foo == -1", never <= or =>, unless I missed one.

Fixed most other warnings I found:

commit f2d581a53639bef26227edcd21925bf228b0162a
Author: Michael Natterer <mitch@gimp.org>
Date:   Fri Feb 12 22:48:59 2016 +0100

    Bug 761170 - Warnings building with clang
    
    Fix a bunch of clang warnings.

 app/core/gimpcontext.c         | 16 ++++++++++++----
 app/core/gimpimage-new.c       |  2 +-
 app/dialogs/lebl-dialog.c      |  4 ----
 app/display/gimpcursorview.c   |  2 +-
 app/tools/gimpcagetool.c       | 14 +++++++-------
 app/tools/gimpsheartool.c      |  4 ++--
 libgimpbase/gimpenv.c          |  2 +-
 libgimpwidgets/gimpruler.c     |  2 +-
 modules/color-selector-water.c |  5 -----
 9 files changed, 25 insertions(+), 26 deletions(-)

(In reply to Michael Natterer from comment #3)
> I think I looked at all those warnings now and they all compare
> "foo == -1", never <= or =>, unless I missed one.

(In reply to Massimo from comment #1)
> ...
> > 
> > -Wtautological-compare
> > enums probably default to unsigned if they do not have any negative values
> > example:
> > >gradient-editor-commands.c:123:18: warning: comparison of unsigned enum expression >= 0 is always true [-Wtautological-compare]
> > >      color_type >= 0 &&
> > >      ~~~~~~~~~~ ^  ~
> > >gradient-editor-commands.c:277:18: warning: comparison of unsigned enum expression >= 0 is always true [-Wtautological-compare]
> > >      color_type >= 0 &&
> > >      ~~~~~~~~~~ ^  ~

Only these two:

https://git.gnome.org/browse/gimp/tree/app/actions/gradient-editor-commands.c#n123

they're useless, if gtk_radio_action_get_current_value ever returns
< 0 that expression is true.

(In reply to Michael Natterer from comment #2)
> I don't think it's worth the effort, we use -1 as "none of these values"
> all over the place. GIMP would pretty much fall apart if that didn't work.
> What platforms are these?

I had the gimp tree configured for gcc, but passed CC=clang
to make.

There are 2 cases that could be refactored to avoid the warnings

https://git.gnome.org/browse/gimp/tree/app/display/gimpimagewindow.c#n868

this basically inlines two g_list_find calls, but two early return
would do the same without the need of the side != -1 check in the
loop condition.

here:

https://git.gnome.org/browse/gimp/tree/app/tools/gimpfreeselecttool.c#n1456

handle_type could be initialized only when used

something like

if (dist <= MAX ({GRAB,SHOW}_THRESHOLD))
  {
     handle_type = ? : ;

Probably since a few days, GIMP fails to build with clang, it stops at:

>  CC       metadata-impexp.o
>  CC       metadata-xml.o
>In file included from metadata-impexp.c:40:
>./metadata-tags.h:223:27: warning: duplicate 'const' declaration specifier [-Wduplicate-decl-specifier]
>static const metadata_tag const equivalent_metadata_tags[] =
>                          ^
>1 warning generated.
>In file included from metadata-editor.c:38:
>./metadata-tags.h:223:27: warning: duplicate 'const' declaration specifier [-Wduplicate-decl-specifier]
>static const metadata_tag const equivalent_metadata_tags[] =
>                          ^
>metadata-editor.c:5266:43: error: invalid suffix 'd' on floating constant
>                alt_d = (alt_d * 12 * 2.54d) / 100;
>                                          ^
>metadata-editor.c:5719:38: error: invalid suffix 'd' on floating constant
>          alt_d = (alt_d * (12 * 2.54d)) / 100;
>                                     ^
>metadata-editor.c:5731:31: error: invalid suffix 'd' on floating constant
>          alt_d = alt_d * 3.28d;
>                              ^
>1 warning and 3 errors generated.
>make[3]: *** [Makefile:765: metadata-editor.o] Error 1

$ git describe
GIMP_2_9_4-1716-ga223ba8f40

Thanks, fixed those, does it now build again?

commit d315cc1034158cae0d12d9e81557babc550df160
Author: Michael Natterer <mitch@gimp.org>
Date:   Fri Jul 14 12:26:10 2017 +0200

    Bug 761170 - Warnings building with clang
    
    Fix clang warnings along with some other minor stuff, no logic changes.

 plug-ins/metadata/metadata-editor.c |  6 +++---
 plug-ins/metadata/metadata-impexp.c | 10 ++++++++--
 plug-ins/metadata/metadata-tags.h   |  2 +-
 plug-ins/metadata/metadata-xml.c    | 25 +++++++++----------------
 plug-ins/metadata/metadata-xml.h    |  2 +-
 5 files changed, 22 insertions(+), 23 deletions(-)

Can you perhaps prepare a patch for the other warnings you see?

Created attachment 355588 [details] [review]
fix few clang warnings

> Can you perhaps prepare a patch for the other warnings you see?


The attached patch should fix the easy ones:


>gimppluginmanager-restore.c:287:44: warning: implicit conversion from enumeration type 'GFileCreateFlags' to different enumeration type 'GFileQueryInfoFlags' [-Wenum-conversion]
>                                           G_FILE_CREATE_NONE,
>                                           ^~~~~~~~~~~~~~~~~~
>1 warning generated.

>gimpcolorprofile.c:855:32: warning: suggest braces around initialization of subobject [-Wmissing-braces]
>  GimpMatrix3       matrix = { 0, };
>                               ^
>                               {}
>gimpcolorprofile.c:855:32: warning: suggest braces around initialization of subobject [-Wmissing-braces]
>  GimpMatrix3       matrix = { 0, };
>                               ^
>                               {}
>2 warnings generated.

>gimpcolorspace.c:1044:18: warning: explicitly assigning value of variable of type 'gdouble' (aka 'double') to itself [-Wself-assign]
>      value      = value; 
>      ~~~~~      ^ ~~~~~
>1 warning generated.

>border-average.c:171:11: warning: variable 'result_color' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
>      if (gimp_drawable_is_rgb (drawable_id))
>          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>border-average.c:200:28: note: uninitialized use occurs here
>  values[1].data.d_color = result_color;
>                           ^~~~~~~~~~~~
>border-average.c:171:7: note: remove the 'if' if its condition is always true
>      if (gimp_drawable_is_rgb (drawable_id)) 
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 
>border-average.c:168:7: warning: variable 'result_color' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
>  if (status == GIMP_PDB_SUCCESS)
>      ^~~~~~~~~~~~~~~~~~~~~~~~~~
>border-average.c:200:28: note: uninitialized use occurs here
>  values[1].data.d_color = result_color;
>                           ^~~~~~~~~~~~
>border-average.c:168:3: note: remove the 'if' if its condition is always true
>  if (status == GIMP_PDB_SUCCESS)
>  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>border-average.c:124:3: note: variable 'result_color' is declared here
>  GimpRGB            result_color;
>  ^
>2 warnings generated.

>file-raw-data.c:1071:33: warning: implicit conversion from enumeration type 'GimpImageBaseType' to different enumeration type 'GimpImageType' [-Wenum-conversion]
>  GimpImageType      ltype    = GIMP_RGB;
>                     ~~~~~      ^~~~~~~~
>file-raw-data.c:1072:33: warning: implicit conversion from enumeration type 'GimpImageType' to different enumeration type 'GimpImageBaseType' [-Wenum-conversion]
>  GimpImageBaseType  itype    = GIMP_RGB_IMAGE;
>                     ~~~~~      ^~~~~~~~~~~~~~
>2 warnings generated.

>hot.c:360:57: warning: passing 'guint *' (aka 'unsigned int *') to parameter of type 'gint *' (aka 'int *') converts between pointers to integer types with different sign [-Wpointer-sign]
>                                      &sel_x1, &sel_y1, &width, &height))
>                                                        ^~~~~~
>../../libgimp/gimpdrawable_pdb.h:59:88: note: passing argument to parameter 'width' here
>                                                           gint                       *width,
>                                                                                       ^
>hot.c:360:65: warning: passing 'guint *' (aka 'unsigned int *') to parameter of type 'gint *' (aka 'int *') converts between pointers to integer types with different sign [-Wpointer-sign]
>                                      &sel_x1, &sel_y1, &width, &height))
>                                                                ^~~~~~~
>../../libgimp/gimpdrawable_pdb.h:60:88: note: passing argument to parameter 'height' here
>                                                           gint                       *height);
>                                                                                       ^

>mail.c:378:17: warning: address of array 'mail_info.subject' will always evaluate to 'true' [-Wpointer-bool-conversion]
>  if (mail_info.subject &&
>      ~~~~~~~~~~^~~~~~~ ~~
>mail.c:384:17: warning: address of array 'mail_info.comment' will always evaluate to 'true' [-Wpointer-bool-conversion]
>  if (mail_info.comment &&
>      ~~~~~~~~~~^~~~~~~ ~~
>mail.c:390:17: warning: address of array 'mail_info.receipt' will always evaluate to 'true' [-Wpointer-bool-conversion]
>  if (mail_info.receipt &&
>      ~~~~~~~~~~^~~~~~~ ~~
>3 warnings generated.

>gfig-dialog.c:1904:34: warning: implicit conversion from enumeration type 'FillType' to different enumeration type 'GimpBucketFillMode' [-Wenum-conversion]
>  GimpBucketFillMode fill_mode = FILL_NONE;
>                     ~~~~~~~~~   ^~~~~~~~~
>1 warning generated.

>scheme-wrapper.c:81:28: warning: duplicate 'const' declaration specifier [-Wduplicate-decl-specifier]
>static const NamedConstant const script_constants[] =
>                           ^

(In reply to Michael Natterer from comment #8)
> Thanks, fixed those, does it now build again?
> 

Yes, thank you.

Comment on attachment 355588 [details] [review]
fix few clang warnings

Thanks, pushed to master:

commit f2306abc0a9c6d3c40d0409375e7e03992b58eb7
Author: Massimo Valentini <mvalentini@src.gnome.org>
Date:   Fri Jul 14 13:10:46 2017 +0200

    Bug 761170: Warnings building with clang
    
    Fix some of the warnings.

 app/plug-in/gimppluginmanager-restore.c | 2 +-
 libgimpcolor/gimpcolorprofile.c         | 2 +-
 libgimpcolor/gimpcolorspace.c           | 2 +-
 plug-ins/common/border-average.c        | 2 +-
 plug-ins/common/file-raw-data.c         | 4 ++--
 plug-ins/common/hot.c                   | 2 +-
 plug-ins/common/mail.c                  | 9 +++------
 plug-ins/gfig/gfig-dialog.c             | 2 +-
 plug-ins/script-fu/scheme-wrapper.c     | 2 +-
 9 files changed, 12 insertions(+), 15 deletions(-)

Clang doesn't build GIMP tree on linux. It stops with:

>   CC       gimpintcombobox.lo
> gimpintcombobox.c:703:1: error: conflicting types for 'gimp_int_combo_box_get_layout'
> gimp_int_combo_box_get_layout (GimpIntComboBox *combo_box)
> ^
> ./gimpintcombobox.h:107:15: note: previous declaration is here
>               gimp_int_combo_box_get_layout      (GimpIntComboBox       *combo_box);
>               ^
> 1 error generated.


The problem is in this commit:

https://git.gnome.org/browse/gimp/commit/?id=47ac6111de4fb77645be257fc4c6820d3ad6cb9c

the declaration with a const on the return value:

+const GimpIntComboBoxLayout
+              gimp_int_combo_box_get_layout      (GimpIntComboBox       *combo_box);
+

and the definition without

+ * Since: 2.10
+ **/
+GimpIntComboBoxLayout
+gimp_int_combo_box_get_layout (GimpIntComboBox *combo_box)
+{

removing the const fixes the build

Thanks.  Fixed in master:

commit 8e51e7c35195c798c1fbecb010d6d5a5be72c9e0
Author: Ell <ell_se@yahoo.com>
Date:   Fri Dec 1 05:11:11 2017 -0500

    libgimpwidgets: fix return type of gimp_int_combo_box_get_layout()
    
    ... and a small cleanup.

 libgimpwidgets/gimpintcombobox.c | 3 ++-
 libgimpwidgets/gimpintcombobox.h | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gimp/issues/835.