[PATCH 4/4] RISC-V: Print comma and tabs as the `text' style

[Tsukasa OI]

On 2022/08/10 20:20, Andrew Burgess wrote:
> Tsukasa OI via Binutils <binutils@sourceware.org> writes:
> 
>> On the RISC-V disassembler, some separators have non-`text' style when
>> printed with another word with another style.
>>
>> This commit splits those, making sure that those comma and tabs are printed
>> with the `text' style.
>>
>> opcodes/ChangeLog:
>>
>> 	* riscv-dis.c (print_insn_args): Split and print the comma as text.
>> 	(riscv_disassemble_insn): Split and print tabs as text.
>> 	(riscv_disassemble_data): Likewise.
> 
> I'm not a binutils maintainer, so can't approve this patch.  However,
> this looks good to me with one minor nit, see below...
> 
>> ---
>>  opcodes/riscv-dis.c | 31 ++++++++++++++++++++-----------
>>  1 file changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
>> index fe18c18642e..460f77676d2 100644
>> --- a/opcodes/riscv-dis.c
>> +++ b/opcodes/riscv-dis.c
>> @@ -373,9 +373,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
>>  		     (int)EXTRACT_RVV_OFFSET (l));
>>  	      break;
>>  	    case 'm':
>> -	      if (! EXTRACT_OPERAND (VMASK, l))
>> -		print (info->stream, dis_style_register, ",%s",
>> -		       riscv_vecm_names_numeric[0]);
>> +	      if (!EXTRACT_OPERAND (VMASK, l))
>> +		{
>> +		  print (info->stream, dis_style_text, ",");
>> +		  print (info->stream, dis_style_register, "%s",
>> +			 riscv_vecm_names_numeric[0]);
>> +		}
>>  	      break;
>>  	    }
>>  	  break;
>> @@ -709,7 +712,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
>>      case 4:
>>      case 8:
>>        (*info->fprintf_styled_func)
>> -	(info->stream, dis_style_assembler_directive, ".%dbyte\t", insnlen);
>> +	(info->stream, dis_style_assembler_directive, ".%dbyte", insnlen);
>> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>>        (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
>>  				    "0x%llx", (unsigned long long) word);
>>        break;
>> @@ -717,7 +721,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
>>        {
>>          int i;
>>  	(*info->fprintf_styled_func)
>> -	  (info->stream, dis_style_assembler_directive, ".byte\t");
>> +	  (info->stream, dis_style_assembler_directive, ".byte");
>> +	(*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>>          for (i = 0; i < insnlen; ++i)
>>            {
>>              if (i > 0)
>> @@ -905,21 +910,24 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>>      case 1:
>>        info->bytes_per_line = 6;
>>        (*info->fprintf_styled_func)
>> -	(info->stream, dis_style_assembler_directive, ".byte\t");
>> -      (*info->fprintf_styled_func)
>> -	(info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
>> +	(info->stream, dis_style_assembler_directive, ".byte");
>> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>> +      (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
>> +				    "0x%02x", (unsigned)data);
> 
> You lost the space in '(unsigned) data'.
> 
> Thanks,
> Andrew

Interesting.

I formatted this line using Visual Studio Code (which uses clang-format)
and I think this is a minor difference between clang-format's "GNU"
indent and real GNU Indent's defaults.

Changing clang-format's settings just like "SpaceAfterCStyleCast: true"
will mimic the default GNU Indent's behavior (better).  It's too minor
(this source file has other (type)(value) without any spaces) and will
not republish the patchset unless there's some other reasons.

But thank you for letting me know that there's something clang-format
misses and I will follow "SpaceAfterCStyleCast: true" in the future.

Thanks,
Tsukasa

> 
>>        break;
>>      case 2:
>>        info->bytes_per_line = 8;
>>        (*info->fprintf_styled_func)
>> -	(info->stream, dis_style_assembler_directive, ".short\t");
>> +	(info->stream, dis_style_assembler_directive, ".short");
>> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>>        (*info->fprintf_styled_func)
>>  	(info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
>>        break;
>>      case 4:
>>        info->bytes_per_line = 8;
>>        (*info->fprintf_styled_func)
>> -	(info->stream, dis_style_assembler_directive, ".word\t");
>> +	(info->stream, dis_style_assembler_directive, ".word");
>> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>>        (*info->fprintf_styled_func)
>>  	(info->stream, dis_style_immediate, "0x%08lx",
>>  	 (unsigned long) data);
>> @@ -927,7 +935,8 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>>      case 8:
>>        info->bytes_per_line = 8;
>>        (*info->fprintf_styled_func)
>> -	(info->stream, dis_style_assembler_directive, ".dword\t");
>> +	(info->stream, dis_style_assembler_directive, ".dword");
>> +      (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>>        (*info->fprintf_styled_func)
>>  	(info->stream, dis_style_immediate, "0x%016llx",
>>  	 (unsigned long long) data);
>> -- 
>> 2.34.1
>