[PATCH] D36201: Merge manifest namespaces.

[Zachary Turner via Phabricator via llvm-commits]

zturner added inline comments.


================
Comment at: llvm/lib/WindowsManifest/WindowsManifestMerger.cpp:108-110
+  int HRef1Priority = std::distance(HRef1Position, std::end(MtNsHrefsPrefixes));
+  int HRef2Priority = std::distance(HRef2Position, std::end(MtNsHrefsPrefixes));
+  return HRef1Priority > HRef2Priority;
----------------
ecbeckmann wrote:
> zturner wrote:
> > ecbeckmann wrote:
> > > ruiu wrote:
> > > > Since HRef1Position and HRef2Position are iterators for the same map, you can compare them directly, can't you?
> > > the only way to directly compare iterators is using std::distance.  However, behavior is undefined if the 2nd iterator is not reachable by incrementing the first iterator, which will often be the case for our usage.  Therefore, the only way to correctly compare is to first find each one's absolute position in the map, which I do here.
> > Maps are ordered, so why not just use the ordering function directly?
> > 
> > ```
> > return FROM_XML_CHAR(HRef1) > FROM_XML_CHAR(HRef2)?
> > ```
> Unfortunately HRef1 and HRef2 do not point within the table, but rather xmlChar arrays allocated by libxml2's parser, so we can't compare those.
I think I'm still mis-understanding something.  

The code here is looking both entries up by key, then after determining that both keys are present, it then computes each iterator's distance from the end.  And if the first one is farther from the end (e.g. closer to the beginning) than the second one, it returns true, otherwise it returns false.

But when you iterate a map, it iterates in key order.  So saying "the first one is closer to the beginning" is identical to saying "the first one is less than the second one".  So as long as you use the same comparison function that the map uses internally to order keys, then it should be the same.  And in this case, the map seems to be using a string comparison.

So it seems like all you have to do is string compare the two arguments and return true if the first one is less than the second one?


https://reviews.llvm.org/D36201