Re: [PATCH v3] gdb/symtab.c Fix completion of template class members

[Keith Seitz <keiths@redhat.com>]

Hi,

On 3/3/26 2:36 AM, Daniel Knezevic wrote:
> This is v3 of:
> https://inbox.sourceware.org/gdb-patches/20251113095559.404539-1-daniel.knezevic@htecgroup.com/raw
> 
> Do not break the symbol if it contains '<' or '>' characters.
> 
> This patch fixes formatting of the code and adds more tests for testing
> completion of nested template classes.

Thank you for the quick update. I just have a few trivial nits. There is
no need to submit a v4 solely for these.

> diff --git a/gdb/testsuite/gdb.cp/cpcompletion.cc b/gdb/testsuite/gdb.cp/cpcompletion.cc
> index b85f168928c..defbd75ad34 100644
> --- a/gdb/testsuite/gdb.cp/cpcompletion.cc
> +++ b/gdb/testsuite/gdb.cp/cpcompletion.cc
> @@ -74,6 +74,28 @@ struct baz
>     S s;
>   };
>   
> +/* The Outer class is used to test completion of nested template classes*/

Please end the comment with a period and two spaces. [That follows our
coding style guidelines.]

> +template <typename T>
> +struct Outer
> +{
> +  T value;
> +
> +  template <typename U>
> +  struct Inner
> +  {
> +    U inner_value;
> +    T outer_ref;
> +  };
> +
> +  Inner<int> get_int_inner () const
 > +  {> +    Inner<int> i;
> +    i.inner_value = 0;
> +    i.outer_ref = value;
> +    return i;
> +  }
> +};
> +
>   int main ()
>   {
>     baz<int> obj (2.3, 0.1);
> @@ -84,5 +106,9 @@ int main ()
>     Foo foo1;
>     foo1.set_foo (42);		// Set breakpoint here.
>     a.get();			// Prevent compiler from throwing 'a' away.
> +  Outer<double> outer;
> +  outer.value = 4.2;
> +  auto inner = outer.get_int_inner();

Need a space between function/method name and parentheses.

> +  Outer<double>::Inner<long> long_inner;
>     return 0;
>   }
> diff --git a/gdb/testsuite/gdb.cp/cpcompletion.exp b/gdb/testsuite/gdb.cp/cpcompletion.exp
> index 1fb9a6bf01d..9ae17026f7d 100644
> --- a/gdb/testsuite/gdb.cp/cpcompletion.exp
> +++ b/gdb/testsuite/gdb.cp/cpcompletion.exp
> @@ -137,3 +137,39 @@ with_test_prefix "expression with namespace" {
>   }
>   
>   test_gdb_complete_unique "break baz(int" "break baz(int, double)"
> +
> +# Test completion with a template class.
> +test_gdb_complete_unique "p baz<int>::" "p baz<int>::baz<double>(int, double)"
> +
> +# Test completion of nested template classes
> +with_test_prefix "nested template classes" {
> +    # Unlike in linespecs, tab- and complete-command completion work a
> +    # bit differently when completing around the scope operator.  The
> +    # matches in the tab-completion case only show the part of the
> +    # symbol after the scope, since ':' is a word break character.
> +
> +    set tab_completion_list {
> +	"Inner<int>"
> +	"Inner<long>"
> +	"get_int_inner() const"
> +    }
> +    test_gdb_complete_tab_multiple "ptype Outer<double>:" ":" $tab_completion_list
> +    test_gdb_complete_tab_multiple "ptype Outer<double>::" "" $tab_completion_list
> +
> +    # OTOH, the complete command must show the whole command, with
> +    # qualified symbol displayed as entered by the user.
> +    set cmd_completion_list {
> +	"Outer<double>::Inner<int>"
> +	"Outer<double>::Inner<long>"
> +	"Outer<double>::get_int_inner() const"
> +    }
> +    test_gdb_complete_cmd_multiple "ptype " "Outer<double>:" $cmd_completion_list
> +    test_gdb_complete_cmd_multiple "ptype " "Outer<double>::" $cmd_completion_list
> +    test_gdb_complete_cmd_multiple "ptype " "Outer<double>::Inn" {
> +	"Outer<double>::Inner<int>"
> +	"Outer<double>::Inner<long>"
> +    }
> +
> +    # Add a disambiguating character and we get a unique completion.
> +    test_gdb_complete_unique "ptype Outer<double>::g" "ptype Outer<double>::get_int_inner() const"

This last line looks a bit long. Suggest adding '\' between the
two arguments, moving the second argument to the next line.

> +}

Otherwise, this LGTM.

Reviewed-By: Keith Seitz <keiths@redhat.com>

Thank you!
Keith