Re: [PATCH v2 05/65] Arm: use is_whitespace()

["Richard Earnshaw (lists)" <Richard.Earnshaw@arm.com>]

On 27/01/2025 16:55, Jan Beulich wrote:
> On 27.01.2025 17:31, Richard Earnshaw (lists) wrote:
>> On 27/01/2025 15:50, Jan Beulich wrote:
>>> --- a/gas/config/tc-arm.c
>>> +++ b/gas/config/tc-arm.c
>>> @@ -1081,7 +1081,7 @@ const char FLT_CHARS[] = "rRsSfFdDxXeEpP
>>>  
>>>  /* Separator character handling.  */
>>>  
>>> -#define skip_whitespace(str)  do { if (*(str) == ' ') ++(str); } while (0)
>>> +#define skip_whitespace(str)  do { if (is_whitespace (*(str))) ++(str); } while (0)
>>>  
>>>  enum fp_16bit_format
>>>  {
>>> @@ -1510,13 +1510,9 @@ parse_neon_type (struct neon_type *type,
>>>  		  return FAIL;
>>>  		}
>>>  	      goto done;
>>> -	    case '0': case '1': case '2': case '3': case '4':
>>> -	    case '5': case '6': case '7': case '8': case '9':
>>> -	    case ' ': case '.':
>>> +	    default:
>>>  	      as_bad (_("unexpected type character `b' -- did you mean `bf'?"));
>>>  	      return FAIL;
>>> -	    default:
>>> -	      break;
>>>  	    }
>>
>> This entire switch statement has now degenerated into 'f' or error.  So I think it would be better to just replace it with an if-else.
> 
> I can do that, but in other projects I'm active we'd deliberately ask
> that switch() be used simply in the expectation that if any further
> character would want checking for, code churn would then be lower. If
> you're fine with the extra churn, I can of course adjust here.
> 
> Jan

Never say never, but I can't see other characters being needed here.  I'll take that hit if it's needed at some point in the future.  Note, we already don't handle this when parsing the numbers after an 'f', so we're not consistent anyway.

I don't particularly like the error recovery here anyway, but that's another story.  Printing "unexpected type character `b' -- did you mean `bf'?" is not exactly informative when there's so little context shown: it's not even as though 'bf' is the complete answer, the type is 'bf16'; but to do this even close to properly we'd need to tokenize the input and print the entire substring up to the next token separator.


R.