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

[James S Blachly, MD via Phabricator via lldb-commits]

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 functionality in ClangASTContext::GetBuiltinTypeForDWARFEncodingAndBitSize.

Thanks again



================
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):
----------------
JDevlieghere wrote:
> This should be 7
I believe clang7 requires -fchar8_t, whereas the test harness here passes -std=c++2a ; char8_t is enabled via -std=c++2a beginning in clang-8

"(11): Prior to Clang 8, this feature is not enabled by -std=c++2a, but can be enabled with -fchar8_t. "

https://clang.llvm.org/cxx_status.html#p0482


================
Comment at: lldb/source/Symbol/ClangASTContext.cpp:1383-1384
+            streq(type_name, "wchar") || // dlang
+            streq(type_name, "const(wchar)") ||
+            streq(type_name, "immutable(wchar)")) {
           return CompilerType(this, ast->Char16Ty.getAsOpaquePtr());
----------------
clayborg wrote:
> are "const(T)" and "immutable(T)" the actualy type names or are they layers on top of a base "wchar" type? These shouldn't be needed if so as the base "wchar" type should end up handling the base type correctly. Remove?
In LDC (the LLVM D compiler), application of type qualifier immutable or const to a type T defines a new type.
I am not a DWARF expert, but running a sample program through lldb seems to confirm this:

error: need to add support for DW_TAG_base_type 'char' encoded with DW_ATE = 0x10, bit_size = 8
error: need to add support for DW_TAG_base_type 'const(char)' encoded with DW_ATE = 0x10, bit_size = 8
error: need to add support for DW_TAG_base_type 'immutable(char)' encoded with DW_ATE = 0x10, bit_size = 8

Whereas -- interestingly -- the reference compiler DMD encodes them differently (as char, const char, and char; respectively as the DWARF spec I guess has no qualifier for immutable). The LDC behavior is IMO more true to language spec.



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66447/new/

https://reviews.llvm.org/D66447