Bug in ValueIteratorBase::operator-

[kevingrant]

ValueIteratorBase::operator- implementation appears to be backwards. It calls this->computeDistance(other) and computeDistance expects that other is reachable from this (i.e. other >= this). But with iterators it1 - it2 generally implies that it1 is reachable from it2. Hence the following usage does not work as expected.

Repro

#include "json/json.h"
#include "stdio.h"

int main(int argc, char *argv[])
{
  Json::Value json;
  json["k1"] = "a";
  json["k2"] = "b";
  for (auto it = json.begin(); it != json.end(); ++it) {
    printf("Value #%d is %s\n", it - json.begin(), it->asString().c_str());
  }
  return 0;
}

Expected result

Value #0 is a
Value #1 is b

Actual result

Value #0 is a
<hang>

[cdunn2001]

Ugh! You're right, but look at this:

 76 ValueIteratorBase::difference_type
 77 ValueIteratorBase::computeDistance(const SelfType& other) const {
 78 #ifndef JSON_VALUE_USE_INTERNAL_MAP
 79 #ifdef JSON_USE_CPPTL_SMALLMAP
 80   return current_ - other.current_;
 81 #else

I think Baptiste only tested his iterators with CPPTL. Now we need to reverse line 80 also. Fortunately, it is called only via the code which you have fixed, so we can safely reverse line 80 too. Unfortunately, that is inline code. Fortunately, so is the line which you fixed! Unfortunately, computeDistance() is protected, not private, but I'll be that nobody has extended that Iterator. So all will be well.

I'll change line 80, add a test, and keep your commit...

[ghost]

Why the distance can't be negative?