Re: [PATCH, doc RFA] Add "skip regexp"

[Pedro Alves <palves@redhat.com>]

On 02/02/2016 01:03 AM, Doug Evans wrote:
> Hi.
> 
> The "skip" command is great, but it can be really cumbersome to use with  
> c++.
> 
> E.g., consider stepping into functions that take std::string arguments.
> 
>    foo (bar ("abc"));
> 
> where bar takes a std::string argument.
> 
> There are four functions you generally always want to skip in this case
> (cut-n-pasted from my gdb session):
> 
> std::allocator<char>::allocator (this=0x7fffffffe6bf)
> std::basic_string<char, std::char_traits<char>, std::allocator<char>  
>> ::basic_string (this=0x7fffffffe6b0, __s=0x400ad1 "abc", __a=...)
> std::basic_string<char, std::char_traits<char>, std::allocator<char>  
>> ::~basic_string (this=0x7fffffffe6b0, __in_chrg=<optimized out>)
> std::allocator<char>::~allocator (this=0x7fffffffe6bf, __in_chrg=<optimized  
> out>)
> 
> With this patch one can specify the skip as:
> 
> skip regexp ^std::(allocator|basic_string)<.*>::~?\1 *\(
> 
> Skipping every templated class constructor/destructor in std
> could be specified as:
> 
> skip regexp ^std::([a-zA-z0-9_]+)<.*>::~?\1 *\(

I think this is great.  Thanks for tackling this.  This was
originally discussed in the context of the original
implementation, but was left out:

 https://sourceware.org/bugzilla/show_bug.cgi?id=8287

I wonder about the UI though.  The command name doesn't make it
clear, so I was wondered whether "skip regexp" also matches the
filename the function is implemented in.  I figured out it doesn't
from the documentation, but it got to me consider how we'd extend
the UI if/when we want to skip files with a regexp too, as
then "skip regexp" becomes ambiguous.  Maybe "skip rfunction" instead,
and then we'd have "skip rfile" ?


Also, ot asking you to implement this, just thinking out loud, but
I also wondered if it wouldn't have made more sense to be able
to mix file and function names, using command options, rather than
have distinct skip types.  Like:

 skip -file foo.c -function foo
 skip -rfile libfoo/*.c -rfunction ^foo_.*

I guess we could still have that and leave "skip function", etc.
for compatibility.

> 
> +skip regexp regular-expression
> +  A variation of "skip function" where the function name is specified
> +  as a regular expression.
> +



> +If you wanted to skip every C++ constructor and destructor in the  
> @code{std}
> +namespace you could do:

Using past tense reads a bit odd to me.  I'd suggest:

 If you want to skip every C++ constructor and destructor in the
 @code{std} namespace you can do:

> 
> +/* Return the name of the skiplist entry kind.  */
> +
> +static const char *
> +skiplist_entry_kind_name (struct skiplist_entry *e)
> +{
> +  switch (e->kind)
> +    {
> +    case SKIP_FILE: return "file";
> +    case SKIP_FUNCTION: return "function";
> +    case SKIP_REGEXP: return "regexp";
> +    default:
> +      gdb_assert_not_reached ("bad skiplist_entry kind");
> +    }

Line breaks before return.

> +}
> +
> +/* Create a SKIP_FILE object. */
> +
> +static struct skiplist_entry *
> +make_skip_file (const char *name)
> +{
> +  struct skiplist_entry *e = XCNEW (struct skiplist_entry);
> +
> +  e->kind = SKIP_FILE;
> +  e->text = xstrdup (name);
> +  e->enabled = 1;
> +
> +  return e;
> +}
> +
> +/* Create a SKIP_FUNCTION object. */
> +
> +static struct skiplist_entry *
> +make_skip_function (const char *name)
> +{
> +  struct skiplist_entry *e = XCNEW (struct skiplist_entry);
> +
> +  e->kind = SKIP_FUNCTION;
> +  e->enabled = 1;
> +  e->text = xstrdup (name);
> +
> +  return e;
> +}
> +
> +/* Create a SKIP_REGEXP object.
> +   The regexp is not parsed, the caller must do that afterwards.  */
> +
> +static struct skiplist_entry *
> +make_skip_regexp (const char *regexp)
> +{
> +  struct skiplist_entry *e = XCNEW (struct skiplist_entry);
> +
> +  e->kind = SKIP_REGEXP;
> +  e->enabled = 1;
> +  e->text = xstrdup (regexp);
> +
> +  return e;
> +}
> +

Seems like all these three functions could be replaced by
a single:

static struct skiplist_entry *
make_skiplist_entry (enum skip_kind kind, const char *text)
{
   struct skiplist_entry *e = XCNEW (struct skiplist_entry);

  e->kind = kind;
  e->text = xstrdup (text);
  e->enabled = 1;

  return e;
}

Thanks,
Pedro Alves