[cfe-commits] [Review] rolling out ASTContext::getTypeSizeInBytes()

[John McCall]

Ken Dyck wrote:
> The attached patch rolls out the recently-added
> ASTContext::getTypeSizeInBytes() method, replacing many (hopefully all)
> of the expressions that calculate the byte size of a type by dividing
> its bit size by a literal 8 or the bit size of the char type.
>
> The changes are numerous but straighforward, touching portions of
> CodeGen, Analysis, and AST. Could somebody review the patch before I
> commit?
>   

I'd feel a lot more confident in our correctness if we used an opaque 
type for all this. :)


--- lib/AST/ExprConstant.cpp    (revision 89760)
+++ lib/AST/ExprConstant.cpp    (working copy)
@@ -424,7 +424,7 @@
   if (PointeeType->isVoidType() || PointeeType->isFunctionType())
     SizeOfPointee = 1;
   else
-    SizeOfPointee = Info.Ctx.getTypeSize(PointeeType) / 8;
+    SizeOfPointee = Info.Ctx.getTypeSizeInBytes(PointeeType);

The void/function pointer case here also needs to be updated, yes?  
Possibly to Info.Ctx.getCharWidth()?

Similarly here:

@@ -1156,7 +1156,7 @@
 
         uint64_t D = LHSValue.getLValueOffset() - 
RHSValue.getLValueOffset();
         if (!ElementType->isVoidType() && !ElementType->isFunctionType())
-          D /= Info.Ctx.getTypeSize(ElementType) / 8;
+          D /= Info.Ctx.getTypeSizeInBytes(ElementType);
 
         return Success(D, E);
       }

This implicitly assumes that arithmetic on void* is in 1-byte chunks.

--- lib/AST/ASTContext.cpp    (revision 89760)
+++ lib/AST/ASTContext.cpp    (working copy)
@@ -3085,15 +3085,15 @@
 /// getObjCEncodingTypeSize returns size of type for objective-c encoding
 /// purpose.
 int ASTContext::getObjCEncodingTypeSize(QualType type) {
-  uint64_t sz = getTypeSize(type);
+  uint64_t sz = getTypeSizeInBytes(type);
 
   // Make all integer and enum types at least as large as an int
   if (sz > 0 && type->isIntegralType())
-    sz = std::max(sz, getTypeSize(IntTy));
+    sz = std::max(sz, getTypeSizeInBytes(IntTy));
   // Treat arrays as pointers, since that's how they're passed in.
   else if (type->isArrayType())
-    sz = getTypeSize(VoidPtrTy);
-  return sz / getTypeSize(CharTy);
+    sz = getTypeSizeInBytes(VoidPtrTy);
+  return sz;
 }


You've changed this function from returning the size in chars to the 
size in bytes.  Intentional?  If so, please document.

Everything else looks fine.

John.


> Cheers,
> -Ken
>   
> ------------------------------------------------------------------------
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>   

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20091124/15bfbf75/attachment.html>