[PATCH] Microsoft C Record Layout

[Chandler Carruth]

  Just some peanut gallery comments.


================
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(
----------------
FYI, in new code please follow the doxygen guidelines in the coding standards:

http://llvm.org/docs/CodingStandards.html#doxygen-use-in-documentation-comments

================
Comment at: lib/AST/MicrosoftRecordLayoutBuilder.cpp:58
@@ +57,3 @@
+  /// account when returning alignment.
+  inline std::pair<CharUnits, CharUnits> getAdjustedFieldInfo(
+      const FieldDecl *FD) const;
----------------
Why inline?

================
Comment at: test/Sema/ms_bitfield_layout.c:158
@@ +157,3 @@
+
+int main() {
+	A a;
----------------
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)

================
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]>
+
----------------
It seems nicer for maintenance to put the CHECKs with the types they are making assertions about.


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