Re: [PATCH v2 01/11] gdb: add symbol_search_kind enum for global_symbol_searcher

[Andrew Burgess <aburgess@redhat.com>]

I think this is looking good.  Just one minor comment inline below:

simon.marchi@polymtl.ca writes:

> @@ -5166,20 +5179,18 @@ symtab_symbol_info (bool quiet, bool exclude_minsyms,
>        const char *classname;
>        switch (kind)
>  	{
> -	case VAR_DOMAIN:
> +	case symbol_search_kind::VARIABLE:
>  	  classname = "variable";
>  	  break;
> -	case FUNCTION_DOMAIN:
> +	case symbol_search_kind::FUNCTION:
>  	  classname = "function";
>  	  break;
> -	case TYPE_DOMAIN:
> +	case symbol_search_kind::TYPE:
>  	  classname = "type";
>  	  break;
> -	case MODULE_DOMAIN:
> +	case symbol_search_kind::MODULE:
>  	  classname = "module";
>  	  break;
> -	default:
> -	  gdb_assert_not_reached ("invalid domain enum");
>  	}

I initially wrote: please don't drop the default case, but then I
checked the V1 review and saw this change was made specifically to
address the feedback there.

What I don't understand is you said:

> > I think we could drop the "default" case as part of this refactoring.  All
> > cases are handled; we lose compile time warnings for unhandled cases with
> > the "default" case.
> 
> Agreed that we can drop the default.  Note that we don't build with
> -Wswitch, but we specifically enable it locally here and there.

If the warning is mostly off, including here, then deleting the default
seems like the wrong choice.

I'm not too fussed, I think there's a middle group, that would keep my
happy:

 1. initialise classname to nullptr, and

 2. follow this switch statement with:
      gdb_assert (classname != nullptr);

Even with the warning on it doesn't protect us from random garbage being
cast to type symbol_search_kind and then passed to this function, which
the 'default:' case does protect against.  But the trailing assert is
probably better anyway as it ensures each switch case is doing what
needs to be done.

With that:

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew