[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added inline comments. Comment at: llvm/lib/Support/Unicode.cpp:272 +/// Unicode code points of the Cf category are considered +/// fornatting characters. +bool isFormatting(int UCS) { tahonermann wrote: > Thanks! I made a commit to fix it :) Reposito

[PATCH] D108469: Improve handling of static assert messages.

tahonermann added a comment. Herald added a reviewer: NoQ. I missed my opportunity to review while being on vacation last week. I reviewed to keep myself informed; spotted a typo. Comment at: llvm/lib/Support/Unicode.cpp:272 +/// Unicode code points of the Cf category are cons

[PATCH] D108469: Improve handling of static assert messages.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG64ab2b1dcc51: Improve handling of static assert messages. (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 440804. cor3ntin added a comment. - Undo the change of diagnostic message formatting as they break libc++ tests so we will keep printing instead Changing the format should be done -if at all- in a separate change. Repository: rG LLVM Github Monorepo

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added a comment. In D108469#3616983 , @leonardchan wrote: > I think this might be the cause of the many libc++ static_assert failures > we're seeing on our builders: > https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linu

[PATCH] D108469: Improve handling of static assert messages.

leonardchan added a comment. I think this might be the cause of the many libc++ static_assert failures we're seeing on our builders: https://luci-milo.appspot.com/ui/p/fuchsia/builders/toolchain.ci/clang-linux-x64/b8810079977358640657/overview Repository: rG LLVM Github Monorepo CHANGES SIN

[PATCH] D108469: Improve handling of static assert messages.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG870b6d218397: Improve handling of static assert messages. (authored by cor3ntin). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. LGTM! The precommit CI failures appear to be unrelated as best I can tell. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 440645. cor3ntin marked an inline comment as done. cor3ntin added a comment. Fix rst syntax Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/docs/Relea

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 440642. cor3ntin added a comment. Trigger a CI rerun Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added a comment. I've removed the parent revision as this review no longer requires it to proceed. That should get precommit CI working again. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 440640. cor3ntin added a comment. Try to fix the patch application issue Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/docs/ReleaseNotes.rst clang

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added a comment. Patch application is still failing the precommit CI; it'd be nice to get that corrected for some early test feedback. Comment at: clang/docs/ReleaseNotes.rst:275 This fixes `Issue 55962 `_. +

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; erichkeane wrote: > cor3ntin wrote: > > cor3nt

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; cor3ntin wrote: > cor3ntin wrote: > > erichkea

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; cor3ntin wrote: > erichkeane wrote: > > What abo

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; erichkeane wrote: > What about...? I doesn't see

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; What about...? Repository: rG LLVM Github

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; erichkeane wrote: > Something here that uses the

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 440354. cor3ntin added a comment. - Add release note - Address Erich's comment. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/docs/ReleaseNotes.rst

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added a comment. I'm generally OK with this. BUT Aaron/JF/Tom should review it. Comment at: clang/lib/Basic/Diagnostic.cpp:817 +if (isPrintable(*Begin) || isWhitespace(*Begin)) { + OutStr.push_back(*Begin); + ++Begin; OutStream << *Beg

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 440301. cor3ntin marked an inline comment as done. cor3ntin added a comment. - Add a comment to pushEscapedString - Use a stream interface which turns out to be simpler and cleaner, thanks Erich! - Fix typos and small nits Repository: rG LLVM Github Mon

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:16592 +StringLiteral *MsgStr = cast(AssertMessage); +if (MsgStr->getCharByteWidth() == 1) + Msg << MsgStr->getString(); cor3ntin wrote: > aaron.ballman wrote: > >

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:793 + OutStr.reserve(OutStr.size() + Str.size()); + auto it = reinterpret_cast(Str.data()); + auto end = it + Str.size(); aaron.ballman wrote: > Please fix the clang-tidy and clang-fo

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:821 + llvm::UTF32 CodepointValue; + llvm::UTF32 *CpPtr = &CodepointValue; + const unsigned char *CodepointBegin = Begin; aaron.ballman wrote: > We don't really need this var

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 440261. cor3ntin marked 5 inline comments as done. cor3ntin added a comment. - Address Aaron's and Erich's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 F

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: llvm/lib/Support/Unicode.cpp:268 + // hyphen in most terminals. + return Printables.contains(UCS) || UCS == 0x00AD; +} As a nit, this condition might be worth reversing to get the 'fast thing' on the LHS of the sho

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added reviewers: tahonermann, clang-language-wg. aaron.ballman added a comment. Thanks for picking this back up! I have mostly nits, a few questions, and am adding some extra reviewers who may have an opinion. Also, I think the summary needs to be updated (this is no longer waiting

[PATCH] D108469: Improve handling of static assert messages.

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

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 440063. cor3ntin marked 4 inline comments as done. cor3ntin added a comment. Herald added a subscriber: steakhal. Herald added a project: All. - Rebase - Address Aaron's comments. - Fix tests - Keep dumping wide/utf16/utf32 strings escaped until we can ensure

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added a comment. In D108469#3033475 , @aaron.ballman wrote: > Btw, this CI failure looks relevant: > https://reviews.llvm.org/harbormaster/unit/view/1055822/ but... it looks more > relevant to the parent patch than this one (and the parent see

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:792 +static void pushEscapedString(StringRef Str, SmallVectorImpl &OutStr) { + OutStr.reserve(OutStr.size() + Str.size()); + const unsigned char *Begin = aaron.ballman wrote: > cor3

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:792 +static void pushEscapedString(StringRef Str, SmallVectorImpl &OutStr) { + OutStr.reserve(OutStr.size() + Str.size()); + const unsigned char *Begin = cor3ntin wrote: > jfb wrote

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added a comment. Btw, this CI failure looks relevant: https://reviews.llvm.org/harbormaster/unit/view/1055822/ but... it looks more relevant to the parent patch than this one (and the parent seems to have a clean CI). May be worth looking into whether this is an issue or not. =

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:792 +static void pushEscapedString(StringRef Str, SmallVectorImpl &OutStr) { + OutStr.reserve(OutStr.size() + Str.size()); + const unsigned char *Begin = jfb wrote: > Can this addition o

[PATCH] D108469: Improve handling of static assert messages.

jfb added a comment. Can you test all the values in this? https://godbolt.org/z/h7n54fa5x Comment at: clang/lib/Basic/Diagnostic.cpp:792 +static void pushEscapedString(StringRef Str, SmallVectorImpl &OutStr) { + OutStr.reserve(OutStr.size() + Str.size()); + const unsigned cha

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 375273. cor3ntin added a comment. Correctly rebase on top of D105759 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clan

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 375251. cor3ntin added a comment. Fix build and address most of Aaron's comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/include/clang/Basic/D

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 375244. cor3ntin added a comment. Rebasing Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Basi

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 373408. cor3ntin added a comment. Properly comment isFormatting/isPrintable. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/include/clang/Basic/Diagn

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added a comment. In D108469#3002209 , @aaron.ballman wrote: > In D108469#2957652 , @jfb wrote: > >> I worry that changing the general `static_assert` printing (adding a colon, >> and dropping the quotes

[PATCH] D108469: Improve handling of static assert messages.

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

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 373404. cor3ntin marked 5 inline comments as done. cor3ntin added a comment. - Fix tests, cland tidy Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/i

[PATCH] D108469: Improve handling of static assert messages.

aaron.ballman added a comment. In D108469#2957652 , @jfb wrote: > I worry that changing the general `static_assert` printing (adding a colon, > and dropping the quotes) will get @hwright's law to drop on us. We can try > and see if e.g. users of clang h

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin planned changes to this revision. cor3ntin added a comment. I'll fix the tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 ___ cfe-commits mailing

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 367844. cor3ntin added a comment. Remove superfluous assert Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 367840. cor3ntin added a comment. Herald added subscribers: llvm-commits, dexonsmith, hiraditya. Herald added a project: LLVM. - Add more tests - Add a table for format codepoints - which we want to output as is. These include among other ZWJ (for emojis) an

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:818 + llvm::raw_string_ostream stream(number); + stream << ""; + OutStr.append(number.begin(), number.end()); jfb wrote: > We don't have a better hex formatter? 😟 > Not a bi

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin added a comment. In D108469#2957652 , @jfb wrote: > I worry that changing the general `static_assert` printing (adding a colon, > and dropping the quotes) will get @hwright's law to drop on us. We can try > and see if e.g. users of clang have a

[PATCH] D108469: Improve handling of static assert messages.

jfb added a subscriber: hwright. jfb added a comment. I worry that changing the general `static_assert` printing (adding a colon, and dropping the quotes) will get @hwright's law to drop on us. We can try and see if e.g. users of clang have automated checks for `static_assert` in their CI pipel

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 367812. cor3ntin added a comment. Rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108469/new/ https://reviews.llvm.org/D108469 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Basic/

[PATCH] D108469: Improve handling of static assert messages.

cor3ntin updated this revision to Diff 367802. cor3ntin retitled this revision from "Improve handling of static assert messages. Instead of dumping the string literal (which quotes it and escape every non-ascii symbol), we can use the content of the string (which we know is valid UTF-8 by virtue

[PATCH] D108469: Improve handling of static assert messages. Instead of dumping the string literal (which quotes it and escape every non-ascii symbol), we can use the content of the string (which we

cor3ntin created this revision. Herald added a subscriber: martong. cor3ntin requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. ...g). `FormatDiagnostic` is modified to escape non printable characters and invalid UTF-8. This ensures that uni