[PATCH] D17321: DIEData, DIEWriter: introduce and begin migration.

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: include/llvm/CodeGen/DIEData.h:152
@@ +151,3 @@
+      return attr1(Attr, dwarf::DW_FORM_data1, Data);
+    else if (Data < (1ull << 16))
+      return attr2(Attr, dwarf::DW_FORM_data2, Data, BE);
----------------
dblaikie wrote:
> else-after-return
Fixed by removing the returns. I wanted to eventually make some of these functions return a reference to the attribute that could be filled in later, but that probably isn't appropriate for attributes like this one which need to know the value up front.

================
Comment at: lib/CodeGen/AsmPrinter/DwarfUnit.cpp:685
@@ -684,1 +684,3 @@
 
+void DwarfUnit::string(DIEWriter &W, dwarf::Attribute Attr, StringRef Str) {
+  DwarfStringPool::EntryRef Entry = DU->getStringPool().getEntry(*Asm, Str);
----------------
dblaikie wrote:
> Probably best not to name a function the same as a common library type. I think functions that use string as a verb usually end up named "stringify" or the like.. but that might not be what's being described here, not sure yet.
Hm. I wanted to be concise with names in the new code, in order to distinguish visually from the old code. I think this naming might be okay if there are only a few weird cases like `string`.


http://reviews.llvm.org/D17321