[PATCH] D36201: Merge manifest namespaces.

Eric Beckmann via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 15 15:26:19 PDT 2017


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


https://reviews.llvm.org/D36201





More information about the llvm-commits mailing list