i don't think m_ffStates[i] can be null and compare.m_ffStates[i] can ever be NOT null.
it should be an assert of UTIL_THROW_IF(...), rather than an if statement On 25/06/2015 12:09, Jeroen Vermeulen wrote: > Looking at replacing WordsBitmap's implementation with std::vector<bool> > (less code, less memory) I came across this function: > > « > /** check, if two hypothesis can be recombined. > this is actually a sorting function that allows us to > keep an ordered list of hypotheses. This makes recombination > much quicker. > */ > int > Hypothesis:: > RecombineCompare(const Hypothesis &compare) const > { > // -1 = this < compare > // +1 = this > compare > // 0 = this ==compare > int comp = m_sourceCompleted.Compare(compare.m_sourceCompleted); > if (comp != 0) > return comp; > > for (unsigned i = 0; i < m_ffStates.size(); ++i) { > if (m_ffStates[i] == NULL || compare.m_ffStates[i] == NULL) { > comp = m_ffStates[i] - compare.m_ffStates[i]; > } else { > comp = m_ffStates[i]->Compare(*compare.m_ffStates[i]); > } > if (comp != 0) return comp; > } > > return 0; > } > » > > > My problem is with this conditional subtraction: > > « > if (m_ffStates[i] == NULL || compare.m_ffStates[i] == NULL) { > comp = m_ffStates[i] - compare.m_ffStates[i]; > » > > The result of that subtraction looks technically undefined to me, in > which case _theoretically_ I could replace it with anything I liked > including code to recite Homer in Morse code on the hard-disk light. > But what is it meant to do in practice? > > The assignment to comp casts the value from std::ptrdiff_t to int. On a > two's-complement system with 64-bit pointers, 32-bit ints, and a zero > null pointer, the could would boil down to: take either m_ffStates[i] or > ~m_ff_states[i] + 1 depending on which one is non-null, divide it by the > size of FFState, drop the most-significant 32 bits, and compare the > least-signficant 32 bits to zero. Occasionally you may get a completely > unexpected zero even though one of the pointers is non-null, but usually > you'll get an arbitrary positive or negative result. On the other hand > it wouldn't surprise me if the optimizer were allowed to make convenient > assumptions about the truncation, and just re-use the sign of the > original ptrdiff_t. > > Am I right in thinking this comparison is more or less arbitrary, as > long as the result is consistent and only zero if the two pointers are > both null? If so, would anyone mind if I made it compare just the > nullness of the two pointers? > > The actual code may not end up looking like this, but I'm thinking along > the lines of: > > « > comp = ( > int(m_ffStates[i] == NULL) - int(compare.m_ffStates[i] == NULL) > ); > » > > This way, a null pointer would be deterministically "less than" a > non-null pointer. > > > Jeroen > _______________________________________________ > Moses-support mailing list > Moses-support@mit.edu > http://mailman.mit.edu/mailman/listinfo/moses-support -- Hieu Hoang Researcher New York University, Abu Dhabi http://www.hoang.co.uk/hieu _______________________________________________ Moses-support mailing list Moses-support@mit.edu http://mailman.mit.edu/mailman/listinfo/moses-support