kicad-developers team mailing list archive

[Chris Pavlina <pavlina.chris@xxxxxxxxx>]

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

commit 7028c4890610592116725c7971c67683ea3ae304
Author: Chris Pavlina <cpavlin1@xxxxxxxxxxxxxx>
Date:   Fri Jun 26 09:31:58 2015 -0400

    Correct abs to std::abs

diff --git a/polygon/math_for_graphics.cpp b/polygon/math_for_graphics.cpp
index d7c4be6..c43f323 100644
--- a/polygon/math_for_graphics.cpp
+++ b/polygon/math_for_graphics.cpp
@@ -68,13 +68,13 @@ bool FindLineSegmentIntersection( double a, double b, int xi, int yi, int xf, in
             else
             {
                 if( dist )
-                    *dist = std::min( abs( a - xi ), abs( a - xf ) );
+                    *dist = std::min( std::abs( a - xi ), std::abs( a - xf ) );
 
                 return false;
             }
         }
 
-        if( fabs( b - d ) < 1E-12 )
+        if( std::abs( b - d ) < 1E-12 )
         {
             // parallel lines
             if( dist )
@@ -429,7 +429,7 @@ double GetPointToLineDistance( double a, double b, int x, int y, double* xpp, do
             *ypp    = y;
         }
 
-        return abs( a - x );
+        return std::abs( a - x );
     }
 
     // find c,d such that (x,y) lies on y = c + dx where d=(-1/b)
@@ -466,7 +466,7 @@ double GetPointToLineSegmentDistance( int x, int y, int xi, int yi, int xf, int
     {
         // vertical line segment
         if( InRange( y, yi, yf ) )
-            return abs( x - xi );
+            return std::abs( x - xi );
         else
             return std::min( Distance( x, y, xi, yi ), Distance( x, y, xf, yf ) );
     }
@@ -474,7 +474,7 @@ double GetPointToLineSegmentDistance( int x, int y, int xi, int yi, int xf, int
     {
         // horizontal line segment
         if( InRange( x, xi, xf ) )
-            return abs( y - yi );
+            return std::abs( y - yi );
         else
             return std::min( Distance( x, y, xi, yi ), Distance( x, y, xf, yf ) );
     }

commit 739399cf6ff97dd0a70de75749f193053a66ffa4
Author: Chris Pavlina <cpavlin1@xxxxxxxxxxxxxx>
Date:   Fri Jun 26 09:32:45 2015 -0400

    Delete unused variables

diff --git a/common/geometry/shape_collisions.cpp b/common/geometry/shape_collisions.cpp
index a5dbbfa..25a6116 100644
--- a/common/geometry/shape_collisions.cpp
+++ b/common/geometry/shape_collisions.cpp
@@ -373,11 +373,9 @@ bool SHAPE_RECT::Collide( const SEG& aSeg, int aClearance ) const
         {
             SEG s( vts[i], vts[i + 1], i );
 
-            int64_t dist = s.Distance( aSeg );
-
             if( s.Distance( aSeg ) < aClearance )
                 return true;
         }
 
         return false;
-    }
\ No newline at end of file
+    }
diff --git a/common/pgm_base.cpp b/common/pgm_base.cpp
index f4c8f86..e423fba 100644
--- a/common/pgm_base.cpp
+++ b/common/pgm_base.cpp
@@ -61,7 +61,6 @@
 const wxChar PGM_BASE::workingDirKey[] = wxT( "WorkingDir" );     // public
 
 static const wxChar languageCfgKey[]   = wxT( "LanguageID" );
-static const wxChar kicadFpLibPath[]   = wxT( "KicadFootprintLibraryPath" );
 static const wxChar pathEnvVariables[] = wxT( "EnvironmentVariables" );
 static const wxChar showEnvVarWarningDialog[] = wxT( "ShowEnvVarWarningDialog" );
 static const wxChar traceEnvVars[]     = wxT( "KIENVVARS" );

