[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2023-01-03 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman abandoned this revision. aaron.ballman added a comment. In D132877#4023269 , @cor3ntin wrote: > @aaron.ballman I don't think this patch is no longer necessary given that we > merged the math profile Agreed, I'm going to abandon this for

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2023-01-03 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment. > I don't think this patch is no longer necessary given that we merged the math > profile I agree; we can revisit this if complaints from users due to use of characters not in the math profile materializes. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2023-01-03 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman I don't think this patch is no longer necessary given that we merged the math profile Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132877/new/ https://reviews.llvm.org/D132877

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. @aaron.ballman i am not worried about performance. The old table was fairly compact - as it described large ranges - so a drop in the ocean in terms of binary size. And we only need to check them for the case where we are only going to emit a warning or an error

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as done. aaron.ballman added a comment. In D132877#3756458 , @cor3ntin wrote: > I am strongly against this change. If we do want to do this, we need to > restore the original tables from c++11 and check against

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Tom Honermann via Phabricator via cfe-commits
tahonermann added a comment. I don't have a strong opinion regarding when, or if, the diagnostic is reverted to an always-error. It looks like gcc is not even planning to diagnose identifiers that are ill-formed according to the new rules by default. With regard to Corentin's opposition to the

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. I'm not really a fan of the argument of "this was accepted as a DR, therefore new versions of clang have to reject code that old versions accepted". Regardless of what the C++ committee does, our commitment to our users is to ensure that code written against

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin added a comment. I am strongly against this change. If we do want to do this, we need to restore the original tables from c++11 and check against them not to extend the set to something much wider than was supported before c++23. It should be fairly easy to extract those tables from

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Tobias Hieta via Phabricator via cfe-commits
thieta added a comment. Hmm. 15.0.0 is just a week away - I am not planning any more RCs unless we hit something critical. What's the risk of taking this specific change at this point? Would it make more sense to wait for 15.0.1? (I am guessing it's better if it goes into 15.0.0 or not in 15.x

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman marked an inline comment as done. aaron.ballman added a comment. In D132877#3756328 , @efriedma wrote: > If we don't specifically call out the deprecation period in the diagnostic, > usage will grow beyond what we expect. (Most people

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Erich Keane via Phabricator via cfe-commits
erichkeane added a comment. In D132877#3756328 , @efriedma wrote: > If we don't specifically call out the deprecation period in the diagnostic, > usage will grow beyond what we expect. (Most people don't read the release > notes; they'll just see it's

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Eli Friedman via Phabricator via cfe-commits
efriedma added a comment. If we don't specifically call out the deprecation period in the diagnostic, usage will grow beyond what we expect. (Most people don't read the release notes; they'll just see it's possible to turn off the error message, and turn it off.) If we're okay with people

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added subscribers: thieta, tstellar. aaron.ballman added a comment. CC @tstellar and @thieta for any early backporting concerns. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132877/new/ https://reviews.llvm.org/D132877

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

2022-08-29 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: tahonermann, cor3ntin, clang-language-wg. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. WG21 P1949 and WG14 N2836 were both adopted