[PATCH] D104975: Implement P1949

intractabilis added a comment. In D104975#3493992 , @tahonermann wrote: > Please refrain from such unhelpful comments. Absolutely, no problem. It's great that my other comments were helpful. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D104975: Implement P1949

tahonermann added a comment. > Since it is a C++23 feature, can we at least have it only if the user > requests C++23 standard compliance? WG21 adopted P1949 as a defect report (DR) against prior standards with the intent that the changes be applied retroactive

[PATCH] D104975: Implement P1949

intractabilis added a comment. I checked the latest GCC 12. Despite P1949 support, I can use math symbols like 𝜕 and 𝛻 without any problem with any combination of `-std=c++xx`, xx ≠ 98, xx ≠ 03. Thanks, God, there are reasonable people in the GCC team. test.cpp

[PATCH] D104975: Implement P1949

intractabilis added a comment. In D104975#3492099 , @tahonermann wrote: > I am quite sure that gcc will also be implementing P1949 > as adopted for C++23. Yes, you are right, it is available in GCC 12 (https://gcc.gnu.

[PATCH] D104975: Implement P1949

tahonermann added a comment. > I hope at least GCC will not implement this arbitrary restriction on math > symbols. I would argue a partial derivative symbol in C++ has more sense than > a skull emoji. I am quite sure that gcc will also be implementing P1949 as

[PATCH] D104975: Implement P1949

intractabilis added a comment. I hope at least GCC will not implement this arbitrary restriction on math symbols. I would argue a partial derivative symbol in C++ has more sense than a skull emoji. P.S. I would love to see features like `std::jthread` in Clang instead of removing math symbols

[PATCH] D104975: Implement P1949

sdowney added a comment. In D104975#3486313 , @intractabilis wrote: > Can you roll this back and don't support P1949 > ? For some inexplicable reason ∂, 𝜕 partial > derivative symbols are now not supported. Neither as XI

[PATCH] D104975: Implement P1949

aaron.ballman added a comment. In D104975#3486313 , @intractabilis wrote: > Can you roll this back and don't support P1949 > ? No; P1949 was adopted for C++23, so this is effectively a n

[PATCH] D104975: Implement P1949

intractabilis added a comment. Herald added a project: All. Can you roll this back and don't support P1949 ? For some inexplicable reason ∂, 𝜕 partial derivative symbols are now not supported. Neither as XID_Start nor as XID_Continue. Where is logic in that? It h

[PATCH] D104975: Implement P1949

aaron.ballman closed this revision. aaron.ballman added a comment. Thank you for the patch, I've landed it on your behalf in 4e80636db71a1b6123d15ed1f9eda3979b4292de Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 366865. cor3ntin added a comment. Fix comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/CharInfo.h clang/include/clang/Bas

[PATCH] D104975: Implement P1949

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM aside from a commenting nit that I managed to miss last time (sorry about that). Btw, would you like to apply for commit privileges now that you've gotten a few patches unde

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 366860. cor3ntin marked an inline comment as done. cor3ntin added a comment. Fix Aaron's comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/inclu

[PATCH] D104975: Implement P1949

aaron.ballman added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1622-1623 + + const bool isIDStart = isAllowedInitiallyIDChar(CodePoint, LangOpts); + const bool isIDContinue = isIDStart || isAllowedIDChar(CodePoint, LangOpts); + Com

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 366430. cor3ntin added a comment. Fix test formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/CharInfo.h clang/include/c

[PATCH] D104975: Implement P1949

cor3ntin marked 2 inline comments as done. cor3ntin added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:117 +def err_character_not_allowed : Error< + "character not allowed in identifiers">; def ext_unicode_whitespace : ExtWarn<

[PATCH] D104975: Implement P1949

