aaron.ballman added a comment.

In D150528#4368023 <https://reviews.llvm.org/D150528#4368023>, @yronglin wrote:

> In D150528#4365688 <https://reviews.llvm.org/D150528#4365688>, @aaron.ballman 
> wrote:
>
>> In general, I think this is looking pretty good, thank you! I'll leave it to 
>> @erichkeane to do the final sign-off as attributes code owner since this is 
>> making a fair number of changes in that area.
>
> Thanks for your review @aaron.ballman ! BTW, I have two questions:
>
> 1. Should we print the cached alignment value when dump AST?

I think it would be nice to dump that as well, but not critical.

> 2. The AlignedAttr::getAlignment has a bit redundant with GetAlignOfType(in 
> ExprConstant.cpp ), should we merge these?

It would be good to unify them, but that can be done in a follow-up as it's a 
bit orthogonal.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D150528/new/

https://reviews.llvm.org/D150528

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

Reply via email to