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