On Dec 18, 2009, at 11:31 AM, Ken Dyck wrote: >>> +++ 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.
A forward definition is enough, a caller just needs to include the header if it uses the method, but files that don't use the method don't need the definition of the class. >> 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. Sounds great, thanks for working on this! -Chris _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
