kicad-developers team mailing list archive

[Wayne Stambaugh <stambaughw@xxxxxxxxx>]

I committed patches 1 & 2 for the time being.  I'm not completely sold
enum part of patch 3.  Using a word like tautological always makes me
suspicious. :).  Please don't assume a compiler warning is bad code and
that the person who wrote the code didn't understand what they were
doing.  Too many developers put way too much stock in compiler warnings.
 Sometimes they are valid but more often than not they are benign or
worse noise that distracts developers from real issues.  From now until
the stable release, please only post patches that fix actual bugs.

Thanks,

Wayne

On 6/26/2015 1:58 PM, Chris Pavlina wrote:
> Hi,
> 
> Attached are three patches to clean up some compiler warnings from clang.
> 
> 1: replace abs() with std::abs() in polygon/math_for_graphics.cpp
> 
> This is the correct usage. The math is being done from double to double, 
> but abs() is an integer function. std::abs() will return the correct 
> types.
> 
> 2: delete a couple unused variables
> 
> 3: correct a couple tautological comparisons
> 
> By defining UNDEFINED_LAYER and UNSELECTED_LAYER as follows:
> 
> #define UNDEFINED_LAYER     LAYER_ID(-1)
> #define UNSELECTED_LAYER    LAYER_ID(-2)
> 
> "enum LAYER_ID" is allowed to be an unsigned type (in fact, it is 
> explicitly declared to be one). A compiler is totally free to assume 
> that any comparison between this unsigned enum and a negative value is 
> false by definition and not actually perform the comparison. Currently 
> we aren't having this problem, but because it's allowed, it could become 
> a problem in the future with different or newer compilers and different 
> optimization settings. I removed the explicit declaration of the enum as 
> unsigned and defined UNDEFINED_LAYER and UNSELECTED_LAYER _in_ the enum.
> 
> In polygon/poly2tri/sweep/sweep.cc, the address of a reference is tested 
> against NULL. This is a similar tautological comparison because C++ 
> explicitly disallows references to NULL. I changed this to a pointer 
> type (which had the added benefit of making it more consistent with the 
> other functions in the file).
> 
> --
> Chris
> 
> 
> 
> _______________________________________________
> Mailing list: https://launchpad.net/~kicad-developers
> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~kicad-developers
> More help   : https://help.launchpad.net/ListHelp
>