commit 6cbd2177bfe3b0ce7f354a9b5fd73cf3d60ccd18
Author: Chris Pavlina <cpavlin1@xxxxxxxxxxxxxx>
Date:   Fri Jun 26 13:29:14 2015 -0400

    Remove tautological comparisons

diff --git a/include/layers_id_colors_and_visibility.h b/include/layers_id_colors_and_visibility.h
index 7415e42..cf609a8 100644
--- a/include/layers_id_colors_and_visibility.h
+++ b/include/layers_id_colors_and_visibility.h
@@ -55,9 +55,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,
@@ -117,12 +114,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)
 
diff --git a/polygon/poly2tri/common/shapes.cc b/polygon/poly2tri/common/shapes.cc
index 06eb1f8..32107ba 100644
--- a/polygon/poly2tri/common/shapes.cc
+++ b/polygon/poly2tri/common/shapes.cc
@@ -476,18 +476,18 @@ void Triangle::SetDelunayEdgeCW( Point& p, bool e )
 
 
 // The neighbor across to given point
-Triangle& Triangle::NeighborAcross( Point& opoint )
+Triangle* Triangle::NeighborAcross( Point& opoint )
 {
     if( &opoint == points_[0] )
     {
-        return *neighbors_[0];
+        return neighbors_[0];
     }
     else if( &opoint == points_[1] )
     {
-        return *neighbors_[1];
+        return neighbors_[1];
     }
 
-    return *neighbors_[2];
+    return neighbors_[2];
 }
 
 
