[PATCH] D44894: [ELF] - Reveal more information in -Map file about assignments.

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Mar 27 14:41:56 PDT 2018


ruiu added inline comments.


================
Comment at: ELF/LinkerScript.h:113-114
 
-  // This is just an offset of this assignment command in the output section.
-  unsigned Offset;
+  // Address of this assignment command. This corresponds to the value
+  // of Dot for the moment before doing the assignment.
+  unsigned Addr;
----------------
I think the original comment makes sense. I can understand what "before doing the assignment" means, but it feels easier to understand if you just say it's an address of an assignment.


================
Comment at: ELF/LinkerScript.h:118
+  // Size of this assignment command. This is usually 0, but if
+  // you move '.' this may be greater than 0."
   unsigned Size;
----------------
Remove "


================
Comment at: ELF/LinkerScript.h:202-207
+
+  // This is just an offset of this assignment command in the output section.
   unsigned Offset;
+
+  // Size of this data command.
   unsigned Size;
----------------
Why do you need to add these field to two different places? We generally should avoid doing this.


================
Comment at: ELF/MapFile.cpp:153
+static void writeAssignment(raw_ostream &OS, SymbolAssignment *Cmd,
+                            const Twine &Padding = "") {
+  // Do not print symbols that were not provided.
----------------
Let's not generalize. Just pass a boolean value instead of a string.

OK, so you are using this function only twice. I'd inline this function.


https://reviews.llvm.org/D44894





More information about the llvm-commits mailing list