[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf75b73549d4a: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas (authored by yronglin). Repository: rG LLVM Github

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin marked 2 inline comments as done. yronglin added a comment. The libcxx CI failure seems not caused bye this patch, I have found that there have a patch to fix this issue. https://reviews.llvm.org/D151508 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

erichkeane added a comment. Still LGTM, thanks! Comment at: clang/lib/AST/AttrImpl.cpp:247 + + if (isAlignmentExpr()) { +return alignmentExpr yronglin wrote: > erichkeane wrote: > > Typically we don't do curleys on a single-statement... but I am on the >

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin marked 2 inline comments as done. yronglin added inline comments. Comment at: clang/lib/AST/AttrImpl.cpp:247 + + if (isAlignmentExpr()) { +return alignmentExpr erichkeane wrote: > Typically we don't do curleys on a single-statement... but I am on

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin updated this revision to Diff 525636. yronglin added a comment. Address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150528/new/ https://reviews.llvm.org/D150528 Files: clang/docs/ReleaseNotes.rst

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. 1 nit I don't care either way about, 1 suggestion. Comment at: clang/lib/AST/AttrImpl.cpp:247 + + if (isAlignmentExpr()) { +return alignmentExpr

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin marked an inline comment as done. yronglin added a comment. Thanks for your comments, @erichkeane ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150528/new/ https://reviews.llvm.org/D150528

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin updated this revision to Diff 525614. yronglin added a comment. Address comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150528/new/ https://reviews.llvm.org/D150528 Files: clang/docs/ReleaseNotes.rst

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

erichkeane added a comment. Just 1 change I'd like, but otherwise this LGTM. Comment at: clang/lib/AST/AttrImpl.cpp:244 + assert(!isAlignmentDependent()); + auto getAlignmentImpl = [&]() -> unsigned { +if (isAlignmentExpr()) { I don't think it makes

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin added a comment. ping～ @erichkeane 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

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin marked 2 inline comments as done. yronglin added a comment. Thanks, maybe I can do these trivially change in separate patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150528/new/ https://reviews.llvm.org/D150528

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin updated this revision to Diff 525224. yronglin added a comment. Address comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150528/new/ https://reviews.llvm.org/D150528 Files: clang/docs/ReleaseNotes.rst

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

aaron.ballman added a comment. In D150528#4368023 , @yronglin wrote: > In D150528#4365688 , @aaron.ballman > wrote: > >> In general, I think this is looking pretty good, thank you! I'll leave it to >>

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin added a comment. In 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

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

aaron.ballman added a comment. 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. Comment at:

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin updated this revision to Diff 524165. yronglin added a comment. Format. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150528/new/ https://reviews.llvm.org/D150528 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Parse/Parser.h

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin updated this revision to Diff 524159. yronglin added a comment. Update Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150528/new/ https://reviews.llvm.org/D150528 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Parse/Parser.h

[PATCH] D150528: [Clang][Attribute] Improve the AST/diagnoses fidelity of alignas and _Alignas

yronglin updated this revision to Diff 524158. yronglin added a comment. Herald added a reviewer: jdoerfert. Herald added subscribers: jplehr, sstefan1. Fix crash. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150528/new/