Re: [PATCH] Canonicalize conversion operators

[Keith Seitz <keiths@redhat.com>]

On 10/18/2017 04:19 AM, Pedro Alves wrote:
>> @@ -1630,7 +1630,13 @@ oper:	OPERATOR NEW
>>  
>>  			  c_print_type ($2, NULL, &buf, -1, 0,
>>  					&type_print_raw_options);
>> -			  $$ = operator_stoken (buf.c_str ());
>> +
>> +			  /* This also needs canonicalization.  */
>> +			  std::string canon
>> +			    = " " + cp_canonicalize_string (buf.c_str ());
>> +			  if (canon.length () == 1)
>> +			    canon = " " + buf.string ();
>> +			  $$ = operator_stoken (canon.c_str ());
> 
> The length() == 1 check gave me pause.  It's checking that
> cp_canonicalize_string returned empty of course, but it
> wasn't super clear on a quick skim.
> 
> I think you could write it like this, making that part clearer,
> and also saving a few string dups and copies:
> 
> 			  /* This also needs canonicalization.  */
> 			  std::string canon
> 			    = cp_canonicalize_string (buf.c_str ());
> 			  if (canon.empty ())
> 			    canon = std::move (buf.string ());
> 			  $$ = operator_stoken ((" " + canon).c_str ());
> 
> 

Yes, indeed. ISTR having something similar to that at one point, but I don't know why I changed it. [Maybe that was during the C++03/C++11 shake out?]

In any case, committed with that change.

Thank you for the review.

Keith