From e969bc94ad021144aa204b183d468c7dfd0c91a1 Mon Sep 17 00:00:00 2001 From: whitequark Date: Sat, 7 May 2016 23:34:21 +0000 Subject: [PATCH] Enable -Wall -Wextra -Wno-unused-parameter on GCC/Clang. This is good practice and helps to catch bugs. Several changes were made to accomodate the newly enabled warnings: * -Wunused-function: * in exposed/, static functions that were supposed to be inlined were explicitly marked as inline; * some actually unused functions were removed; * -Wsign-compare: explicit conversions were added, and in the future we should find a nicer way than aux* fields; * -Wmissing-field-initializers: added initializers; * -Wreorder: reordered properly; * -Wunused-but-set-variable: remove variable. -Wunused-parameter was turned off as enabling it would result in massive amount of churn in UI code. Despite that, we should enable it at some point as it has a fairly high SNR otherwise. --- CMakeLists.txt | 2 +- exposed/CDemo.c | 4 +-- extlib/libdxfrw | 2 +- include/slvs.h | 79 ++++++++++++++++++++++---------------------- src/exportvector.cpp | 2 +- src/glhelper.cpp | 2 +- src/gtk/gtkmain.cpp | 20 +++++------ src/importdxf.cpp | 4 --- src/lib.cpp | 8 ++--- src/polygon.cpp | 10 ++---- src/sketch.h | 4 +-- src/solvespace.h | 18 +++++----- src/srf/boolean.cpp | 11 ------ src/srf/surface.cpp | 3 +- src/textscreens.cpp | 2 +- 15 files changed, 74 insertions(+), 97 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 959b66f..73a3af2 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -46,7 +46,7 @@ if(WIN32) endif() if(CMAKE_CXX_COMPILER_ID STREQUAL GNU OR CMAKE_CXX_COMPILER_ID STREQUAL Clang) - set(WARNING_FLAGS "-Wunused-variable") + set(WARNING_FLAGS "-Wall -Wextra -Wno-unused-parameter") set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${WARNING_FLAGS}") set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 ${WARNING_FLAGS}") endif() diff --git a/exposed/CDemo.c b/exposed/CDemo.c index e5830e6..574f100 100644 --- a/exposed/CDemo.c +++ b/exposed/CDemo.c @@ -36,7 +36,7 @@ static void *CheckMalloc(size_t n) * An example of a constraint in 3d. We create a single group, with some * entities and constraints. *---------------------------------------------------------------------------*/ -static void Example3d(void) +void Example3d(void) { /* This will contain a single group, which will arbitrarily number 1. */ Slvs_hGroup g = 1; @@ -88,7 +88,7 @@ static void Example3d(void) * along the reference frame's xy plane. In a second group, we create some * entities in that group and dimension them. *---------------------------------------------------------------------------*/ -static void Example2d(void) +void Example2d(void) { Slvs_hGroup g; double qw, qx, qy, qz; diff --git a/extlib/libdxfrw b/extlib/libdxfrw index a05d16e..62e4a3c 160000 --- a/extlib/libdxfrw +++ b/extlib/libdxfrw @@ -1 +1 @@ -Subproject commit a05d16eaa85c69d662d3b596ca82cbae56443295 +Subproject commit 62e4a3cd8b19b2406af5a01c0bc45e77c9294068 diff --git a/include/slvs.h b/include/slvs.h index 3181ceb..1c97fbf 100644 --- a/include/slvs.h +++ b/include/slvs.h @@ -219,7 +219,7 @@ DLL void Slvs_MakeQuaternion(double ux, double uy, double uz, * out the structures by hand. The code is included in the header file to * let the compiler inline them if possible. */ -static Slvs_Param Slvs_MakeParam(Slvs_hParam h, Slvs_hGroup group, double val) +static inline Slvs_Param Slvs_MakeParam(Slvs_hParam h, Slvs_hGroup group, double val) { Slvs_Param r; r.h = h; @@ -227,9 +227,9 @@ static Slvs_Param Slvs_MakeParam(Slvs_hParam h, Slvs_hGroup group, double val) r.val = val; return r; } -static Slvs_Entity Slvs_MakePoint2d(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hEntity wrkpl, - Slvs_hParam u, Slvs_hParam v) +static inline Slvs_Entity Slvs_MakePoint2d(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hEntity wrkpl, + Slvs_hParam u, Slvs_hParam v) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -241,8 +241,8 @@ static Slvs_Entity Slvs_MakePoint2d(Slvs_hEntity h, Slvs_hGroup group, r.param[1] = v; return r; } -static Slvs_Entity Slvs_MakePoint3d(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hParam x, Slvs_hParam y, Slvs_hParam z) +static inline Slvs_Entity Slvs_MakePoint3d(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hParam x, Slvs_hParam y, Slvs_hParam z) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -255,8 +255,9 @@ static Slvs_Entity Slvs_MakePoint3d(Slvs_hEntity h, Slvs_hGroup group, r.param[2] = z; return r; } -static Slvs_Entity Slvs_MakeNormal3d(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hParam qw, Slvs_hParam qx, Slvs_hParam qy, Slvs_hParam qz) +static inline Slvs_Entity Slvs_MakeNormal3d(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hParam qw, Slvs_hParam qx, + Slvs_hParam qy, Slvs_hParam qz) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -270,8 +271,8 @@ static Slvs_Entity Slvs_MakeNormal3d(Slvs_hEntity h, Slvs_hGroup group, r.param[3] = qz; return r; } -static Slvs_Entity Slvs_MakeNormal2d(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hEntity wrkpl) +static inline Slvs_Entity Slvs_MakeNormal2d(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hEntity wrkpl) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -281,8 +282,8 @@ static Slvs_Entity Slvs_MakeNormal2d(Slvs_hEntity h, Slvs_hGroup group, r.wrkpl = wrkpl; return r; } -static Slvs_Entity Slvs_MakeDistance(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hEntity wrkpl, Slvs_hParam d) +static inline Slvs_Entity Slvs_MakeDistance(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hEntity wrkpl, Slvs_hParam d) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -293,9 +294,9 @@ static Slvs_Entity Slvs_MakeDistance(Slvs_hEntity h, Slvs_hGroup group, r.param[0] = d; return r; } -static Slvs_Entity Slvs_MakeLineSegment(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hEntity wrkpl, - Slvs_hEntity ptA, Slvs_hEntity ptB) +static inline Slvs_Entity Slvs_MakeLineSegment(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hEntity wrkpl, + Slvs_hEntity ptA, Slvs_hEntity ptB) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -307,10 +308,10 @@ static Slvs_Entity Slvs_MakeLineSegment(Slvs_hEntity h, Slvs_hGroup group, r.point[1] = ptB; return r; } -static Slvs_Entity Slvs_MakeCubic(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hEntity wrkpl, - Slvs_hEntity pt0, Slvs_hEntity pt1, - Slvs_hEntity pt2, Slvs_hEntity pt3) +static inline Slvs_Entity Slvs_MakeCubic(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hEntity wrkpl, + Slvs_hEntity pt0, Slvs_hEntity pt1, + Slvs_hEntity pt2, Slvs_hEntity pt3) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -324,11 +325,11 @@ static Slvs_Entity Slvs_MakeCubic(Slvs_hEntity h, Slvs_hGroup group, r.point[3] = pt3; return r; } -static Slvs_Entity Slvs_MakeArcOfCircle(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hEntity wrkpl, - Slvs_hEntity normal, - Slvs_hEntity center, - Slvs_hEntity start, Slvs_hEntity end) +static inline Slvs_Entity Slvs_MakeArcOfCircle(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hEntity wrkpl, + Slvs_hEntity normal, + Slvs_hEntity center, + Slvs_hEntity start, Slvs_hEntity end) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -342,10 +343,10 @@ static Slvs_Entity Slvs_MakeArcOfCircle(Slvs_hEntity h, Slvs_hGroup group, r.point[2] = end; return r; } -static Slvs_Entity Slvs_MakeCircle(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hEntity wrkpl, - Slvs_hEntity center, - Slvs_hEntity normal, Slvs_hEntity radius) +static inline Slvs_Entity Slvs_MakeCircle(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hEntity wrkpl, + Slvs_hEntity center, + Slvs_hEntity normal, Slvs_hEntity radius) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -358,8 +359,8 @@ static Slvs_Entity Slvs_MakeCircle(Slvs_hEntity h, Slvs_hGroup group, r.distance = radius; return r; } -static Slvs_Entity Slvs_MakeWorkplane(Slvs_hEntity h, Slvs_hGroup group, - Slvs_hEntity origin, Slvs_hEntity normal) +static inline Slvs_Entity Slvs_MakeWorkplane(Slvs_hEntity h, Slvs_hGroup group, + Slvs_hEntity origin, Slvs_hEntity normal) { Slvs_Entity r; memset(&r, 0, sizeof(r)); @@ -372,15 +373,15 @@ static Slvs_Entity Slvs_MakeWorkplane(Slvs_hEntity h, Slvs_hGroup group, return r; } -static Slvs_Constraint Slvs_MakeConstraint(Slvs_hConstraint h, - Slvs_hGroup group, - int type, - Slvs_hEntity wrkpl, - double valA, - Slvs_hEntity ptA, - Slvs_hEntity ptB, - Slvs_hEntity entityA, - Slvs_hEntity entityB) +static inline Slvs_Constraint Slvs_MakeConstraint(Slvs_hConstraint h, + Slvs_hGroup group, + int type, + Slvs_hEntity wrkpl, + double valA, + Slvs_hEntity ptA, + Slvs_hEntity ptB, + Slvs_hEntity entityA, + Slvs_hEntity entityB) { Slvs_Constraint r; memset(&r, 0, sizeof(r)); diff --git a/src/exportvector.cpp b/src/exportvector.cpp index 0f301ad..dd4dcbf 100644 --- a/src/exportvector.cpp +++ b/src/exportvector.cpp @@ -182,7 +182,7 @@ public: bool used = false; for(DxfFileWriter::BezierPath &path : writer->paths) { for(SBezier *sb : path.beziers) { - if(sb->auxA != s->h.v) continue; + if((uint32_t)sb->auxA != s->h.v) continue; used = true; break; } diff --git a/src/glhelper.cpp b/src/glhelper.cpp index a629a8e..f789121 100644 --- a/src/glhelper.cpp +++ b/src/glhelper.cpp @@ -191,7 +191,7 @@ void ssglWriteText(const std::string &str, double h, Vector t, Vector u, Vector bool gridFit = !SS.exportMode && u.Equals(SS.GW.projRight) && v.Equals(SS.GW.projUp); double scale = FONT_SCALE(h) / SS.GW.scale; - Vector o = { 3840.0, 3840.0 }; + Vector o = { 3840.0, 3840.0, 0.0 }; for(char32_t chr : ReadUTF8(str)) { const VectorGlyph &glyph = GetVectorGlyph(chr); o.x += ssglDrawCharacter(glyph, t, o, u, v, scale, fn, fndata, gridFit); diff --git a/src/gtk/gtkmain.cpp b/src/gtk/gtkmain.cpp index 4900588..5dada39 100644 --- a/src/gtk/gtkmain.cpp +++ b/src/gtk/gtkmain.cpp @@ -349,7 +349,7 @@ protected: } #ifdef HAVE_GTK2 - virtual bool on_expose_event(GdkEventExpose *event) { + virtual bool on_expose_event(GdkEventExpose *) { return on_draw(get_window()->create_cairo_context()); } #endif @@ -590,7 +590,7 @@ protected: return true; } - virtual bool on_leave_notify_event (GdkEventCrossing*event) { + virtual bool on_leave_notify_event (GdkEventCrossing *) { SS.GW.MouseLeave(); return true; @@ -688,7 +688,7 @@ protected: Gtk::Window::on_hide(); } - virtual bool on_delete_event(GdkEventAny *event) { + virtual bool on_delete_event(GdkEventAny *) { SS.Exit(); return true; @@ -884,7 +884,7 @@ public: MainMenuItem(const GraphicsWindow::MenuEntry &entry) : MenuItem(), _entry(entry), _synthetic(false) { Glib::ustring label(_entry.label); - for(int i = 0; i < label.length(); i++) { + for(size_t i = 0; i < label.length(); i++) { if(label[i] == '&') label.replace(i, 1, "_"); } @@ -956,7 +956,7 @@ static void InitMainMenu(Gtk::MenuShell *menu_shell) { Gtk::Menu *menu = new Gtk::Menu; menu_item->set_submenu(*menu); - if(entry->level >= sizeof(levels) / sizeof(levels[0])) + if((unsigned)entry->level >= sizeof(levels) / sizeof(levels[0])) oops(); levels[entry->level] = menu; @@ -997,10 +997,6 @@ void EnableMenuById(int id, bool enabled) { main_menu_items[id]->set_sensitive(enabled); } -static void ActivateMenuById(int id) { - main_menu_items[id]->activate(); -} - void CheckMenuById(int id, bool checked) { ((MainMenuItem*)main_menu_items[id])->set_active(checked); } @@ -1301,7 +1297,7 @@ protected: return true; } - virtual bool on_leave_notify_event (GdkEventCrossing*event) { + virtual bool on_leave_notify_event (GdkEventCrossing *) { SS.TW.MouseLeave(); return true; @@ -1367,7 +1363,7 @@ protected: Gtk::Window::on_hide(); } - virtual bool on_delete_event(GdkEventAny *event) { + virtual bool on_delete_event(GdkEventAny *) { /* trigger the action and ignore the request */ GraphicsWindow::MenuView(GraphicsWindow::MNU_SHOW_TEXT_WND); @@ -1477,7 +1473,7 @@ std::vector GetFontFiles() { /* Space Navigator support */ #ifdef HAVE_SPACEWARE -static GdkFilterReturn GdkSpnavFilter(GdkXEvent *gxevent, GdkEvent *event, gpointer data) { +static GdkFilterReturn GdkSpnavFilter(GdkXEvent *gxevent, GdkEvent *, gpointer) { XEvent *xevent = (XEvent*) gxevent; spnav_event sev; diff --git a/src/importdxf.cpp b/src/importdxf.cpp index e8d7804..a3a1e58 100644 --- a/src/importdxf.cpp +++ b/src/importdxf.cpp @@ -340,10 +340,6 @@ public: } hStyle styleFor(const DRW_Entity *e) { - DRW_Layer *layer = NULL; - auto bi = layers.find(e->layer); - if(bi != layers.end()) layer = &bi->second; - // Color. // TODO: which color to choose: index or RGB one? int col = getColor(e); diff --git a/src/lib.cpp b/src/lib.cpp index cb9fca2..d87a4f0 100644 --- a/src/lib.cpp +++ b/src/lib.cpp @@ -13,22 +13,22 @@ static System SYS; static int IsInit = 0; -void Group::GenerateEquations(IdList *l) { +void Group::GenerateEquations(IdList *) { // Nothing to do for now. } -void SolveSpace::CnfFreezeInt(uint32_t v, const std::string &name) +void SolveSpace::CnfFreezeInt(uint32_t, const std::string &) { abort(); } -uint32_t SolveSpace::CnfThawInt(uint32_t v, const std::string &name) +uint32_t SolveSpace::CnfThawInt(uint32_t, const std::string &) { abort(); return 0; } -void SolveSpace::DoMessageBox(const char *str, int rows, int cols, bool error) +void SolveSpace::DoMessageBox(const char *, int, int, bool) { abort(); } diff --git a/src/polygon.cpp b/src/polygon.cpp index 7e1cfb7..eae40fe 100644 --- a/src/polygon.cpp +++ b/src/polygon.cpp @@ -67,7 +67,6 @@ bool SEdge::EdgeCrosses(Vector ea, Vector eb, Vector *ppi, SPointList *spl) { Vector d = eb.Minus(ea); double t_eps = LENGTH_EPS/d.Magnitude(); - double dist_a, dist_b; double t, tthis; bool skew; Vector pi; @@ -84,15 +83,12 @@ bool SEdge::EdgeCrosses(Vector ea, Vector eb, Vector *ppi, SPointList *spl) { return true; } - dist_a = a.DistanceToLine(ea, d), - dist_b = b.DistanceToLine(ea, d); - - // Can't just test if dist_a equals dist_b; they could be on opposite - // sides, since it's unsigned. + // Can't just test if distance between d and a equals distance between d and b; + // they could be on opposite sides, since we don't have the sign. double m = sqrt(d.Magnitude()*dthis.Magnitude()); if(sqrt(fabs(d.Dot(dthis))) > (m - LENGTH_EPS)) { // The edges are parallel. - if(fabs(dist_a) > LENGTH_EPS) { + if(fabs(a.DistanceToLine(ea, d)) > LENGTH_EPS) { // and not coincident, so can't be interesecting return false; } diff --git a/src/sketch.h b/src/sketch.h index 492fa33..111aef7 100644 --- a/src/sketch.h +++ b/src/sketch.h @@ -461,8 +461,8 @@ public: // POD members with indeterminate value. Entity() : EntityBase({}), forceHidden(), actPoint(), actNormal(), actDistance(), actVisible(), style(), construction(), - dogd(), beziers(), edges(), edgesChordTol(), screenBBox(), - screenBBoxValid() {}; + beziers(), edges(), edgesChordTol(), screenBBox(), screenBBoxValid(), + dogd() {}; // A linked entity that was hidden in the source file ends up hidden // here too. diff --git a/src/solvespace.h b/src/solvespace.h index 2306a18..46debb9 100644 --- a/src/solvespace.h +++ b/src/solvespace.h @@ -159,12 +159,12 @@ struct FileFilter { // SolveSpace native file format const FileFilter SlvsFileFilter[] = { { "SolveSpace models", { "slvs" } }, - { NULL } + { NULL, {} } }; // PNG format bitmap const FileFilter PngFileFilter[] = { { "PNG", { "png" } }, - { NULL } + { NULL, {} } }; // Triangle mesh const FileFilter MeshFileFilter[] = { @@ -172,12 +172,12 @@ const FileFilter MeshFileFilter[] = { { "Wavefront OBJ mesh", { "obj" } }, { "Three.js-compatible mesh, with viewer", { "html" } }, { "Three.js-compatible mesh, mesh only", { "js" } }, - { NULL } + { NULL, {} } }; // NURBS surfaces const FileFilter SurfaceFileFilter[] = { { "STEP file", { "step", "stp" } }, - { NULL } + { NULL, {} } }; // 2d vector (lines and curves) format const FileFilter VectorFileFilter[] = { @@ -188,23 +188,23 @@ const FileFilter VectorFileFilter[] = { { "DXF file (AutoCAD 2007)", { "dxf" } }, { "HPGL file", { "plt", "hpgl" } }, { "G Code", { "ngc", "txt" } }, - { NULL } + { NULL, {} } }; // 3d vector (wireframe lines and curves) format const FileFilter Vector3dFileFilter[] = { { "STEP file", { "step", "stp" } }, { "DXF file (AutoCAD 2007)", { "dxf" } }, - { NULL } + { NULL, {} } }; // All Importable formats const FileFilter ImportableFileFilter[] = { { "AutoCAD DXF and DWG files", { "dxf", "dwg" } }, - { NULL } + { NULL, {} } }; // Comma-separated value, like a spreadsheet would use const FileFilter CsvFileFilter[] = { { "CSV", { "csv" } }, - { NULL } + { NULL, {} } }; bool GetSaveFile(std::string *filename, const std::string &defExtension, @@ -506,7 +506,7 @@ public: void BezierAsPwl(SBezier *sb); void BezierAsNonrationalCubic(SBezier *sb, int depth=0); - virtual bool OutputConstraints(IdList *constraint) { return false; } + virtual bool OutputConstraints(IdList *) { return false; } virtual bool CanOutputMesh() const { return false; } virtual void StartPath( RgbaColor strokeRgb, double lineWidth, diff --git a/src/srf/boolean.cpp b/src/srf/boolean.cpp index 802aaa5..d064952 100644 --- a/src/srf/boolean.cpp +++ b/src/srf/boolean.cpp @@ -286,17 +286,6 @@ static void DEBUGEDGELIST(SEdgeList *sel, SSurface *surf) { } } -static const char *REGION(int d) { - switch(d) { - case SShell::INSIDE: return "inside"; - case SShell::OUTSIDE: return "outside"; - case SShell::COINC_SAME: return "same"; - case SShell::COINC_OPP: return "opp"; - default: return "xxx"; - } -} - - //----------------------------------------------------------------------------- // We are given src, with at least one edge, and avoid, a list of points to // avoid. We return a chain of edges (that share endpoints), such that no diff --git a/src/srf/surface.cpp b/src/srf/surface.cpp index 5e162b6..9b36cb9 100644 --- a/src/srf/surface.cpp +++ b/src/srf/surface.cpp @@ -636,7 +636,7 @@ void SShell::MakeFromRevolutionOf(SBezierLoopSet *sbls, Vector pt, Vector axis, // Now we actually build and trim the surfaces. for(sbl = sbls->l.First(); sbl; sbl = sbls->l.NextAfter(sbl)) { int i, j; - SBezier *sb, *prev; + SBezier *sb; List hsl = {}; for(sb = sbl->l.First(); sb; sb = sbl->l.NextAfter(sb)) { @@ -673,7 +673,6 @@ void SShell::MakeFromRevolutionOf(SBezierLoopSet *sbls, Vector pt, Vector axis, revsp = hsl.elem[WRAP(i-1, sbl->l.n)]; sb = &(sbl->l.elem[i]); - prev = &(sbl->l.elem[WRAP(i-1, sbl->l.n)]); for(j = 0; j < 4; j++) { SCurve sc; diff --git a/src/textscreens.cpp b/src/textscreens.cpp index 01b0286..e00b2bd 100644 --- a/src/textscreens.cpp +++ b/src/textscreens.cpp @@ -195,7 +195,7 @@ void TextWindow::ScreenChangeGroupOption(int link, uint32_t v) { // When an extrude group is first created, it's positioned for a union // extrusion. If no constraints were added, flip it when we switch between // union and difference modes to avoid manual work doing the smae. - if(g->meshCombine != v && g->GetNumConstraints() == 0 && + if(g->meshCombine != (int)v && g->GetNumConstraints() == 0 && (v == Group::COMBINE_AS_DIFFERENCE || g->meshCombine == Group::COMBINE_AS_DIFFERENCE)) { g->ExtrusionForceVectorTo(g->ExtrusionGetVector().Negated());