cor3ntin added a comment. In D104975#2944703 , @rsmith wrote: > In D104975#2944313 , @cor3ntin > wrote: > >> @rsmith: I modified the script locally to support dxx: dup P - let me >> know if you think that's

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 366426. cor3ntin added a comment. Formatting Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/CharInfo.h clang/include/clang/Basi

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 366425. cor3ntin added a comment. - Support superseeding DRs with paper in make_cxx_dr_status (ie //drXXX: sup P) - Better diagnostic message when a codepoint is identifier continue but not identifier start - Provide a different diagnostic message for

[PATCH] D104975: Implement P1949

rsmith added inline comments. Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:117 +def err_character_not_allowed : Error< + "character not allowed in identifiers">; def ext_unicode_whitespace : ExtWarn< The old diagnostic text here was bad in the c

[PATCH] D104975: Implement P1949

rsmith added a comment. In D104975#2944313 , @cor3ntin wrote: > @rsmith: I modified the script locally to support dxx: dup P - let me > know if you think that's a good solution Not sure if that's a typo: did you mean "sup" rather than "dup"? In this

[PATCH] D104975: Implement P1949

rsmith added inline comments. Comment at: clang/test/CXX/drs/dr2xx.cpp:600 -namespace dr248 { // dr248: yes c++11 - // FIXME: Should this also apply to c++98 mode? This was a DR against C++98. aaron.ballman wrote: > cor3ntin wrote: > > aaron.ballman wrote: > >

[PATCH] D104975: Implement P1949

cor3ntin added a comment. @rsmith: I modified the script to support dxx: dup P - let me know if you think that's a good solution Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 __

[PATCH] D104975: Implement P1949

cor3ntin added inline comments. Comment at: clang/www/cxx_dr_status.html:3 "http://www.w3.org/TR/html4/strict.dtd";> aaron.ballman wrote: > You might have missed this at the top of the file -- this one is a generated > file, and that's why I was t

[PATCH] D104975: Implement P1949

aaron.ballman added inline comments. Comment at: clang/www/cxx_dr_status.html:3 "http://www.w3.org/TR/html4/strict.dtd";> You might have missed this at the top of the file -- this one is a generated file, and that's why I was trying to figure out

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 366315. cor3ntin added a comment. Rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/DiagnosticLexKinds.td clang/lib/Lex/L

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 366286. cor3ntin added a comment. Update dr_cxx_status and add comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/Diagnostic

[PATCH] D104975: Implement P1949

aaron.ballman added a reviewer: rsmith. aaron.ballman added a comment. Adding @rsmith to hopefully answer the question about how to handle the dr status page for these changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://revi

[PATCH] D104975: Implement P1949

aaron.ballman added a subscriber: rsmith. aaron.ballman added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1444 +static const llvm::sys::UnicodeCharSet XIDContinueChars(XIDContinueRanges); +return C == '_' || XIDStartChars.contains(C) || + XIDContinueCha

[PATCH] D104975: Implement P1949

cor3ntin added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1444 +static const llvm::sys::UnicodeCharSet XIDContinueChars(XIDContinueRanges); +return C == '_' || XIDStartChars.contains(C) || + XIDContinueChars.contains(C); aaron.ballman

[PATCH] D104975: Implement P1949

aaron.ballman added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1444 +static const llvm::sys::UnicodeCharSet XIDContinueChars(XIDContinueRanges); +return C == '_' || XIDStartChars.contains(C) || + XIDContinueChars.contains(C); Is lookin

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 364941. cor3ntin added a comment. Update cxx_status Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/DiagnosticLexKinds.td clang/

[PATCH] D104975: Implement P1949

cor3ntin added inline comments. Comment at: clang/lib/Lex/UnicodeCharSets.h:229 +static const llvm::sys::UnicodeCharRange XIDContinueRanges[] = { +{0x0030, 0x0039}, {0x005F, 0x005F}, {0x00B7, 0x00B7}, +{0x0300, 0x036F}, {0x0387, 0x0387}, {0x0483, 0x0487},

[PATCH] D104975: Implement P1949

cor3ntin added a comment. In D104975#2842425 , @jfb wrote: > It would be more user-friendly to say which character is not allowed in the > diagnostic. Agreed, done! > Do we need to have a backward-compatible flag to preserve the old behavior, > or do

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 355036. cor3ntin added a comment. Herald added subscribers: llvm-commits, dexonsmith. Herald added a project: LLVM. Optimize character lookup for ascii Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 354973. cor3ntin added a comment. Fix clang tidy check, update cxx_status Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/Diagnost

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 354684. cor3ntin added a comment. Missing EOF Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/DiagnosticLexKinds.td clang/lib/Le

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 354683. cor3ntin added a comment. Improve commit message Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/DiagnosticLexKinds.td c

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 354681. cor3ntin added a comment. More code format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/DiagnosticLexKinds.td clang/l

[PATCH] D104975: Implement P1949

cor3ntin updated this revision to Diff 354679. cor3ntin added a comment. Clang format, cleanup and better diagnostics Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104975/new/ https://reviews.llvm.org/D104975 Files: clang/include/clang/Basic/Dia

[PATCH] D104975: Implement P1949

cor3ntin added a comment. In D104975#2842425 , @jfb wrote: > It would be more user friendly to say which character is not allowed in the > diagnostic. Agreed > Do we need to have a backwards compatible flag to preserve the old behavior, > or do we bel

[PATCH] D104975: Implement P1949

jfb added a comment. It would be more user friendly to say which character is not allowed in the diagnostic. Do we need to have a backwards compatible flag to preserve the old behavior, or do we believe that nobody will be affected by the change? We should make this choice explicitly and state

[PATCH] D104975: Implement P1949

Quuxplusone added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1462 + static const llvm::sys::UnicodeCharSet XIDStartChars(XIDStartRanges); + return C == '_' || XIDStartChars.contains(C); + } else if (LangOpts.C11) { This is overly inde

[PATCH] D104975: Implement P1949

cor3ntin created this revision. cor3ntin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This adds the Unicode 13 data for XID_Start and XID_Continue. The definition of valid identifier is changed in all C++ modes as P1949