Re: [PATCH v2 03/11] gdb: factor out regexp compilation from global_symbol_searcher::search

[Simon Marchi <simon.marchi@polymtl.ca>]

>> +static std::optional<compiled_regex>
>> +compile_symbol_search_regex (const char *regexp)
>>  {
>> -  std::optional<compiled_regex> name_regex;
>> -  std::optional<compiled_regex> type_regex;
>> +  if (regexp == nullptr)
>> +    return {};
>>  
>> -  if (m_symbol_name_regexp != NULL)
>> -    {
>> -      const char *symbol_name_regexp = m_symbol_name_regexp;
>> -      std::string symbol_name_regexp_holder;
>> +  int cflags
>> +    = REG_NOSUB | (case_sensitivity == case_sensitive_off ? REG_ICASE : 0);
>>  
>> -      /* Make sure spacing is right for C++ operators.
>> -	 This is just a courtesy to make the matching less sensitive
>> -	 to how many spaces the user leaves between 'operator'
>> -	 and <TYPENAME> or <OPERATOR>.  */
>> -      const char *op_end;
>> -      const char *opname = operator_chars (symbol_name_regexp, &op_end);
>> +  return std::make_optional<compiled_regex> (regexp, cflags,
>> +					    _("Invalid regexp"));
> 
> I think the indentation of the "Invalid regexp" line is off.

Ah yeah it's because I switched to make_optional at the last minute (it
avoids using std::in_place.

>> -  if (m_symbol_type_regexp != NULL)
>> +  if (*opname)
>>      {
>> -      int cflags = REG_NOSUB | (case_sensitivity == case_sensitive_off
>> -				? REG_ICASE : 0);
>> -      type_regex.emplace (m_symbol_type_regexp, cflags,
>> -			  _("Invalid regexp"));
>> +      /* -1 means ok; otherwise number of spaces needed.  */
>> +      int fix = -1;
>> +
>> +      if (c_isalpha (*opname) || *opname == '_' || *opname == '$')
>> +	{
>> +	  /* There should 1 space between 'operator' and 'TYPENAME'.  */
> 
> This comment has a (pre-existing) missing 'be':
> 
>   /* There should BE 1 space between .... */
> 
>> +	  if (opname[-1] != ' ' || opname[-2] == ' ')
>> +	    fix = 1;
>> +	}
>> +      else
>> +	{
>> +	  /* There should 0 spaces between 'operator' and 'OPERATOR'.  */
> 
> Same missing 'BE' here.

Fixed those 2.

> 
> With those all fixed:
> 
> Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,

Simon