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@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