Fix some small issues that were flagged by upcoming lint checks. #138
Conversation
s2/shapeindex.go
Outdated
| @@ -1466,7 +1466,7 @@ func (s *ShapeIndex) absorbIndexCell(p *PaddedCell, iter *ShapeIndexIterator, ed | |||
| } | |||
|
|
|||
| // Update the edge list and delete this cell from the index. | |||
| edges, newEdges = newEdges, edges | |||
| edges, newEdges = newEdges, edges //nolint | |||
There was a problem hiding this comment.
What's the lint check being disabled?
There was a problem hiding this comment.
It's complaining about the swap of pointers.
s2/shapeindex.go:1469:9: ineffectual assignment to newEdges (ineffassign)
edges, newEdges = newEdges, edges
^
i.e. https://github.com/google/s2geometry/blob/master/src/s2/mutable_s2shape_index.cc#L1670
at the end of absorb cell we swap the new edges with the existing for the return leaving the one swapped out as dangling here.
it seems to compile and run ok if I do a, _= b, a though it's not clear if that leaves a loose pointer around.
There was a problem hiding this comment.
Isn't this saying that edges and newEdges aren't used after this? Should the argument be a pointer to a slice?
Tests in C++ fail if I delete the corresponding line, but not in Go.
I would leave the lint error for now and add a TODO.
No description provided.