Skip to content

Commit

Permalink
fix shadow problems (Closes #857)
Browse files Browse the repository at this point in the history 
adding -Wshadow to compilation (on cmake)
This is a combination of 16 commits.
  • Loading branch information
@cvvergara
cvvergara committed Aug 29, 2018
1 parent 5ecabd6 commit e7adbc5
Show file tree
Hide file tree
Showing 86 changed files with 540 additions and 545 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ if(CMAKE_COMPILER_IS_GNUCC OR CMAKE_COMPILER_IS_GNUCXX)
"Forcing IEEE 754 using flag -ffloat-store - ${GEOS_ENABLE_FLOATSTORE}")

# Warnings specification
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wno-long-long -fno-implicit-inline-templates -Wconversion -pedantic -W -Wunused -Wuninitialized -Wextra -Wdouble-promotion")
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wall -Wno-long-long -fno-implicit-inline-templates -Wconversion -pedantic -W -Wunused -Wuninitialized -Wextra -Wdouble-promotion -Wshadow")

# Turn on Position Independent Code generation for GEOS C shared library
set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC -Wall -Wconversion -pedantic -Wmissing-prototypes -W -Wunused -Wuninitialized -Wextra -Wdouble-promotion")
Expand Down
4 changes: 2 additions & 2 deletions include/geos/algorithm/RayCrossingCounter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ class GEOS_DLL RayCrossingCounter
const geom::Coordinate& p2,
const geom::Coordinate& q);

RayCrossingCounter(const geom::Coordinate& point)
: point( point),
RayCrossingCounter(const geom::Coordinate& p_point)
: point( p_point),
crossingCount( 0),
isPointOnSegment( false)
{ }
Expand Down
8 changes: 4 additions & 4 deletions include/geos/algorithm/distance/DiscreteFrechetDistance.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,11 @@ class GEOS_DLL DiscreteFrechetDistance
static double distance(const geom::Geometry& g0,
const geom::Geometry& g1, double densifyFrac);

DiscreteFrechetDistance(const geom::Geometry& g0,
const geom::Geometry& g1)
DiscreteFrechetDistance(const geom::Geometry& p_g0,
const geom::Geometry& p_g1)
:
g0(g0),
g1(g1),
g0(p_g0),
g1(p_g1),
ptDist(),
densifyFrac(0.0)
{}
Expand Down
24 changes: 12 additions & 12 deletions include/geos/algorithm/distance/DiscreteHausdorffDistance.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,11 +107,11 @@ class GEOS_DLL DiscreteHausdorffDistance
static double distance(const geom::Geometry& g0,
const geom::Geometry& g1, double densifyFrac);

DiscreteHausdorffDistance(const geom::Geometry& g0,
const geom::Geometry& g1)
DiscreteHausdorffDistance(const geom::Geometry& p_g0,
const geom::Geometry& p_g1)
:
g0(g0),
g1(g1),
g0(p_g0),
g1(p_g1),
ptDist(),
densifyFrac(0.0)
{}
Expand Down Expand Up @@ -155,9 +155,9 @@ class GEOS_DLL DiscreteHausdorffDistance
class MaxPointDistanceFilter : public geom::CoordinateFilter
{
public:
MaxPointDistanceFilter(const geom::Geometry& geom)
MaxPointDistanceFilter(const geom::Geometry& p_geom)
:
geom(geom)
geom(p_geom)
{}

void filter_ro(const geom::Coordinate* pt) override
Expand Down Expand Up @@ -190,9 +190,9 @@ class GEOS_DLL DiscreteHausdorffDistance
public:

MaxDensifiedByFractionDistanceFilter(
const geom::Geometry& geom, double fraction)
const geom::Geometry& p_geom, double fraction)
:
geom(geom),
geom(p_geom),
numSubSegs( std::size_t(util::round(1.0/fraction)) )
{
}
Expand Down Expand Up @@ -221,11 +221,11 @@ class GEOS_DLL DiscreteHausdorffDistance

private:

void compute(const geom::Geometry& g0,
const geom::Geometry& g1)
void compute(const geom::Geometry& p_g0,
const geom::Geometry& p_g1)
{
computeOrientedDistance(g0, g1, ptDist);
computeOrientedDistance(g1, g0, ptDist);
computeOrientedDistance(p_g0, p_g1, ptDist);
computeOrientedDistance(p_g1, p_g0, ptDist);
}

