kicad-developers team mailing list archive
-
kicad-developers team
-
Mailing list archive
-
Message #18422
[PATCH] Clean up clang warnings
I built using clang to see if that compiler would pick up any useful
warnings that gcc isn't. A few were, so I fixed them. You might not want
to apply this patch directly - it also "fixes" a few things that aren't
really problems, not sure if you'd rather leave them than hush up the
compiler. Feel free to remove those from the patch or ask me to.
I've been running KiCad for about a week now with these changes made
locally to test for unintended side effects; nothing found.
Summary:
The important ones:
- enum LAYER_ID, for some reason, is manually set to an *unsigned* char,
and then *negative* constants are manually declared outside the enum.
Compilers can actually *optimize out* comparisons against these! I
redefined the enum as signed and put the negative constants *in* it.
- Use std::abs instead of abs in FindLineSegmentIntersection and
GetPointToLineDistance (in math_for_graphics.cpp). abs() is an
integer function and being called on doubles! This one could be
responsible for some graphics glitches by triggering an unintentional
rounding.
The less important ones:
- Delete string constant kicadFpLibPath, which is static inside
pgm_base.cpp and never used. Unnecessary, but it does keep things clean.
- A few subclasses implement superclasses' virtual methods with
*different type signatures*. This triggers a warning, because it
"hides" the parent's method; the warnings can be shut up by importing
super's version into sub with 'using'. This one's totally unnecessary
other than to quiet a few warnings, don't know what you want to do
with it.
- Remove unused member m_insideUpdateEvent in dialog_rescue_each.
- Change some scoping of nested enums inside eagle_plugin.cpp.
Just shuts up some warnings.
- Remove unused isSpace() from gpcb_plugin.cpp
- Remove comparisons between address of a reference variable and NULL in
sweep.cc. If a reference variable is NULL you're doing something Very
Very Bad, and the compiler is free to optimize /this/ out as well. The
code returning said variable appears to have no way to return NULL.
--
Chris
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" );
diff --git a/eeschema/class_libentry.h b/eeschema/class_libentry.h
index 3d6381c..e623ac2 100644
--- a/eeschema/class_libentry.h
+++ b/eeschema/class_libentry.h
@@ -303,6 +303,10 @@ public:
**/
const EDA_RECT GetBoundingBox( int aUnit, int aConvert ) const;
+ // The no-args GetBoundingBox from the parent class will simply return a
+ // zero bounding box, and trigger a wxAssert if this is a debug build.
+ using EDA_ITEM::GetBoundingBox;
+
/**
* Function GetBodyBoundingBox
* @return the part bounding box ( in user coordinates ) without fields
diff --git a/eeschema/dialogs/dialog_rescue_each.cpp b/eeschema/dialogs/dialog_rescue_each.cpp
index 8b896f4..9b04421 100644
--- a/eeschema/dialogs/dialog_rescue_each.cpp
+++ b/eeschema/dialogs/dialog_rescue_each.cpp
@@ -59,8 +59,6 @@ private:
std::vector<SCH_COMPONENT*>* m_Components;
bool m_AskShowAgain;
- bool m_insideUpdateEvent;
-
bool TransferDataToWindow();
bool TransferDataFromWindow();
void PopulateConflictList();
@@ -82,8 +80,7 @@ DIALOG_RESCUE_EACH::DIALOG_RESCUE_EACH( SCH_EDIT_FRAME* aParent, std::vector<RES
m_Parent( aParent ),
m_Candidates( &aCandidates ),
m_Components( &aComponents ),
- m_AskShowAgain( aAskShowAgain ),
- m_insideUpdateEvent( false )
+ m_AskShowAgain( aAskShowAgain )
{
m_Config = Kiface().KifaceSettings();
m_stdButtonsOK->SetDefault();
diff --git a/include/base_struct.h b/include/base_struct.h
index e6238f3..d9903fb 100644
--- a/include/base_struct.h
+++ b/include/base_struct.h
@@ -47,6 +47,8 @@ extern std::ostream& operator <<( std::ostream& out, const wxSize& size );
extern std::ostream& operator <<( std::ostream& out, const wxPoint& pt );
#endif
+#include <wx/debug.h>
+
/// Flag to enable find and replace tracing using the WXTRACE environment variable.
extern const wxString traceFindReplace;
@@ -318,8 +320,7 @@ public:
virtual const EDA_RECT GetBoundingBox() const
{
#if defined(DEBUG)
- printf( "Missing GetBoundingBox()\n" );
- Show( 0, std::cout ); // tell me which classes still need GetBoundingBox support
+ wxASSERT_MSG( false, "Derived class did not implement GetBoundingBox()\n" );
#endif
// return a zero-sized box per default. derived classes should override
diff --git a/include/layers_id_colors_and_visibility.h b/include/layers_id_colors_and_visibility.h
index 7415e42..ef02568 100644
--- a/include/layers_id_colors_and_visibility.h
+++ b/include/layers_id_colors_and_visibility.h
@@ -56,7 +56,7 @@ typedef int LAYER_NUM;
*/
enum LAYER_ID
#if __cplusplus >= 201103L
- : unsigned char
+ : signed char
#endif
{
F_Cu, // 0
@@ -117,12 +117,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/pcbnew/eagle_plugin.cpp b/pcbnew/eagle_plugin.cpp
index e6eaa38..efd29e0 100644
--- a/pcbnew/eagle_plugin.cpp
+++ b/pcbnew/eagle_plugin.cpp
@@ -258,7 +258,7 @@ struct EWIRE
LAYER_NUM layer;
// for style: (continuous | longdash | shortdash | dashdot)
- enum {
+ enum STYLE {
CONTINUOUS,
LONGDASH,
SHORTDASH,
@@ -268,7 +268,7 @@ struct EWIRE
opt_double curve; ///< range is -359.9..359.9
// for cap: (flat | round)
- enum {
+ enum CAP {
FLAT,
ROUND,
};
@@ -316,22 +316,22 @@ EWIRE::EWIRE( CPTREE& aWire )
if( s )
{
if( !s->compare( "continuous" ) )
- style = EWIRE::CONTINUOUS;
+ style = CONTINUOUS;
else if( !s->compare( "longdash" ) )
- style = EWIRE::LONGDASH;
+ style = LONGDASH;
else if( !s->compare( "shortdash" ) )
- style = EWIRE::SHORTDASH;
+ style = SHORTDASH;
else if( !s->compare( "dashdot" ) )
- style = EWIRE::DASHDOT;
+ style = DASHDOT;
}
s = attribs.get_optional<string>( "cap" );
if( s )
{
if( !s->compare( "round" ) )
- cap = EWIRE::ROUND;
+ cap = ROUND;
else if( !s->compare( "flat" ) )
- cap = EWIRE::FLAT;
+ cap = FLAT;
}
// ignoring extent
}
@@ -465,7 +465,7 @@ struct EATTR
opt_double ratio;
opt_erot rot;
- enum { // for 'display'
+ enum DISPLAY { // for 'display'
Off,
VALUE,
NAME,
@@ -523,13 +523,13 @@ EATTR::EATTR( CPTREE& aAttribute )
{
// (off | value | name | both)
if( !stemp->compare( "off" ) )
- display = EATTR::Off;
+ display = Off;
else if( !stemp->compare( "value" ) )
- display = EATTR::VALUE;
+ display = VALUE;
else if( !stemp->compare( "name" ) )
- display = EATTR::NAME;
+ display = NAME;
else if( !stemp->compare( "both" ) )
- display = EATTR::BOTH;
+ display = BOTH;
}
}
@@ -546,7 +546,7 @@ struct ETEXT
opt_double ratio;
opt_erot rot;
- enum { // for align
+ enum ALIGN { // for align
CENTER,
CENTER_LEFT,
TOP_CENTER,
@@ -599,23 +599,23 @@ ETEXT::ETEXT( CPTREE& aText )
// (bottom-left | bottom-center | bottom-right | center-left |
// center | center-right | top-left | top-center | top-right)
if( !stemp->compare( "center" ) )
- align = ETEXT::CENTER;
+ align = CENTER;
else if( !stemp->compare( "center-right" ) )
- align = ETEXT::CENTER_RIGHT;
+ align = CENTER_RIGHT;
else if( !stemp->compare( "top-left" ) )
- align = ETEXT::TOP_LEFT;
+ align = TOP_LEFT;
else if( !stemp->compare( "top-center" ) )
- align = ETEXT::TOP_CENTER;
+ align = TOP_CENTER;
else if( !stemp->compare( "top-right" ) )
- align = ETEXT::TOP_RIGHT;
+ align = TOP_RIGHT;
else if( !stemp->compare( "bottom-left" ) )
- align = ETEXT::BOTTOM_LEFT;
+ align = BOTTOM_LEFT;
else if( !stemp->compare( "bottom-center" ) )
- align = ETEXT::BOTTOM_CENTER;
+ align = BOTTOM_CENTER;
else if( !stemp->compare( "bottom-right" ) )
- align = ETEXT::BOTTOM_RIGHT;
+ align = BOTTOM_RIGHT;
else if( !stemp->compare( "center-left" ) )
- align = ETEXT::CENTER_LEFT;
+ align = CENTER_LEFT;
}
}
@@ -630,7 +630,7 @@ struct EPAD
opt_double diameter;
// for shape: (square | round | octagon | long | offset)
- enum {
+ enum SHAPE {
SQUARE,
ROUND,
OCTAGON,
@@ -679,15 +679,15 @@ EPAD::EPAD( CPTREE& aPad )
{
// (square | round | octagon | long | offset)
if( !s->compare( "square" ) )
- shape = EPAD::SQUARE;
+ shape = SQUARE;
else if( !s->compare( "round" ) )
- shape = EPAD::ROUND;
+ shape = ROUND;
else if( !s->compare( "octagon" ) )
- shape = EPAD::OCTAGON;
+ shape = OCTAGON;
else if( !s->compare( "long" ) )
- shape = EPAD::LONG;
+ shape = LONG;
else if( !s->compare( "offset" ) )
- shape = EPAD::OFFSET;
+ shape = OFFSET;
}
rot = parseOptionalEROT( attribs );
@@ -785,7 +785,7 @@ struct EPOLYGON
int layer;
opt_double spacing;
- enum { // for pour
+ enum POUR { // for pour
SOLID,
HATCH,
CUTOUT,
@@ -825,11 +825,11 @@ EPOLYGON::EPOLYGON( CPTREE& aPolygon )
{
// (solid | hatch | cutout)
if( !s->compare( "hatch" ) )
- pour = EPOLYGON::HATCH;
+ pour = HATCH;
else if( !s->compare( "cutout" ) )
- pour = EPOLYGON::CUTOUT;
+ pour = CUTOUT;
else
- pour = EPOLYGON::SOLID;
+ pour = SOLID;
}
orphans = parseOptionalBool( attribs, "orphans" );
diff --git a/pcbnew/gpcb_plugin.cpp b/pcbnew/gpcb_plugin.cpp
index e138efb..25f7646 100644
--- a/pcbnew/gpcb_plugin.cpp
+++ b/pcbnew/gpcb_plugin.cpp
@@ -55,10 +55,6 @@
static const wxString traceFootprintLibrary( wxT( "GedaPcbFootprintLib" ) );
-static const char delims[] = " \t\r\n";
-
-static bool inline isSpace( int c ) { return strchr( delims, c ) != 0; }
-
static void inline traceParams( wxArrayString& aParams )
{
wxString tmp;
diff --git a/pcbnew/legacy_plugin.cpp b/pcbnew/legacy_plugin.cpp
index 8c4cee2..7e43d16 100644
--- a/pcbnew/legacy_plugin.cpp
+++ b/pcbnew/legacy_plugin.cpp
@@ -258,7 +258,7 @@ static inline char* ReadLine( LINE_READER* rdr, const char* caller )
using namespace std; // auto_ptr
-static inline const char* ShowVertJustify( EDA_TEXT_VJUSTIFY_T vertical )
+static const char* ShowVertJustify( EDA_TEXT_VJUSTIFY_T vertical )
{
const char* rs;
switch( vertical )
@@ -271,7 +271,7 @@ static inline const char* ShowVertJustify( EDA_TEXT_VJUSTIFY_T vertical )
return rs;
}
-static inline const char* ShowHorizJustify( EDA_TEXT_HJUSTIFY_T horizontal )
+static const char* ShowHorizJustify( EDA_TEXT_HJUSTIFY_T horizontal )
{
const char* rs;
switch( horizontal )
diff --git a/pcbnew/pcad2kicadpcb_plugin/pcb.h b/pcbnew/pcad2kicadpcb_plugin/pcb.h
index 5d566ff..3073dbe 100644
--- a/pcbnew/pcad2kicadpcb_plugin/pcb.h
+++ b/pcbnew/pcad2kicadpcb_plugin/pcb.h
@@ -59,6 +59,8 @@ public:
int GetNewTimestamp();
int GetNetCode( wxString aNetName );
+ // Bring in parent's Parse with a different signature to avoid hiding it
+ using PCB_MODULE::Parse;
void Parse( wxStatusBar* aStatusBar,
wxXmlDocument* aXmlDoc,
wxString aActualConversion );
diff --git a/pcbnew/pcad2kicadpcb_plugin/pcb_cutout.h b/pcbnew/pcad2kicadpcb_plugin/pcb_cutout.h
index 9f8fa6b..6546ae5 100644
--- a/pcbnew/pcad2kicadpcb_plugin/pcb_cutout.h
+++ b/pcbnew/pcad2kicadpcb_plugin/pcb_cutout.h
@@ -43,6 +43,8 @@ public:
PCB_CUTOUT( PCB_CALLBACKS* aCallbacks, BOARD* aBoard, int aPCadLayer );
~PCB_CUTOUT();
+ // Bring in parent's Parse with a different signature to avoid hiding it
+ using PCB_POLYGON::Parse;
virtual bool Parse( XNODE* aNode,
wxString aDefaultMeasurementUnit,
wxString actualConversion );
diff --git a/pcbnew/pcad2kicadpcb_plugin/pcb_keepout.h b/pcbnew/pcad2kicadpcb_plugin/pcb_keepout.h
index d6b9513..e10215d 100644
--- a/pcbnew/pcad2kicadpcb_plugin/pcb_keepout.h
+++ b/pcbnew/pcad2kicadpcb_plugin/pcb_keepout.h
@@ -42,6 +42,8 @@ public:
PCB_KEEPOUT( PCB_CALLBACKS* aCallbacks, BOARD* aBoard, int aPCadLayer );
~PCB_KEEPOUT();
+ // Bring in parent's Parse with a different signature to avoid hiding it
+ using PCB_POLYGON::Parse;
virtual bool Parse( XNODE* aNode,
wxString aDefaultMeasurementUnit,
wxString aActualConversion );
diff --git a/pcbnew/pcad2kicadpcb_plugin/pcb_pad.h b/pcbnew/pcad2kicadpcb_plugin/pcb_pad.h
index fa18bd5..b471632 100644
--- a/pcbnew/pcad2kicadpcb_plugin/pcb_pad.h
+++ b/pcbnew/pcad2kicadpcb_plugin/pcb_pad.h
@@ -52,6 +52,8 @@ public:
wxString aDefaultMeasurementUnit,
wxString aActualConversion );
virtual void Flip();
+ // Bring in parent's AddToModule with a different signature to avoid hiding it
+ using PCB_COMPONENT::AddToModule;
void AddToModule( MODULE* aModule, int aRotation, bool aEncapsulatedPad );
void AddToBoard();
diff --git a/pcbnew/specctra.cpp b/pcbnew/specctra.cpp
index c7ddafc..caf3f30 100644
--- a/pcbnew/specctra.cpp
+++ b/pcbnew/specctra.cpp
@@ -133,7 +133,7 @@ int SPECCTRA_DB::findLayerName( const std::string& aLayerName ) const
}
-void SPECCTRA_DB::ThrowIOError( const wxString& fmt, ... ) throw( IO_ERROR )
+void SPECCTRA_DB::ThrowIOError( const char* fmt, ... ) throw( IO_ERROR )
{
wxString errText;
va_list args;
diff --git a/pcbnew/specctra.h b/pcbnew/specctra.h
index f6095d0..ae25493 100644
--- a/pcbnew/specctra.h
+++ b/pcbnew/specctra.h
@@ -3917,7 +3917,16 @@ public:
*/
void LoadSESSION( const wxString& filename ) throw( IO_ERROR, boost::bad_pointer );
- void ThrowIOError( const wxString& fmt, ... ) throw( IO_ERROR );
+ /**
+ * Throw an IO error, formatting using wxString::format.
+ *
+ * @param fmt The format string. Note: This is "const char*" rather than
+ * "const wxString&" to avoid potential issues using variadics with a by-ref
+ * as the final argument before the variadic section.
+ *
+ * @param ... The values referenced by the format string.
+ */
+ void ThrowIOError( const char* fmt, ... ) throw( IO_ERROR );
/**
* Function ExportPCB
diff --git a/polygon/math_for_graphics.cpp b/polygon/math_for_graphics.cpp
index d7c4be6..6e6525e 100644
--- a/polygon/math_for_graphics.cpp
+++ b/polygon/math_for_graphics.cpp
@@ -68,7 +68,7 @@ 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;
}
@@ -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)
diff --git a/polygon/poly2tri/sweep/sweep.cc b/polygon/poly2tri/sweep/sweep.cc
index 75e7adf..62e45c5 100644
--- a/polygon/poly2tri/sweep/sweep.cc
+++ b/polygon/poly2tri/sweep/sweep.cc
@@ -702,13 +702,6 @@ void Sweep::FlipEdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle* t,
Triangle& ot = t->NeighborAcross(p);
Point& op = *ot.OppositePoint(*t, p);
- 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");
- assert(0);
- }
-
if (InScanArea(p, *t->PointCCW(p), *t->PointCW(p), op)) {
// Lets rotate shared edge one vertex CW
RotateTrianglePair(*t, p, ot, op);
@@ -780,13 +773,6 @@ void Sweep::FlipScanEdgeEvent(SweepContext& tcx, Point& ep, Point& eq, Triangle&
Triangle& ot = t.NeighborAcross(p);
Point& op = *ot.OppositePoint(t, p);
- 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");
- assert(0);
- }
-
if (InScanArea(eq, *flip_triangle.PointCCW(eq), *flip_triangle.PointCW(eq), op)) {
// flip with new edge op->eq
FlipEdgeEvent(tcx, eq, op, &ot, op);
