[PATCH] Complete Rewrite of CGRecordLayoutBuilder

[Reid Kleckner]

  Works for me, but we should check with John.


================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:34
@@ -32,6 +33,3 @@
 namespace {
-
-class CGRecordLayoutBuilder {
-public:
-  /// FieldTypes - Holds the LLVM types that the struct is created from.
-  /// 
+/// The CGRecordLowering is responsible for lowering an ASTRecordLayout to an
+/// llvm::Type.  Some of the lowering is straightforward, some is not.  Here we
----------------
I'd probably drop "The" at the start of the sentence here.

================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:36
@@ +35,3 @@
+/// llvm::Type.  Some of the lowering is straightforward, some is not.  Here we
+/// detail some of the complexities and weirdnesses here.
+/// * LLVM does not have unions - Unions can, in theory be represented by any
----------------
I'd drop the trailing "here"

================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:62
@@ +61,3 @@
+///   The location of the clip is stored internally as a sentinal of type
+///   SCISSOR.  If LLVM were updated to read base types (which it probably
+///   should because locations of things such as VBases are bogus in the llvm
----------------
Should SCISSOR be all caps?  The enum is StudlyCaps now.

================
Comment at: lib/CodeGen/CGRecordLayoutBuilder.cpp:200
@@ -204,55 +199,3 @@
 };
-
-}
-
-void CGRecordLayoutBuilder::Layout(const RecordDecl *D) {
-  const ASTRecordLayout &Layout = Types.getContext().getASTRecordLayout(D);
-  Alignment = Layout.getAlignment();
-  Packed = D->hasAttr<PackedAttr>() || Layout.getSize() % Alignment != 0;
-
-  if (D->isUnion()) {
-    LayoutUnion(D);
-    return;
-  }
-
-  if (LayoutFields(D))
-    return;
-
-  // We weren't able to layout the struct. Try again with a packed struct
-  Packed = true;
-  LastLaidOutBase.invalidate();
-  NextFieldOffset = CharUnits::Zero();
-  FieldTypes.clear();
-  Fields.clear();
-  BitFields.clear();
-  NonVirtualBases.clear();
-  VirtualBases.clear();
-
-  LayoutFields(D);
-}
-
-CGBitFieldInfo CGBitFieldInfo::MakeInfo(CodeGenTypes &Types,
-                                        const FieldDecl *FD,
-                                        uint64_t Offset, uint64_t Size,
-                                        uint64_t StorageSize,
-                                        uint64_t StorageAlignment) {
-  llvm::Type *Ty = Types.ConvertTypeForMem(FD->getType());
-  CharUnits TypeSizeInBytes =
-    CharUnits::fromQuantity(Types.getDataLayout().getTypeAllocSize(Ty));
-  uint64_t TypeSizeInBits = Types.getContext().toBits(TypeSizeInBytes);
-
-  bool IsSigned = FD->getType()->isSignedIntegerOrEnumerationType();
-
-  if (Size > TypeSizeInBits) {
-    // We have a wide bit-field. The extra bits are only used for padding, so
-    // if we have a bitfield of type T, with size N:
-    //
-    // T t : N;
-    //
-    // We can just assume that it's:
-    //
-    // T t : sizeof(T);
-    //
-    Size = TypeSizeInBits;
-  }
-
+} // namespace {
+
----------------
"end anonymous namespace" is probably more prevalent in Clang


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