[PATCH] D36201: Merge manifest namespaces.

[Eric Beckmann via Phabricator via llvm-commits]

ecbeckmann added a comment.
ecbeckmann added inline comments.


================
Comment at: llvm/lib/WindowsManifest/WindowsManifestMerger.cpp:221
+        continue;
+      } else if (namespaceOverrides(OriginalAttribute->ns->href,
+                                    Attribute->ns->href)) {
----------------
rnk wrote:
> ruiu wrote:
> > ecbeckmann wrote:
> > > ruiu wrote:
> > > > Well, you want to remove `continue`, as I described in the previous comment. (Please fix not only this place but other places as well.)
> > > wait I'm confused, I thought we want to add the early continues?
> > Ah sorry, I meant not `continue` but `else`.
> Most of these if / else branches are about equivalent in terms of complexity. I'd actually suggest removing all the continues that got added (except the `!Attribute->ns` check). Most of the continues in the current code are redundant, because they'd hit the end of the loop naturally anyway.
I think there's a small benefit in terms of readability


https://reviews.llvm.org/D36201