[PATCH] Initial commit for the llvm-dsymutil tool.

Frederic Riss friss at apple.com
Thu Dec 11 15:45:15 PST 2014


Thanks! I'll address your last comments before commit.

Couple of answers below:


================
Comment at: tools/dsymutil/DebugMap.cpp:42
@@ +41,3 @@
+      Entries.begin(), Entries.end(),
+      [](const Entry &LHS, const Entry &RHS) { return LHS.first < RHS.first; });
+  for (const auto &Sym : Entries) {
----------------
samsonov wrote:
> You don't need a comparator here - all keys in StringMap are different, and std::pair<> objects are compared lexicographically by default, which is what you need.
I didn't know pairs provided lexicographical relational operators, thanks for the tip!

================
Comment at: tools/dsymutil/MachODebugMapParser.cpp:216
@@ +215,3 @@
+  const MachOObjectFile &Binary = *MainOwningBinary.getBinary();
+  section_iterator Section = Binary.section_end();
+  for (const auto &Sym : Binary.symbols()) {
----------------
samsonov wrote:
> Shouldn't this be a local variable defined inside for-loop with no default value? (as it will be initialized by Sym.getSection()) ?
section_iterator doesn't provide a default constructor, and I didn't want to all Binary.section_end() for every iteration. The default value is there only to be able to construct the variable, its value will be overwritten every time it will actually be used.

================
Comment at: tools/dsymutil/dsymutil.cpp:34
@@ +33,3 @@
+                                       desc("Specify a path to prepend to the "
+                                            "object file's filenames."),
+                                       value_desc("path"));
----------------
samsonov wrote:
> "names of object files"?
What about "Specify a directory to prepend to the paths of object files.". I find 'name' a bit too vague (is it only the basename?).

================
Comment at: tools/dsymutil/dsymutil.h:25
@@ +24,3 @@
+
+class DebugMap;
+
----------------
samsonov wrote:
> Shouldn't you #include DebugMap.h here to use DebugMap in unique_ptr?
Good point, I suppose that this didn't show up because every user of dsymutil.h is also a DebugMap.h user, and 'DebugMap' comes before 'dsymutil' in a sorted include list.

http://reviews.llvm.org/D6242

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list