That does look nicer, so it's worth switching to end pointers that are one past the last element. Here is my final code:

template <typename T1, typename T2>

bool imagesOverlap(ImageBase<T1> const& image1, ImageBase<T2> const& image2) {

    auto arr1 = image1.getArray();

    auto beg1Addr = reinterpret_cast<void const *>(arr1.front().begin());

    auto end1Addr = reinterpret_cast<void const *>(arr1.back().end());

    auto arr2 = image2.getArray();

    auto beg2Addr = reinterpret_cast<void const *>(arr2.front().begin());

    auto end2Addr = reinterpret_cast<void const *>(arr2.back().end());

    return beg1Addr < end2Addr && beg2Addr < end1Addr;

}

with an overload that works on masked images by calling the version above on the image, mask and variance planes.

While reviewing this ticket I learned about some very unintuitive behavior in C++ pointer arithmetic, which unfortunately affects imagesOverlap. However, I think a small change to the code (using std::less instead of <) will reduce the problem to the point where it's unlikely to come up with any real compiler. I also recommend a few extra tests of imagesOverlap to cover edge cases.

Jim Bosch we'd appreciate your opinion on whether we should worry about false positives when testing ranges of pointers (as per some comments on the pull request). The options I see are:

• Continue to test ranges and hope for the best
• Do not test anything. Just document that you should not try to warp an image into an overlapping bit of memory (e.g. a sub-view). This has been the state of our stack for years and I do not recall any problems, so I think this is a perfectly viable option and it avoids any concerns about unsafe pointer comparisons. It also removes some code.
• Only test pointer equality, thus rejecting warping an image into itself but not rejecting warping an image into a view of a subregion of itself (which will also produce garbage). It appears even doing that violates C++ standards, but it's hard to imagine that failing for any current compiler. Nonetheless, I feel it offers very little increased safety for the user of warpImage so I'm not keen on it. It seems a lot of fuss for little gain and I worry it may give a false sense of security.

I would be quite content with just documenting and not testing this case.  I don't think it's likely to actually come up, and I think it's fairly standard practice for C++ array libraries to leave it to the user to avoid aliasing in all but the most trivial scenarios.  And as Krzysztof Findeisen has pointed out, it really is quite hard to check this rigorously in a standards-compliant way.

Jim Bosch I only just now saw your note, after merging the ticket. You originally said to leave it to Krzysztof Findeisen and he felt it was OK to risk the test so I have gone ahead with it, but added a warning that it could conceivably give false negatives.

I agree that the risk of trying to warp into a subregion of the input image is fairly low, but given the ease of making live views of subregions of images it is not completely negligible.

I can remove the test if you prefer, but I'll ask you to file a ticket for that and assign it to me.