vleschuk marked an inline comment as done. vleschuk added inline comments.
================ Comment at: include/clang/AST/ASTContext.h:83 uint64_t Width; - unsigned Align; + llvm::DIAlignment Align; bool AlignIsRequired : 1; ---------------- vleschuk wrote: > aprantl wrote: > > I'm not sure we want to use a debug info type inside the AST. I think we > > only want to use them in CGDebugInfo.cpp. > We use TypeInfo and related functions heavily in CGDebugInfo.cpp, leaving > this field as "unsigned" will make us to perform conversions from "unsigned" > to llvm::DIAlignment, I think having this changed will result in simpler code. @dblaikie wrote: > It'd still be a layering violation to have AST depend on debug info > types/concepts. If it makes sense to change it to uint32_t in AST, that'd > probably be reasonable. We are discussing the possibility of changing this to "unsigned" in https://reviews.llvm.org/D25620, after we come to consensus I'll update both patches. Thanks for the advice, I agree that using DebugInfo types in AST was a bad idea. https://reviews.llvm.org/D25621 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits