[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D133887#3799450 , @Izaron wrote: > In D133887#3799399 , @aaron.ballman > wrote: > >> I can fix up the C++ status page as well if you'd like, or I can hold off if >> you're alrea

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-19 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. In D133887#3799399 , @aaron.ballman wrote: > I can fix up the C++ status page as well if you'd like, or I can hold off if > you're already working on this, it's up to you! Thank you! I'll support empty labels in switch statement

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D133887#3799347 , @Izaron wrote: > In D133887#3799310 , @aaron.ballman > wrote: > >> Thank you for working on this! I spotted an issue where we're not quite >> complete with thi

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-19 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. In D133887#3799310 , @aaron.ballman wrote: > Thank you for working on this! I spotted an issue where we're not quite > complete with this implementation work. I pushed up a new test case in > https://github.com/llvm/llvm-project

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-19 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. Thank you for working on this! I spotted an issue where we're not quite complete with this implementation work. I pushed up a new test case in https://github.com/llvm/llvm-project/commit/a244194f73788de6dfd6d753ff09be3625613f9f that shows it (and set the C status

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG510383626fe1: [Clang] Support label at end of compound statement (authored by Izaron). Changed prior to commit: https://reviews.llvm.org/D133887?v

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision as: cor3ntin. cor3ntin added a comment. This revision is now accepted and ready to land. Thanks. This LGTM now Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133887/new/ https://reviews.llvm.org/D133887 _

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 461006. Izaron added a comment. Fix diagnostics for C. Thanks to @cor3ntin and @aaron.ballman! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133887/new/ https://reviews.llvm.org/D133887 Files: clang/docs/Rele

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D133887#3797614 , @Izaron wrote: >> Why checking getLangOpts().C99 instead of just C > > There is no `getLangOpts().C`. Here are possible C/C++ opt flags: > https://github.com/llvm/llvm-project/blob/7914e53e312074828293356f569

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. > Why checking getLangOpts().C99 instead of just C There is no `getLangOpts().C`. Here are possible C/C++ opt flags: https://github.com/llvm/llvm-project/blob/7914e53e312074828293356f569d190ac6eae3bd/clang/include/clang/Basic/LangOptions.def#L86-L100 I have no understandin

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added inline comments. Comment at: clang/lib/Parse/ParseStmt.cpp:738 +} else if (getLangOpts().C99 && !getLangOpts().C2x) { + Diag(Tok, diag::ext_c_label_end_of_compound_statement); +} I do not understand this. Why checking `getLangOpts().C9

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 461002. Izaron added a comment. Add diagnostics for C language Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133887/new/ https://reviews.llvm.org/D133887 Files: clang/docs/ReleaseNotes.rst clang/include/cla

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. In D133887#3797491 , @Izaron wrote: >> It should definitely be without warning in C23 mode and give an extension >> warning in earlier modes. > > @aaron.ballman we have this extension warning for pre-C++23: > > def ext_label_e

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. > It should definitely be without warning in C23 mode and give an extension > warning in earlier modes. @aaron.ballman we have this extension warning for pre-C++23: def ext_label_end_of_compound_statement : ExtWarn< "label at end of compound statement is a C++2b ex

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 460987. Izaron added a comment. Change C2x implementation status and C2x release notes Add an AST test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133887/new/ https://reviews.llvm.org/D133887 Files: clang/

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-15 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D133887#3790414 , @cor3ntin wrote: > Thanks for doing that work. > > Given that this paper was touted as a C compatibility feature, I think we > should implement the C feature at the same time > https://www.open-std.org/

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-14 Thread Shafik Yaghmour via Phabricator via cfe-commits
shafik added a comment. The proposal says: > A label at the end of a compound statement is treated as if it were followed > by a null statement Does it make sense to write an AST test to verify this? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-14 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. Thanks for doing that work. Given that this paper was touted as a C compatibility feature, I think we should implement the C feature at the same time https://www.open-std.org/jtc1/sc22/wg14/www/docs/n2508.pdf If my understanding is correct the change implemented here s

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. I thought it was a good idea to slightly rename `err_label_end_of_compound_statement` to `err_SWITCH_label_end_of_compound_statement` because we aren't going to support empty labels in switch statements. (GCC also doesn't compile it - https://godbolt.org/z/xabKaon8v) S

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: clang-language-wg, aaron.ballman. Herald added a project: All. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Implements paper P2324R2 https://wg21.link/p2324r2 https://github.co