[PATCH] ValueMapper: Eliminate cross-file co-recursion, NFC

[Duncan P. N. Exon Smith via llvm-commits]

>> Sure, PR is fine. Even longer term I want to completely remove the
>> appending linkage, but that will take some time.

Yeah, I think we should kill appending linkage too.  I'll file the PR
before I commit though.

>> The "FIXME: Why is this upgrade done during linking?" is duplicated btw.

Intentional (they cross-reference each other), but not helpful I guess
:).  I'll de-dup.

>> you can probably use std::find for getMappingContextID.

Good point.

>> I wonder if it
>> wouldn't be cleaner to have the caller do something like
>> 
>> unsigned NormalID = Mapper.registerContext(...);
>> unsigned AliasID = Mapped.registerContext(...);
>> 
>> and pass the context to scheduleMap*.
> 
> Or just pass an ArrayRef of MappingContext to the constructor?

Both good ideas.  What I like about the current approach is that it's
hard to misuse the API (as awkward as it is); I'm tempted to leave it
as is unless we find a concrete reason to change it.  But I'll play
around a bit.

>> Cool. BTW, do you intend to also port CloneModule and CloneFunction to
>> use the ValueMapper class? With that you should be able to remove the
>> top level function. This can be another patch.
>> 
>> In any case, LGTM with nits and a few suggestions.

Thanks for the review!  I'll try to get this in today.