[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin added inline comments. Comment at: llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp:46 + auto SecondSemiPos = Line.find(';', FirstSemiPos + 1); + if (FirstSemiPos == std::string::npos) +continue; RKSimon wrote: > @cor3ntin Should

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

RKSimon added inline comments. Comment at: llvm/utils/UnicodeData/UnicodeNameMappingGenerator.cpp:46 + auto SecondSemiPos = Line.find(';', FirstSemiPos + 1); + if (FirstSemiPos == std::string::npos) +continue; @cor3ntin Should this be SecondSemi

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin added a comment. @aaron.ballman @tahonermann Thanks for the review. I landed the change after confirming with Aaron he was happy with it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 _

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc92056d03881: [Clang][C++23] P2071 Named universal character escapes (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LA

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

tahonermann accepted this revision as: tahonermann. tahonermann added a comment. This revision is now accepted and ready to land. > @tahonermann gentle ping (Aaron told me you might have further comments) I'm sorry for the delay. I ran out of time to do the thorough review I would have liked to

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin added a comment. @tahonermann gentle ping (Aaron told me you might have further comments) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 ___ cfe-commi

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin added a comment. In D123064#3585074 , @aaron.ballman wrote: > I had a discussion about the license file on IRC (but not with @tstellar) and > the thinking there was: add the license file. It seems to either be required > or would be harmless t

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 437186. cor3ntin added a comment. Fix wrapping in file header Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang/include/clang/Basic/DiagnosticLexKinds.t

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 437184. cor3ntin added a comment. Add unicode license notice to generated file derived fromn the unicode data Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files:

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 437160. cor3ntin marked 17 inline comments as done. cor3ntin added a comment. Address more style issues found by Aaron Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

aaron.ballman added a comment. I had a discussion about the license file on IRC (but not with @tstellar) and the thinking there was: add the license file. It seems to either be required or would be harmless to add. So if we don't hear something different from @tstellar on this, I think you shou

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

aaron.ballman added inline comments. Comment at: clang/lib/Lex/LiteralSupport.cpp:509-511 +if (std::max(Distance, Match.Distance) - +std::min(Distance, Match.Distance) > +3) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

aaron.ballman added a comment. Some likely final coding style nits. The only thing of substance I'm waiting on is an answer to whether we need to update a license file in order to comply with the Unicode license requirements. @tstellar, any chance you can help there? Comment

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 436703. cor3ntin added a comment. Fix more style issue (variable casing) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang/include/clang/Basic/Diagnosti

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin marked an inline comment as done. cor3ntin added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:3237-3239 + if (C != '{') { +if (Diagnose) + Diag(StartPtr, diag::warn_ucn_escape_incomplete); aaron.ballman wrote: > Is this a case where we

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 436682. cor3ntin marked 12 inline comments as done. cor3ntin added a comment. Regenerate UnicodeNameToCodepointGenerated.cpp to fix the formatting of its header. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D12

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 436661. cor3ntin marked an inline comment as done. cor3ntin added a comment. Address Aaron's comments - `{}` => `llvm::None` in Lexer.cpp - Fix casing in UnicodeNameToCodepoint.cpp to match the style, and a couple I missed in UnicodeNameMappingGenerator.cpp

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

aaron.ballman added inline comments. Comment at: llvm/lib/Support/UnicodeNameToCodepoint.cpp:49 +std::string s; +s.reserve(64); +auto n = this; cor3ntin wrote: > aaron.ballman wrote: > > aaron.ballman wrote: > > > Any particular reason for 64? > > Sti

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin added inline comments. Comment at: llvm/lib/Support/UnicodeNameToCodepoint.cpp:49 +std::string s; +s.reserve(64); +auto n = this; aaron.ballman wrote: > aaron.ballman wrote: > > Any particular reason for 64? > Still wondering why 64 bytes spe

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

aaron.ballman added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:3139 Diag(SlashLoc, diag::warn_ucn_not_valid_in_c89); -return 0; +return {}; } aaron.ballman wrote: > FWIW, I think using `llvm::None` instead of `{}` is more clear to read

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

tstellar added a comment. In D123064#3575503 , @cor3ntin wrote: > @tstellar I saw you say in another path that you got confirmation from > lawyers that it's okay to include UnicodeData.txt (which this patch doesn't > do) and derived data (which this pat

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 436142. cor3ntin added a comment. Do not hardcode the list of generated code points ranges in the generator code to ease maintainance burden. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://r

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 436137. cor3ntin added a comment. Use utohexstr and revert the changes that put to_hexString in StringExtras. I'll remove to_hexString entierly in a separate NFC change to avoid duplication and further confusion. Repository: rG LLVM Github Monorepo CHANG

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin added a comment. @tstellar I saw you say in another path that you got confirmation from lawyers that it's okay to include UnicodeData.txt (which this patch doesn't do) and derived data (which this patch does). Can you confirm? Thanks Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 436133. cor3ntin added a comment. - Rebase - The generator code is more consistent with LLVM style guides. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: c

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

aaron.ballman added inline comments. Comment at: llvm/include/llvm/ADT/StringExtras.h:329 +std::string to_hexString(uint64_t Value, bool UpperCase = true); + cor3ntin wrote: > aaron.ballman wrote: > > `utohexstr()` already exists on line 152 -- any reason we ca

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin added inline comments. Comment at: clang/lib/Lex/LiteralSupport.cpp:238 + diag::err_delimited_escape_missing_brace) +<< std::string(1, 'o'); aaron.ballman wrote: > Can you use `"o"` instead of constructing a `std::string` for it

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 421958. cor3ntin added a comment. Cleanup generated code header comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang/include/clang/Basic/Diagnostic

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 421956. cor3ntin marked 17 inline comments as done. cor3ntin added a comment. Address Aaron's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

lattner added a comment. Yeah, we should discuss this, thanks for raising this Aaron. I'm not sure exactly what is being pulled in: @cor3ntin can you please email a summary of the situation to bo...@llvm.org and we'll discuss it and run it by Heather as needed? Thanks -Chris Repository:

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

aaron.ballman added reviewers: aaron.ballman, tahonermann, clang-language-wg. aaron.ballman added subscribers: tstellar, lattner. aaron.ballman added a comment. Thank you for this functionality! I did a pretty quick pass over it and have some comments. Comment at: clang/includ

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 421939. cor3ntin added a comment. Add a test for `\o' Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang/include/clang/Basic/DiagnosticLexKinds.td clan

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 421787. cor3ntin added a comment. A non-existing name could return an engaged value if the whole string matched the node's name, even if that node had no attached value. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

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

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 421769. cor3ntin added a comment. Fix linking on windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang/include/clang/Basic/DiagnosticLexKinds.td

[PATCH] D123064: [Clang][C++23] P2071 Named universal character escapes

cor3ntin updated this revision to Diff 421768. cor3ntin added a comment. Add missing header in generated code to fix windows CI. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123064/new/ https://reviews.llvm.org/D123064 Files: clang/include/clan