rnk added inline comments.

> CGDebugInfo.cpp:47
>  
> +namespace {
> +template <typename Type>

LLVM prefers `static` to anonymous namespaces 
http://llvm.org/docs/CodingStandards.html#anonymous-namespaces

> CGDebugInfo.cpp:48
> +namespace {
> +template <typename Type>
> +uint64_t GetTypeAlignIfRequired(Type Ty, const ASTContext &Ctx) {

Instead of templating, have a `const Type*` overload and a QualType overload 
like getTypeInfo does:

  /// \brief Get the size and alignment of the specified complete type in bits.
  TypeInfo getTypeInfo(const Type *T) const;
  TypeInfo getTypeInfo(QualType T) const { return getTypeInfo(T.getTypePtr()); }

> CGDebugInfo.cpp:49
> +template <typename Type>
> +uint64_t GetTypeAlignIfRequired(Type Ty, const ASTContext &Ctx) {
> +  auto TI = Ctx.getTypeInfo(Ty);

LLVM style uses a leading lower case for method names: 
http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

> CGDebugInfo.cpp:53
> +                            : 0;
> +}
> +uint64_t GetDeclAlignIfRequired(const Decl *D, const ASTContext &Ctx) {

Please add a blank line between top level decls

https://reviews.llvm.org/D24426



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to