EntityRange::end() causes invalid memory read

[w1th0utnam3]

Detected using valgrind:

==939== Invalid read of size 4
==939==    at 0xD68DE32: dolfin::mesh::MeshEntityIterator<dolfin::mesh::MeshEntity>::MeshEntityIterator(dolfin::mesh::MeshEntity const&, unsigned long, unsigned long) (MeshIterator.h:110)
==939==    by 0xD7E22E5: end (MeshIterator.h:374)
==939==    by 0xD7E22E5: dolfin::mesh::TopologyComputation::compute_from_transpose(dolfin::mesh::Mesh&, unsigned long, unsigned long) (TopologyComputation.cpp:355)

The end() method of the EntityRange constructs an EntityIterator: https://github.com/FEniCS/dolfinx/blob/64d27fd1fead5b874f4da9e096b99eb6c08a7a7a/cpp/dolfin/mesh/MeshIterator.h#L374 where n points beyond the last element of the range. In the Iterator's constructor, this address is dereferenced: https://github.com/FEniCS/dolfinx/blob/64d27fd1fead5b874f4da9e096b99eb6c08a7a7a/cpp/dolfin/mesh/MeshIterator.h#L110 causing an out of bounds read. It's not fatal but still undefined behavior.

Possible fixes:

• Use a separate sentinel type for end iterators that only allows comparison with the default iterators
• Additional constructors/factory methods for end iterators that don't dereference the memory
• Add a hacky conditional that doesn't dereference the address if the arguments point after the end, however this may not be nice for finding bugs where one accidentally does this.

[chrisrichardson]

Can we just delete line 110?

[w1th0utnam3]

I think that should work. The _local_index is updated in the dereferencing operators of the iterator anyway. So these statements in the constructors seem redundant. I'll make a PR.