[PATCH] D70557: [lld][COFF] Add support for /map

[Adrian McCarthy via Phabricator via llvm-commits]

amccarth accepted this revision.
amccarth added a comment.

Sorry, somehow I'd missed that I'd been added as a reviewer.  This looks good to me.  (Though I do have a couple questions in the inline comments.)



================
Comment at: lld/COFF/LLDMapFile.cpp:69
+      return a->getRVA() < b->getRVA();
+    });
+  }
----------------
Stupid question:  Is it possible for two symbols to share an address?  If so, should this be a `std::stable_sort` for deterministic build outputs?


================
Comment at: lld/COFF/MapFile.cpp:81
+    return a.first < b.first || (a.first == b.first && a.second < b.second);
+  });
+  auto end = std::unique(v.begin(), v.end(), [](const auto &a, const auto &b) {
----------------
Is the lambda necessary in this sort?  Don't `std::pair`s naturally use lexicographic ordering?

Do all the `Defined *`s live inside a single object l(ike an array)?  If not, then comparing pointers to individual ones is technically undefined behavior though it's very likely to work as expected.


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

https://reviews.llvm.org/D70557