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

Duncan P. N. Exon Smith via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 15 18:41:14 PDT 2016


>>> 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.

The registration approach actually works really well (way better than
what I had).  I have the ValueMap implicitly register a default context
at ID==0, and the schedule*() functions default to ID==0.  I also hid
the MappingContext class as an implementation detail.  Thanks for the
suggestion.

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

A couple of preps in r266490 and r266493; final commit was r266503.



More information about the llvm-commits mailing list