Re: [PATCH] Allow DW_ATE_UTF for Rust characters

[Tom Tromey <tom@tromey.com>]

>> Changing this code to use a character type revealed a couple of
>> oddities in the C/C++ handling of TYPE_CODE_CHAR.  This patch fixes
>> these as well.

Joel> I don't see any real problem with this patch, and in particular
Joel> the change in dwarf2/read.c looks good to me. But I couldn't
Joel> really grasp what the oddities you are referring to were, and
Joel> so I don't understand the changes in c-lang.c and c-valprint.c
Joel> Can you tell us more about them?

Sure thing, will do so inline, below.

>> @@ -88,7 +88,7 @@ classify_type (struct type *elttype, struct gdbarch *gdbarch,
>> {
>> const char *name = elttype->name ();
>> 
>> -      if (elttype->code () == TYPE_CODE_CHAR || !name)
>> +      if (name == nullptr)
>> {

Here the code assumes that any TYPE_CODE_CHAR type that reaches this
point must be the C "char" type.  That seems unwarranted, and also
causes a test failure if it is left out of the patch, because all
DW_ATE_UTF types will now end up as TYPE_CODE_CHAR.  Previously they
used TYPE_CODE_INT because they used one of these:

  builtin_type->builtin_char16
    = arch_integer_type (gdbarch, 16, 1, "char16_t");
  builtin_type->builtin_char32
    = arch_integer_type (gdbarch, 32, 1, "char32_t");

>> @@ -438,6 +438,7 @@ c_value_print_inner (struct value *val, struct ui_file *stream, int recurse,
>> c_value_print_struct (val, stream, recurse, options);
>> break;
>> 
>> +    case TYPE_CODE_CHAR:
>> case TYPE_CODE_INT:
>> c_value_print_int (val, stream, options);

This change just arranges for TYPE_CODE_CHAR to use the C-specific print
function (which already handles C character types).  Without this, the
change to use TYPE_CODE_CHAR would cause the code to use the generic
printing functions, which IIRC don't use the C charset decoding code.

>> +            char_label: DW_TAG_base_type {
>> +                {DW_AT_byte_size 4 DW_FORM_sdata}
>> +                {DW_AT_encoding @DW_ATE_UTF}
>> +                {DW_AT_name char}
>> +            }
>> +
>> +	    DW_TAG_variable {
>> +		{name cvalue}
>> +		{type :$char_label}
>> +		{const_value 97 DW_FORM_udata}

Joel> I'm wondering if there might be thinko here or not (not sure
Joel> I interpret the description correctly): In the base type DIE,
Joel> we say DW_FORM_sdata, but then for the value, we say
Joel> DW_FORM_udata. Should these two match, are they separate
Joel> entities?

They are separate.  The DW_FORM_sdata just describes the form of the
size value for the base type.  Either sdata or udata would be fine
there; this doesn't affect the signed-ness of the type itself, which is
what matters for the value of the variable.

Tom