On Friday, December 18, 2009 12:59 PM, Chris Lattner wrote: > > On Dec 18, 2009, at 7:55 AM, Ken Dyck wrote: > > > Author: kjdyck > > Date: Fri Dec 18 09:55:54 2009 > > New Revision: 91689 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=91689&view=rev > > Log: > > Change the return type of ASTContext::getTypeSizeInChars() from > > uint64_t to the new opaque value type, CharUnits. This will help us > > avoid accidentally mixing quantities that are in bit and > character units. > > Nice. I assume that this will start getting more pervasive > throughout the codebase?
That's the plan. Mainly in CodeGen code, but also touching Analysis,AST, and a tiny bit of Sema. A similar getTypeAlignInChars() is also in the plans. > > +++ cfe/trunk/include/clang/AST/ASTContext.h Fri Dec 18 > 09:55:54 2009 > > @@ -18,6 +18,7 @@ > > #include "clang/Basic/LangOptions.h" > > #include "clang/Basic/OperatorKinds.h" > > #include "clang/AST/Attr.h" > > +#include "clang/AST/CharUnits.h" > > Please move the bodies of getTypeSizeInChars out of line to > avoid this #include. I don't think a forward declaration of the class is going to work here. The getTypeSizeInChars() method returns a CharUnits, not a reference or a pointer, so the full class definition is required. > > +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Fri Dec 18 09:55:54 2009 > > @@ -471,7 +471,8 @@ > > const llvm::Type *IntPtr = > > llvm::IntegerType::get(VMContext, LLVMPointerWidth); > > llvm::Value *SizeVal = > > - llvm::ConstantInt::get(IntPtr, > getContext().getTypeSizeInChars(Ty)); > > + llvm::ConstantInt::get(IntPtr, > > + > > + getContext().getTypeSizeInChars(Ty).getRaw()); > > Is this going to be a common operation? If so, it might make > sense to add a "getLLVMConstantInt(const llvm::Type*)" to > CharUnit. Actually, on second thought, that would be a > layering violation because AST shouldn't depend on LLVM IR. > Maybe this should be a method on CGM? Something like > CGM.getSizeConstant(getContext().getTypeSizeInChars(Ty)) ? > > I don't know if this will happen enough to make it worthwhile though. It has shown up fairly frequently (5-15 times, roughly) in the prototyping that I've been doing on a private branch so far, so I'll look into adding something to CGM before I roll out any more of these. -Ken _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