void computeOrientedDistance(const geom::Geometry& discreteGeom,
Expand Down
4 changes: 2 additions & 2 deletions include/geos/algorithm/locate/IndexedPointInAreaLocator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ class IndexedPointInAreaLocator : public PointOnGeometryLocator
algorithm::RayCrossingCounter * counter;

public:
SegmentVisitor( algorithm::RayCrossingCounter * counter)
: counter( counter)
SegmentVisitor( algorithm::RayCrossingCounter * p_counter)
: counter( p_counter)
{ }

~SegmentVisitor() override
Expand Down
4 changes: 2 additions & 2 deletions include/geos/algorithm/locate/SimplePointInAreaLocator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ class SimplePointInAreaLocator : public PointOnGeometryLocator
static bool containsPointInPolygon(const geom::Coordinate& p,
const geom::Polygon *poly);

SimplePointInAreaLocator( const geom::Geometry * g)
: g( g)
SimplePointInAreaLocator( const geom::Geometry * p_g)
: g( p_g)
{ }

int locate( const geom::Coordinate * p) override
Expand Down
10 changes: 5 additions & 5 deletions include/geos/geom/prep/AbstractPreparedPolygonContains.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,20 +109,20 @@ class AbstractPreparedPolygonContains : public PreparedPolygonPredicate
virtual bool fullTopologicalPredicate( const geom::Geometry * geom) =0;

public:
AbstractPreparedPolygonContains( const PreparedPolygon * const prepPoly)
: PreparedPolygonPredicate( prepPoly),
AbstractPreparedPolygonContains( const PreparedPolygon * const p_prepPoly)
: PreparedPolygonPredicate( p_prepPoly),
hasSegmentIntersection( false),
hasProperIntersection( false),
hasNonProperIntersection( false),
requireSomePointInInterior(true)
{ }

AbstractPreparedPolygonContains( const PreparedPolygon * const prepPoly, bool requireSomePointInInterior)
: PreparedPolygonPredicate( prepPoly),
AbstractPreparedPolygonContains( const PreparedPolygon * const p_prepPoly, bool p_requireSomePointInInterior)
: PreparedPolygonPredicate( p_prepPoly),
hasSegmentIntersection( false),
hasProperIntersection( false),
hasNonProperIntersection( false),
requireSomePointInInterior(requireSomePointInInterior)
requireSomePointInInterior(p_requireSomePointInInterior)
{ }

~AbstractPreparedPolygonContains() override
Expand Down
4 changes: 2 additions & 2 deletions include/geos/geom/prep/PreparedPolygonPredicate.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,8 +128,8 @@ class PreparedPolygonPredicate
*
* @param prepPoly the PreparedPolygon to evaluate
*/
PreparedPolygonPredicate( const PreparedPolygon * const prepPoly)
: prepPoly( prepPoly)
PreparedPolygonPredicate( const PreparedPolygon * const p_prepPoly)
: prepPoly( p_prepPoly)
{ }

virtual ~PreparedPolygonPredicate()
Expand Down
8 changes: 4 additions & 4 deletions include/geos/geom/util/GeometryExtracter.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,15 @@ class GEOS_DLL GeometryExtracter {
template <class ComponentType, class TargetContainer>
static void extract(const Geometry& geom, TargetContainer& lst)
{
if ( const ComponentType* c = dynamic_cast<const ComponentType*>(&geom) )
if ( const ComponentType* p_c = dynamic_cast<const ComponentType*>(&geom) )
{
lst.push_back(c);
lst.push_back(p_c);
}
else if ( const GeometryCollection* c =
else if ( const GeometryCollection* p_c1 =
dynamic_cast<const GeometryCollection*>(&geom) )
{
GeometryExtracter::Extracter<ComponentType, TargetContainer> extracter(lst);
c->apply_ro(&extracter);
p_c1->apply_ro(&extracter);
}
}

Expand Down
6 changes: 3 additions & 3 deletions include/geos/index/intervalrtree/IntervalRTreeLeafNode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ class IntervalRTreeLeafNode : public IntervalRTreeNode
public:

/// @param item externally owned
IntervalRTreeLeafNode( double min, double max, void * item)
: IntervalRTreeNode( min, max),
item( item)
IntervalRTreeLeafNode( double p_min, double p_max, void * p_item)
: IntervalRTreeNode( p_min, p_max),
item( p_item)
{ }

~IntervalRTreeLeafNode() override
Expand Down
6 changes: 3 additions & 3 deletions include/geos/index/intervalrtree/IntervalRTreeNode.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ class IntervalRTreeNode
max( DoubleNegInfinity )
{ }

IntervalRTreeNode( double min, double max)
: min( min ),
max( max )
IntervalRTreeNode( double p_min, double p_max)
: min( p_min ),
max( p_max )
{ }

virtual ~IntervalRTreeNode()
Expand Down
4 changes: 2 additions & 2 deletions include/geos/linearref/LocationIndexedLine.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ class GEOS_DLL LocationIndexedLine
*
* @param linearGeom the linear geometry to reference along
*/
LocationIndexedLine(const geom::Geometry *linearGeom)
: linearGeom(linearGeom)
LocationIndexedLine(const geom::Geometry *p_linearGeom)
: linearGeom(p_linearGeom)
{
checkGeometryType();
}
Expand Down
4 changes: 2 additions & 2 deletions include/geos/noding/MCIndexSegmentSetMutualIntersector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ class MCIndexSegmentSetMutualIntersector : public SegmentSetMutualIntersector
SegmentOverlapAction& operator=(const SegmentOverlapAction& rhs) = delete;

public:
SegmentOverlapAction(SegmentIntersector & si) :
index::chain::MonotoneChainOverlapAction(), si(si)
SegmentOverlapAction(SegmentIntersector & p_si) :
index::chain::MonotoneChainOverlapAction(), si(p_si)
{}

void overlap(index::chain::MonotoneChain& mc1, std::size_t start1,
Expand Down
6 changes: 3 additions & 3 deletions include/geos/noding/OrientedCoordinateArray.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,10 @@ class GEOS_DLL OrientedCoordinateArray
*
* @param pts the coordinates to orient
*/
OrientedCoordinateArray(const geom::CoordinateSequence& pts)
OrientedCoordinateArray(const geom::CoordinateSequence& p_pts)
:
pts(&pts),
orientationVar(orientation(pts))
pts(&p_pts),
orientationVar(orientation(p_pts))
{
}

Expand Down
12 changes: 6 additions & 6 deletions include/geos/noding/SegmentIntersectionDetector.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,9 @@ class SegmentIntersectionDetector : public SegmentIntersector

protected:
public:
SegmentIntersectionDetector( LineIntersector * li)
SegmentIntersectionDetector( LineIntersector * p_li)
:
li( li),
li( p_li),
findProper(false),
findAllTypes(false),
_hasIntersection(false),
Expand All @@ -77,14 +77,14 @@ class SegmentIntersectionDetector : public SegmentIntersector
}


void setFindProper( bool findProper)
void setFindProper( bool p_findProper)
{
this->findProper = findProper;
this->findProper = p_findProper;
}

void setFindAllIntersectionTypes( bool findAllTypes)
void setFindAllIntersectionTypes( bool p_findAllTypes)
{
this->findAllTypes = findAllTypes;
this->findAllTypes = p_findAllTypes;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions include/geos/triangulate/DelaunayTriangulationBuilder.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ class GEOS_DLL DelaunayTriangulationBuilder
*
* @param tolerance the tolerance distance to use
*/
inline void setTolerance(double tolerance)
inline void setTolerance(double p_tolerance)
{
this->tolerance = tolerance;
this->tolerance = p_tolerance;
}

private:
Expand Down
4 changes: 2 additions & 2 deletions include/geos/triangulate/quadedge/QuadEdge.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,8 @@ class GEOS_DLL QuadEdge {
*
* @param nextEdge edge
*/
inline void setNext(QuadEdge *next) {
this->next = next;
inline void setNext(QuadEdge *p_next) {
this->next = p_next;
}

/***************************************************************************
Expand Down
4 changes: 2 additions & 2 deletions include/geos/triangulate/quadedge/QuadEdgeSubdivision.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ class GEOS_DLL QuadEdgeSubdivision {
* @param locator
* a QuadEdgeLocator
*/
inline void setLocator(std::unique_ptr<QuadEdgeLocator> locator) {
this->locator = std::move(locator);
inline void setLocator(std::unique_ptr<QuadEdgeLocator> p_locator) {
this->locator = std::move(p_locator);
}

/**
Expand Down
10 changes: 5 additions & 5 deletions src/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
#
# This is free software; you can redistribute and/or modify it under
# the terms of the GNU Lesser General Public Licence as published
# by the Free Software Foundation.
# by the Free Software Foundation.
# See the COPYING file for more information.
#
#################################################################################
Expand All @@ -26,14 +26,14 @@ if(GEOS_ENABLE_MACOSX_FRAMEWORK)

add_library(GEOS SHARED ${geos_SOURCES} ${geos_c_SOURCES})

math(EXPR CVERSION "${VERSION_MAJOR} + 1")
# VERSION = current version, SOVERSION = compatibility version
math(EXPR CVERSION "${VERSION_MAJOR} + 1")
# VERSION = current version, SOVERSION = compatibility version
set_target_properties(GEOS
PROPERTIES
CLEAN_DIRECT_OUTPUT 1
FRAMEWORK 1
VERSION "${CVERSION}.${VERSION_MINOR}.${VERSION_PATCH}"
SOVERSION ${CVERSION}
VERSION "${CVERSION}.${VERSION_MINOR}.${VERSION_PATCH}"
SOVERSION ${CVERSION}
FRAMEWORK_VERSION ${VERSION_MAJOR}
BUILD_WITH_INSTALL_RPATH TRUE
INSTALL_NAME_DIR "${CMAKE_INSTALL_PREFIX}"
Expand Down
42 changes: 21 additions & 21 deletions src/algorithm/ConvexHull.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,37 +118,37 @@ ConvexHull::toCoordinateSequence(Coordinate::ConstVect &cv)

/* private */
void
ConvexHull::computeOctPts(const Coordinate::ConstVect &inputPts,
ConvexHull::computeOctPts(const Coordinate::ConstVect &p_inputPts,
Coordinate::ConstVect &pts)
{
// Initialize all slots with first input coordinate
pts = Coordinate::ConstVect(8, inputPts[0]);
pts = Coordinate::ConstVect(8, p_inputPts[0]);

for (size_t i=1, n=inputPts.size(); i<n; ++i)
for (size_t i=1, n=p_inputPts.size(); i<n; ++i)
{
if (inputPts[i]->x < pts[0]->x) {
pts[0] = inputPts[i];
if (p_inputPts[i]->x < pts[0]->x) {
pts[0] = p_inputPts[i];
}
if (inputPts[i]->x - inputPts[i]->y < pts[1]->x - pts[1]->y) {
pts[1] = inputPts[i];
if (p_inputPts[i]->x - p_inputPts[i]->y < pts[1]->x - pts[1]->y) {
pts[1] = p_inputPts[i];
}
if (inputPts[i]->y > pts[2]->y) {
pts[2] = inputPts[i];
if (p_inputPts[i]->y > pts[2]->y) {
pts[2] = p_inputPts[i];
}
if (inputPts[i]->x + inputPts[i]->y > pts[3]->x + pts[3]->y) {
pts[3] = inputPts[i];
if (p_inputPts[i]->x + p_inputPts[i]->y > pts[3]->x + pts[3]->y) {
pts[3] = p_inputPts[i];
}
if (inputPts[i]->x > pts[4]->x) {
pts[4] = inputPts[i];
if (p_inputPts[i]->x > pts[4]->x) {
pts[4] = p_inputPts[i];
}
if (inputPts[i]->x - inputPts[i]->y > pts[5]->x - pts[5]->y) {
pts[5] = inputPts[i];
if (p_inputPts[i]->x - p_inputPts[i]->y > pts[5]->x - pts[5]->y) {
pts[5] = p_inputPts[i];
}
if (inputPts[i]->y < pts[6]->y) {
pts[6] = inputPts[i];
if (p_inputPts[i]->y < pts[6]->y) {
pts[6] = p_inputPts[i];
}
if (inputPts[i]->x + inputPts[i]->y < pts[7]->x + pts[7]->y) {
pts[7] = inputPts[i];
if (p_inputPts[i]->x + p_inputPts[i]->y < pts[7]->x + pts[7]->y) {
pts[7] = p_inputPts[i];
}
}

Expand All @@ -157,10 +157,10 @@ ConvexHull::computeOctPts(const Coordinate::ConstVect &inputPts,

/* private */
bool
ConvexHull::computeOctRing(const Coordinate::ConstVect &inputPts,
ConvexHull::computeOctRing(const Coordinate::ConstVect &p_inputPts,
Coordinate::ConstVect &dest)
{
computeOctPts(inputPts, dest);
computeOctPts(p_inputPts, dest);

// Remove consecutive equal Coordinates
// unique() returns an iterator to the end of the resulting
Expand Down
Loading

3 comments on commit e7adbc5

@hobu
Copy link
Member

@hobu hobu commented on e7adbc5 Sep 17, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused how this kind of thing makes the code better, and the consequences for blind application are leaks as seen https://trac.osgeo.org/geos/ticket/923#comment:1

@cvvergara
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have loved to have the commits added individually, (they were 16) so your comment would be in the commit that caused the problem.
Also I would have loved to have the leak test mentioned on the ticket 923 as part of the test suite.
But none of both things happened.
And all of the tests passed.
But I am working on fixing it.

@hobu
Copy link
Member

@hobu hobu commented on e7adbc5 Sep 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point about these kinds of changes providing little benefit for unexpected cost still stands. "There were no tests to catch my breaking things" isn't a satisfactory excuse.

Why make these kinds of changes to the code base?

Please sign in to comment.