On 5/17/2021 11:55 PM, Elijah Newren wrote: >> /* >> * cached_irrelevant: Caching of rename_sources that aren't relevant. >> * >> * cached_pairs records both renames and deletes. Sometimes we >> * do not know if a path is a rename or a delete because we pass >> * RELEVANT_LOCATION to diffcore_rename_extended() which might >> * describe a path as "irrelevant" instead of as a "rename" or "delete". >> * We need to cache such paths too, but separately from cached_pairs. >> */ >> >> Does this make sense? diffcore_rename_extended() might need an update >> to match this extra, explicit state. > Hmm, let's flesh out the description a bit and try to be more > explicit. How about: > > /* > * cached_irrelevant: Caching of rename_sources that aren't relevant. > * > * If we try to detect a rename for a source path and succeed, it's > * part of a rename. If we try to detect a rename for a source path > * and fail, then it's a delete. If we do not try to detect a rename > * for a path, then we don't know if it's a rename or a delete. If > * merge-ort doesn't think the path is relevant, then we just won't > * cache anything for that path. But there's a slight problem in > * that merge-ort can think a path is RELEVANT_LOCATION, but due to > * commit 9bd342137e ("diffcore-rename: determine which > * relevant_sources are no longer relevant", 2021-03-13), > * diffcore-rename can downgrade the path to RELEVANT_NO_MORE. To > * avoid excessive calls to diffcore_rename_extended() we still need > * to cache such paths, though we cannot record them as either > * renames or deletes. So we cache them here as a "turned out to be > * irrelevant *for this commit*" as they are often also irrelevant > * for subsequent commits, though we will have to do some extra > * checking to see whether such paths become relevant for rename > * detection when cherry-picking/rebasing subsequent commits. > */ This is more informative, thanks. -Stolee