[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

amccarth added a comment. A couple inline comments. I think this is looking pretty good. Comment at: lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/char8_t/main.cpp:1 +#include + Is this include necessary? Comment at: lldb/trunk/source

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rL369582: Add char8_t support (C++20) (authored by JDevlieghere, committed by ). Herald added a project: LLVM. Herald added

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

labath added a comment. In D66447#1639490 , @JDevlieghere wrote: > Sounds like I simply misunderstood your earlier comment. I thought you meant > putting a full UTF-8 *character* in a `char8_t. Ah yes, I can see how that request could have been interpr

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere added a comment. I changed the test to use `frame variable` again. With `target variable` the UTF-8 formatting doesn't work. Given that this patch just copies the Char16 and Char32 implementation, I think that's something for a different patch. I'll file a PR when this gets in.

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

shafik added a comment. In D66447#1638783 , @labath wrote: > In D66447#1638047 , @JDevlieghere > wrote: > > > In D66447#1637640 , @labath wrote: > > > > > This looks good to

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere updated this revision to Diff 216422. JDevlieghere added a comment. - Use UTF8 string CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66447/new/ https://reviews.llvm.org/D66447 Files: lldb/include/lldb/lldb-enumerations.h lldb/packages/Python/lldbsuite/test/lang/cpp/char

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere added a comment. In D66447#1638783 , @labath wrote: > In D66447#1638047 , @JDevlieghere > wrote: > > > In D66447#1637640 , @labath wrote: > > > > > This looks g

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

labath added a comment. In D66447#1638047 , @JDevlieghere wrote: > In D66447#1637640 , @labath wrote: > > > This looks good to me, but why are we using a nul character to test utf8 > > support? Shouldn't we insert

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere added a comment. In D66447#1637640 , @labath wrote: > This looks good to me, but why are we using a nul character to test utf8 > support? Shouldn't we insert some funnier characters too? I mean, one of the > advantages of unicode is that it

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere updated this revision to Diff 216224. JDevlieghere added a comment. - Remove clean rule - Use more readable names CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66447/new/ https://reviews.llvm.org/D66447 Files: lldb/include/lldb/lldb-enumerations.h lldb/packages/Python/

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

labath added a comment. This looks good to me, but why are we using a nul character to test utf8 support? Shouldn't we insert some funnier characters too? I mean, one of the advantages of unicode is that it should not be affected by the system code pages and such, so hopefully this would not ca

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere added a comment. In D66447#1636456 , @jblachly wrote: > Thank you for creating a revision and reviewing this. > > I made inline comments on the test harness and Dlang types / qualifiers. > > With removal of the Dlang types, where is the approp

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere updated this revision to Diff 216175. JDevlieghere marked an inline comment as done. JDevlieghere added a comment. - Address Pavel's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66447/new/ https://reviews.llvm.org/D66447 Files: lldb/include/lldb/lldb-enumerati

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

labath added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lang/cpp/char8_t/Makefile:4-6 +CFLAGS := -g -O0 -std=c++2a + +clean: OBJECTS+=$(wildcard main.d.*) Replace with `CFLAGS_EXTRAS+=-std=c++2a` Comment at: lldb/packages

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

jblachly added a comment. Thank you for creating a revision and reviewing this. I made inline comments on the test harness and Dlang types / qualifiers. With removal of the Dlang types, where is the appropriate place to put them? It is not clear to me whether language plugins can replace the fun

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere updated this revision to Diff 216008. JDevlieghere added a comment. - Feedback Greg. - Remove dlang types. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66447/new/ https://reviews.llvm.org/D66447 Files: lldb/include/lldb/lldb-enumerations.h lldb/packages/Python/lldbsui

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere marked an inline comment as done. JDevlieghere added inline comments. Comment at: lldb/packages/Python/lldbsuite/test/lang/cpp/char8_t/TestCxxChar8_t.py:26 + +@skipIf(compiler="clang", compiler_version=['<', '5.0']) +def test(self): This sho

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

clayborg requested changes to this revision. clayborg added inline comments. This revision now requires changes to proceed. Comment at: lldb/include/lldb/lldb-enumerations.h:170 // etc... + eFormatUnicode8, eFormatUnicode16, add to the end,

[Lldb-commits] [PATCH] D66447: Add char8_t support (C++20)

JDevlieghere created this revision. JDevlieghere added a reviewer: LLDB. Herald added a subscriber: teemperor. Herald added a project: LLDB. This patch adds support for C++20 char8_t, as well as dlang's char/wchar/dchar types. Original patch by James Blachly, modified by me. Repository: rLLD