<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">I agree with John. If we're rolling out a new API why not introduce now a new type that adds the "dimensionality" of the value? (bytes versus bits)<div><br></div><div>John: Is that what you meant?<br><div><br><div><div>On Nov 24, 2009, at 1:25 PM, John McCall wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">
<div bgcolor="#ffffff" text="#000000">
Ken Dyck wrote:
<blockquote cite="mid:8F2E4A8BCDA0B84DA6C9088EB5B27747B214C5@NAMAIL.ad.onsemi.com" type="cite">
<pre wrap="">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?
</pre>
</blockquote>
<br>
I'd feel a lot more confident in our correctness if we used an opaque
type for all this. :)<br>
<br>
<br>
--- lib/AST/ExprConstant.cpp (revision 89760)<br>
+++ lib/AST/ExprConstant.cpp (working copy)<br>
@@ -424,7 +424,7 @@<br>
if (PointeeType->isVoidType() || PointeeType->isFunctionType())<br>
SizeOfPointee = 1;<br>
else<br>
- SizeOfPointee = Info.Ctx.getTypeSize(PointeeType) / 8;<br>
+ SizeOfPointee = Info.Ctx.getTypeSizeInBytes(PointeeType);<br>
<br>
The void/function pointer case here also needs to be updated, yes?
Possibly to Info.Ctx.getCharWidth()?<br>
<br>
Similarly here:<br>
<br>
@@ -1156,7 +1156,7 @@<br>
<br>
uint64_t D = LHSValue.getLValueOffset() -
RHSValue.getLValueOffset();<br>
if (!ElementType->isVoidType() &&
!ElementType->isFunctionType())<br>
- D /= Info.Ctx.getTypeSize(ElementType) / 8;<br>
+ D /= Info.Ctx.getTypeSizeInBytes(ElementType);<br>
<br>
return Success(D, E);<br>
}<br>
<br>
This implicitly assumes that arithmetic on void* is in 1-byte chunks.<br>
<br>
--- lib/AST/ASTContext.cpp (revision 89760)<br>
+++ lib/AST/ASTContext.cpp (working copy)<br>
@@ -3085,15 +3085,15 @@<br>
/// getObjCEncodingTypeSize returns size of type for objective-c
encoding<br>
/// purpose.<br>
int ASTContext::getObjCEncodingTypeSize(QualType type) {<br>
- uint64_t sz = getTypeSize(type);<br>
+ uint64_t sz = getTypeSizeInBytes(type);<br>
<br>
// Make all integer and enum types at least as large as an int<br>
if (sz > 0 && type->isIntegralType())<br>
- sz = std::max(sz, getTypeSize(IntTy));<br>
+ sz = std::max(sz, getTypeSizeInBytes(IntTy));<br>
// Treat arrays as pointers, since that's how they're passed in.<br>
else if (type->isArrayType())<br>
- sz = getTypeSize(VoidPtrTy);<br>
- return sz / getTypeSize(CharTy);<br>
+ sz = getTypeSizeInBytes(VoidPtrTy);<br>
+ return sz;<br>
}<br>
<br>
<br>
You've changed this function from returning the size in chars to the
size in bytes. Intentional? If so, please document. <br>
<br>
Everything else looks fine.<br>
<br>
John.<br>
<br>
<br>
<blockquote cite="mid:8F2E4A8BCDA0B84DA6C9088EB5B27747B214C5@NAMAIL.ad.onsemi.com" type="cite">
<pre wrap="">Cheers,
-Ken
</pre>
<pre wrap=""><hr size="4" width="90%">
_______________________________________________
cfe-commits mailing list
<a class="moz-txt-link-abbreviated" href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a>
</pre>
</blockquote>
<br>
</div>
_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits<br></blockquote></div><br></div></div></body></html>