Re: [Kicad-developers] [PATCH] Warnings cleanup
Hi,
On 04.07.2015 12:53, Camille 019 wrote:
-if( layer == NO_AVAILABLE_LAYERS )
+if( layer == static_castint(NO_AVAILABLE_LAYERS) )
Nope, that relies on undefined overflow semantics, and compilers that do
constant propagation through casts would miscompile this code.
The correct way to solve this is the patch from Chris Pavlina:
--- a/include/layers_id_colors_and_visibility.h
+++ b/include/layers_id_colors_and_visibility.h
@@ -55,7 +55,7 @@ typedef int LAYER_NUM;
*/
enum LAYER_ID
#if __cplusplus = 201103L
-: unsigned char
+: signed char
#endif
{
F_Cu, // 0
@@ -116,12 +116,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_LAYERLAYER_ID(-2)
#define MAX_CU_LAYERS (B_Cu - F_Cu + 1)
By including the negative values in the enum, the compiler is forced to
use an underlying data type that can represent them.
The alternative would be to remove the special cases from the enum, and
create a separate variable for validity of layer selection in those
parts of the program that need it.
Simon
signature.asc
Description: OpenPGP digital signature
___
Mailing list: https://launchpad.net/~kicad-developers
Post to : kicad-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~kicad-developers
More help : https://help.launchpad.net/ListHelp
[Kicad-developers] [PATCH] Warnings cleanup
Hi, A set of patch to clean some warnings I get with Clang. The list of warnings : ../pcbnew/router/pns_router.cpp:208:10: warning: unused variable 'nonOrtho' [-Wunused-variable] bool nonOrtho = false; ../pcbnew/router/pns_topology.cpp:385:21: note: use function 'std::abs' instead gap = (int) abs( refDir.Cross( displacement ) / refDir.EuclideanNorm() ) - lp-Width(); ^~~ ../gerbview/./gerbview_frame.h:430:40: warning: comparison of constant 4294967295 with expression of type 'int' is always true [-Wtautological-constant-out-of-range-compare] return getNextAvailableLayer() != NO_AVAILABLE_LAYERS; ~~~ ^ ~~~ ../gerbview/files.cpp:189:23: warning: comparison of constant 4294967295 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare] if( layer == NO_AVAILABLE_LAYERS ) ~ ^ ~~~ ../gerbview/files.cpp:272:23: warning: comparison of constant 4294967295 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare] if( layer == NO_AVAILABLE_LAYERS ) ~ ^ ~~ ../gerbview/select_layers_to_pcb.cpp:336:19: warning: comparison of constant 4294967294 with expression of type 'LAYER_NUM' (aka 'int') is always false [-Wtautological-constant-out-of-range-compare] if( layer == UNSELECTED_LAYER ) ~ ^ ../gerbview/select_layers_to_pcb.cpp:363:12: warning: comparison of constant 4294967294 with expression of type 'LAYER_NUM' (aka 'int') is always true [-Wtautological-constant-out-of-range-compare] if( jj != UNSELECTED_LAYER !IsValidLayer( jj ) ) ~~ ^ ../gerbview/select_layers_to_pcb.cpp:368:12: warning: comparison of constant 4294967294 with expression of type 'LAYER_NUM' (aka 'int') is always true [-Wtautological-constant-out-of-range-compare] if( jj != UNSELECTED_LAYER !IsValidLayer( jj ) ) ~~ ^ ../gerbview/select_layers_to_pcb.cpp:375:16: warning: comparison of constant 4294967294 with expression of type 'LAYER_NUM' (aka 'int') is always false [-Wtautological-constant-out-of-range-compare] if( jj == UNSELECTED_LAYER ) ~~ ^ ../gerbview/dialogs/dialog_select_one_pcb_layer.cpp:156:30: warning: comparison of constant 4294967294 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare] if( UNSELECTED_LAYER == aDefaultLayer ) ^ ~ Best regards, -- Camille patch_warnings.diff Description: Binary data patch_Wtautological-constant-out-of-range-compare.diff Description: Binary data ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Warnings cleanup
Mmmm I am not an expert but does using the static_castint on the enum constant really solve the problem or just make it invisible? I think the enums should not be uint64_t but some smaller int type. include/layers_id_colors_and_visibility.h The layer ids are just a small enum (fits in a CHAR), maybe the NO_AVAILABLE_LAYERS and UNSELECTED_LAYER should be defined there a bit differently. #define UNDEFINED_LAYER LAYER_ID(-1) #define UNSELECTED_LAYERLAYER_ID(-2) Apparently this results in these super-size consts on your system Just my $ 0.02 of opinion. Greetings, Edwin van den Oetelaar On Sat, Jul 4, 2015 at 12:53 PM, Camille 019 camille...@outlook.com wrote: Hi, A set of patch to clean some warnings I get with Clang. The list of warnings : ../pcbnew/router/pns_router.cpp:208:10: warning: unused variable 'nonOrtho' [-Wunused-variable] bool nonOrtho = false; ../pcbnew/router/pns_topology.cpp:385:21: note: use function 'std::abs' instead gap = (int) abs( refDir.Cross( displacement ) / refDir.EuclideanNorm() ) - lp-Width(); ^~~ ../gerbview/./gerbview_frame.h:430:40: warning: comparison of constant 4294967295 with expression of type 'int' is always true [-Wtautological-constant-out-of-range-compare] return getNextAvailableLayer() != NO_AVAILABLE_LAYERS; ~~~ ^ ~~~ ../gerbview/files.cpp:189:23: warning: comparison of constant 4294967295 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare] if( layer == NO_AVAILABLE_LAYERS ) ~ ^ ~~~ ../gerbview/files.cpp:272:23: warning: comparison of constant 4294967295 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare] if( layer == NO_AVAILABLE_LAYERS ) ~ ^ ~~ ../gerbview/select_layers_to_pcb.cpp:336:19: warning: comparison of constant 4294967294 with expression of type 'LAYER_NUM' (aka 'int') is always false [-Wtautological-constant-out-of-range-compare] if( layer == UNSELECTED_LAYER ) ~ ^ ../gerbview/select_layers_to_pcb.cpp:363:12: warning: comparison of constant 4294967294 with expression of type 'LAYER_NUM' (aka 'int') is always true [-Wtautological-constant-out-of-range-compare] if( jj != UNSELECTED_LAYER !IsValidLayer( jj ) ) ~~ ^ ../gerbview/select_layers_to_pcb.cpp:368:12: warning: comparison of constant 4294967294 with expression of type 'LAYER_NUM' (aka 'int') is always true [-Wtautological-constant-out-of-range-compare] if( jj != UNSELECTED_LAYER !IsValidLayer( jj ) ) ~~ ^ ../gerbview/select_layers_to_pcb.cpp:375:16: warning: comparison of constant 4294967294 with expression of type 'LAYER_NUM' (aka 'int') is always false [-Wtautological-constant-out-of-range-compare] if( jj == UNSELECTED_LAYER ) ~~ ^ ../gerbview/dialogs/dialog_select_one_pcb_layer.cpp:156:30: warning: comparison of constant 4294967294 with expression of type 'int' is always false [-Wtautological-constant-out-of-range-compare] if( UNSELECTED_LAYER == aDefaultLayer ) ^ ~ Best regards, -- Camille ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
