[PATCH] Microsoft C Record Layout

[Warren Hunt]

================
Comment at: lib/AST/MicrosoftRecordLayoutBuilder.cpp:58
@@ +57,3 @@
+  /// account when returning alignment.
+  inline std::pair<CharUnits, CharUnits> getAdjustedFieldInfo(
+      const FieldDecl *FD) const;
----------------
Chandler Carruth wrote:
> Why inline?
Originally this function was just a wrapper.  It's grown and the inline is no longer necessary.

================
Comment at: lib/AST/MicrosoftRecordLayoutBuilder.cpp:55-57
@@ +54,5 @@
+
+  /// getAdjustedFieldInfo - Gets the size and alignment.  This function
+  /// takes the field packed attribute and MaxFieldAlignment into
+  /// account when returning alignment.
+  inline std::pair<CharUnits, CharUnits> getAdjustedFieldInfo(
----------------
Chandler Carruth wrote:
> FYI, in new code please follow the doxygen guidelines in the coding standards:
> 
> http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments
Okay cool, I'll change it, I had just been copying the style that was there.

================
Comment at: test/Sema/ms_bitfield_layout.c:158
@@ +157,3 @@
+
+int main() {
+	A a;
----------------
Chandler Carruth wrote:
> This test isn't actually run in the regression test suite. I would remove all of the executable parts of your test from what you commit (although clearly it may be useful to keep them around for your testing and understanding)
I agree it's not useful for lit.  I was concerned that without reference to the structs, they might get eliminated by the compiler.  I'm assuming now that this isn't the case?

================
Comment at: test/Sema/ms_bitfield_layout.c:310-314
@@ +309,7 @@
+
+// CHECK: Type: struct A
+// CHECK:   Size:128
+// CHECK:   DataSize:128
+// CHECK:   Alignment:32
+// CHECK:   FieldOffsets: [0, 32, 64, 64, 96, 99, 112]>
+
----------------
Chandler Carruth wrote:
> It seems nicer for maintenance to put the CHECKs with the types they are making assertions about.
I agree, I'll move them.


http://llvm-reviews.chandlerc.com/D1026