[PATCH] D20840: [codeview] Translate basic DITypes to CV type records

[Reid Kleckner via llvm-commits]

rnk added inline comments.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:780
@@ +779,3 @@
+    break;
+  case dwarf::DW_ATE_decimal_float:
+  case dwarf::DW_ATE_float:
----------------
majnemer wrote:
> I think this is for stuff like `_Decimal64`, we shouldn't map it to floats...
> 
> The closest thing I can think of is the currency type.
Eh, let's remove it. I'll give you a dollar if a real user complains in the next year.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:811-812
@@ +810,4 @@
+  case dwarf::DW_ATE_unsigned_char:
+    if (ByteSize == 1)
+      STK = SimpleTypeKind::UnsignedCharacter;
+    break;
----------------
majnemer wrote:
> zturner wrote:
> > ```
> > else if (ByteSize == 2)
> >   STK = SimpleTypeKind::Character16;
> > else if (ByteSize == 4)
> >   STK = SimpleTypeKind::Character32;
> > ```
> > 
> > Is there any way to distinguish `wchar_t` from `DW_ATE_unsigned_char` with `ByteSize == 2`?
> The BasicType's name maybe?
Ooh, all the character types are all messed up. That needs real work... Added some stuff.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:857-859
@@ +856,5 @@
+  }
+  // FIXME: MSVC folds qualifiers into PointerOptions in the context of a method
+  // 'this' pointer, but not normal contexts. Figure out what we're supposed to
+  // do.
+  PointerOptions PO = PointerOptions::None;
----------------
zturner wrote:
> Is this true?  Surely there has to be a `PointerOptions` on a non member pointer.
It's related to method 'this' pointers, not member pointers. This is the test case I can use to get them to fill in PointerOptions:
  struct A {
    const int *a;
    void f() const;
  };
  void A::f() const {}
  A a;
It's used for the const qualification on the 'this' parameter of A::f, but not the const qualifier on the `const int *a;` field. In that case, they use an LF_MODIFIER record.

================
Comment at: lib/CodeGen/AsmPrinter/CodeViewDebug.cpp:895-897
@@ +894,5 @@
+      break;
+    default:
+      IsModifier = false;
+      break;
+    }
----------------
majnemer wrote:
> Todo for unaligned?
> Also, should we set flat?
Yeah, there's unaligned and also a similar issue with __restrict. I can't convince MSVC to set IsFlat, so I left it off.


http://reviews.llvm.org/D20840