[PATCH] D137164: [LLParser] Handle mixed blockaddress forward references with names and IDs

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

dexonsmith accepted this revision.
dexonsmith added a subscriber: dblaikie.
dexonsmith added a comment.
This revision is now accepted and ready to land.

In D137164#3917926 <https://reviews.llvm.org/D137164#3917926>, @MichielDerhaeg wrote:

> In D137164#3915823 <https://reviews.llvm.org/D137164#3915823>, @dexonsmith wrote:
>
>> Would it be simpler to define `<` for that case? Or, we could define a custom one to send to the `std::map`:
>>
>>   struct CompareValID {
>>     static bool operator<(const ValID &LHS, const ValID &RHS) {
>>       if (LHS.Kind != RHS.Kind)
>>         return LHS.Kind < RHS.Kind;
>>       return LHS < RHS;
>>     }
>>   };
>
> That was my initial idea, but if you search for other ForwardRef-related datastructures in LLParser.cpp, you'll see that they solved it this way for all other cases as well.
> I did it like this to remain consistent.

Hmm. Okay, the pattern followed elsewhere is reasonable. LGTM then.

That said, the pattern of splitting the map seems to add boilerplate and I'm not convinced it's less error-prone than a shared map. @dblaikie, any thoughts? (I.e., would it be better to tidy it up, or is there some value in using the split maps?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D137164/new/

https://reviews.llvm.org/D137164