================
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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to