kicad-developers team mailing list archive

[Chris Pavlina <pavlina.chris@xxxxxxxxx>]

Hi,

layers_id_colors_and_visibility.h defines enum LAYER_ID as unsigned, 
then #defines two negative constants to be used with the enum. 
Comparisons of these generate the following warning:

warning: comparison of constant 4294967295 with expression of type 'int' 
is always true [-Wtautological-constant-out-of-range-compare]


Currently there is no problem (I actually dug into the assembler to make 
sure the comparison was being generated). However, this is a time bomb 
just waiting for a different compiler version which decides to optimize 
out what it sees as a tautological comparison, and the fix is very, very 
simple.

Attached patch does two things:

1) Move the negative constants INTO the enum so that the compiler knows 
to assign the enum a backing type that supports negatives.

2) Remove the unnecessary forced-typing of the enum. There is absolutely 
no reason this enum has to be an unsigned-char; I suspect this was an 
instance of overzealous and misguided "optimization". This 
"optimization" will at best not do anything (as the compiler will decide 
to pad out the char to the platform's word length anyway), and at worst 
make things even /slower/ (if the compiler actually decides to access it 
as a char).

--
Chris

commit ad4a1b6d2a938b8d108000a5799b47618112c0ad
Author: Chris Pavlina <cpavlin1@xxxxxxxxxxxxxx>
Date:   Tue Jan 5 11:02:13 2016 -0500

    Fix enum comparison warning

diff --git a/include/layers_id_colors_and_visibility.h b/include/layers_id_colors_and_visibility.h
index b424aea..db72495 100644
--- a/include/layers_id_colors_and_visibility.h
+++ b/include/layers_id_colors_and_visibility.h
@@ -54,9 +54,6 @@ typedef int     LAYER_NUM;
  * One of these cannot be "incremented".
  */
 enum LAYER_ID
-#if __cplusplus >= 201103L
-    : unsigned char
-#endif
 {
     F_Cu,           // 0
     In1_Cu,
@@ -116,12 +113,12 @@ enum LAYER_ID
     B_Fab,
     F_Fab,
 
-    LAYER_ID_COUNT
-};
+    LAYER_ID_COUNT,
 
+    UNDEFINED_LAYER = -1,
+    UNSELECTED_LAYER = -2,
+};
 
-#define UNDEFINED_LAYER     LAYER_ID(-1)
-#define UNSELECTED_LAYER    LAYER_ID(-2)
 
 #define MAX_CU_LAYERS       (B_Cu - F_Cu + 1)