diff --git a/polygon/poly2tri/common/shapes.h b/polygon/poly2tri/common/shapes.h
index c65f485..b41f97e 100644
--- a/polygon/poly2tri/common/shapes.h
+++ b/polygon/poly2tri/common/shapes.h
@@ -1,4 +1,5 @@
 /*
+ * Copyright (c) 2015 KiCad Developers, see AUTHORS.txt for contributors.
  * Poly2Tri Copyright (c) 2009-2010, Poly2Tri Contributors
  * http://code.google.com/p/poly2tri/
  *
@@ -209,7 +210,7 @@ public:
     inline bool IsInterior();
     inline void IsInterior( bool b );
 
-    Triangle&   NeighborAcross( Point& opoint );
+    Triangle*   NeighborAcross( Point& opoint );
 
     void        DebugPrint();
 
diff --git a/polygon/poly2tri/sweep/sweep.cc b/polygon/poly2tri/sweep/sweep.cc
index 75e7adf..8e8a581 100644
--- a/polygon/poly2tri/sweep/sweep.cc
+++ b/polygon/poly2tri/sweep/sweep.cc
@@ -118,7 +118,7 @@ void Sweep::EdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle* triangl
       // We are modifying the constraint maybe it would be better to
       // not change the given constraint and just keep a variable for the new constraint
       tcx.edge_event.constrained_edge->q = p1;
-      triangle = &triangle->NeighborAcross(point);
+      triangle = triangle->NeighborAcross(point);
       EdgeEvent( tcx, ep, *p1, triangle, *p1 );
     } else {
       std::runtime_error("EdgeEvent - collinear points not supported");
@@ -135,7 +135,7 @@ void Sweep::EdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle* triangl
       // We are modifying the constraint maybe it would be better to
       // not change the given constraint and just keep a variable for the new constraint
       tcx.edge_event.constrained_edge->q = p2;
-      triangle = &triangle->NeighborAcross(point);
+      triangle = triangle->NeighborAcross(point);
       EdgeEvent( tcx, ep, *p2, triangle, *p2 );
     } else {
       std::runtime_error("EdgeEvent - collinear points not supported");
@@ -699,10 +699,10 @@ void Sweep::FillLeftConcaveEdgeEvent(SweepContext& tcx, Edge* edge, Node& node)
 
 void Sweep::FlipEdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle* t, Point& p)
 {
-  Triangle& ot = t->NeighborAcross(p);
-  Point& op = *ot.OppositePoint(*t, p);
+  Triangle* ot = t->NeighborAcross(p);
+  Point& op = *ot->OppositePoint(*t, p);
 
-  if (&ot == NULL) {
+  if (ot == NULL) {
     // If we want to integrate the fillEdgeEvent do it here
     // With current implementation we should never get here
     //throw new RuntimeException( "[BUG:FIXME] FLIP failed due to missing triangle");
@@ -711,27 +711,27 @@ void Sweep::FlipEdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle* t,
 
   if (InScanArea(p, *t->PointCCW(p), *t->PointCW(p), op)) {
     // Lets rotate shared edge one vertex CW
-    RotateTrianglePair(*t, p, ot, op);
+    RotateTrianglePair(*t, p, *ot, op);
     tcx.MapTriangleToNodes(*t);
-    tcx.MapTriangleToNodes(ot);
+    tcx.MapTriangleToNodes(*ot);
 
     if (p == eq && op == ep) {
       if (eq == *tcx.edge_event.constrained_edge->q && ep == *tcx.edge_event.constrained_edge->p) {
         t->MarkConstrainedEdge(&ep, &eq);
-        ot.MarkConstrainedEdge(&ep, &eq);
+        ot->MarkConstrainedEdge(&ep, &eq);
         Legalize(tcx, *t);
-        Legalize(tcx, ot);
+        Legalize(tcx, *ot);
       } else {
         // XXX: I think one of the triangles should be legalized here?
       }
     } else {
       Orientation o = Orient2d(eq, op, ep);
-      t = &NextFlipTriangle(tcx, (int)o, *t, ot, p, op);
+      t = &NextFlipTriangle(tcx, (int)o, *t, *ot, p, op);
       FlipEdgeEvent(tcx, ep, eq, t, p);
     }
   } else {
-    Point& newP = NextFlipPoint(ep, eq, ot, op);
-    FlipScanEdgeEvent(tcx, ep, eq, *t, ot, newP);
+    Point& newP = NextFlipPoint(ep, eq, *ot, op);
+    FlipScanEdgeEvent(tcx, ep, eq, *t, *ot, newP);
     EdgeEvent(tcx, ep, eq, t, p);
   }
 }
@@ -777,10 +777,10 @@ Point& Sweep::NextFlipPoint(Point& ep, Point& eq, Triangle& ot, Point& op)
 void Sweep::FlipScanEdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle& flip_triangle,
                               Triangle& t, Point& p)
 {
-  Triangle& ot = t.NeighborAcross(p);
-  Point& op = *ot.OppositePoint(t, p);
+  Triangle* ot = t.NeighborAcross(p);
+  Point& op = *ot->OppositePoint(t, p);
 
-  if (&t.NeighborAcross(p) == NULL) {
+  if (t.NeighborAcross(p) == NULL) {
     // If we want to integrate the fillEdgeEvent do it here
     // With current implementation we should never get here
     //throw new RuntimeException( "[BUG:FIXME] FLIP failed due to missing triangle");
@@ -789,7 +789,7 @@ void Sweep::FlipScanEdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle&
 
   if (InScanArea(eq, *flip_triangle.PointCCW(eq), *flip_triangle.PointCW(eq), op)) {
     // flip with new edge op->eq
-    FlipEdgeEvent(tcx, eq, op, &ot, op);
+    FlipEdgeEvent(tcx, eq, op, ot, op);
     // TODO: Actually I just figured out that it should be possible to
     //       improve this by getting the next ot and op before the the above
     //       flip and continue the flipScanEdgeEvent here
@@ -798,8 +798,8 @@ void Sweep::FlipScanEdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle&
     // Turns out at first glance that this is somewhat complicated
     // so it will have to wait.
   } else{
-    Point& newP = NextFlipPoint(ep, eq, ot, op);
-    FlipScanEdgeEvent(tcx, ep, eq, flip_triangle, ot, newP);
+    Point& newP = NextFlipPoint(ep, eq, *ot, op);
+    FlipScanEdgeEvent(tcx, ep, eq, flip_triangle, *ot, newP);
   }
 }