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