Template function support

Template function support

[sami wagiaalla]

[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]

I thought I would wright a summery of what I have been up to since this 
work has taken a while now.

The original motivation for this work was to provide support for the 
following senario


template<typename T>
int foo(T){
   return 11;
}

int main (){
   return foo(1);
}

(gdb) print foo(1)

Currently foo would be stored in the symbol table as foo<int>()

My idea was to remove <int> from foo<int>() and store it just as plain 
foo. This would allow for normal overload resolution to take place. The 
patch attached to this message demonstrates that approach.

The problem I ran into is that if the user does this:

(gdb) print foo

then foo<int> is printed

or in the case of a class like:

template<typename T>
class B{};

(gdb) ptype B

would return the first found template *instance*

To fix this problem I plan to take a suggestion made by Tom which is to 
add a meta variable corresponding to a group of instance of the same 
template. This variable will enable the consumer of the lookup (printing 
code, evaluation code, overload resolution code, or breakpoint code) to 
do something sensible when this type of symbol is encountered.

The scope of this has expanded but the good news is that we will solve 
more problems with this extension.

Any comments ?

Sami

Warning: the attached patch might be severely not GNU formatted :)

[-- Attachment #2: search_name.patch --]
[-- Type: text/plain, Size: 15245 bytes --]

commit 46fda01fae49faf17ca481aeb1fea84e9d01012f
Author: Sami Wagiaalla <swagiaal@redhat.com>
Date:   Thu Jun 10 11:55:02 2010 -0400

    Create search_name and fixed template lookup

diff --git a/gdb/cp-name-parser.y b/gdb/cp-name-parser.y
index 6d7b600..c9b5747 100644
--- a/gdb/cp-name-parser.y
+++ b/gdb/cp-name-parser.y
@@ -1966,9 +1966,12 @@ cp_comp_to_string (struct demangle_component *result, int estimated_len)
 			       &err);
 }
 
-/* Convert a demangled name to a demangle_component tree.  On success,
-   the root of the new tree is returned; it is valid until the next
-   call to this function and should not be freed.  On error, NULL is
+/* Convert a demangled name to a demangle_component tree.  The structure
+   of the tree depends on the format of each node in the tree.  For
+   information on the structure of a node see the comment corresponding
+   to its type in demangle_component_type.
+   On success, the root of the new tree is returned; it is valid until the
+   next call to this function and should not be freed.  On error, NULL is
    returned, and an error message will be set in *ERRMSG (which does
    not need to be freed).  */
 
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index da2c8fc..5a431b3 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -428,6 +428,72 @@ cp_remove_params (const char *demangled_name)
   return ret;
 }
 
+/* Returns 1 if the given name contains template parameters;
+   otherwise return 0.
+   Used as a quick heuristic to avoid more expensive template
+   parsing when not necessary.  */
+
+int
+cp_name_has_template_parameters (const char *name)
+{
+  char *c = strchr (name, '<');
+  return c != NULL && strchr (name, '>') > c;
+}
+
+/* Remove template parameters components from the give tree.  */
+
+static struct demangle_component *
+cp_remove_template_params_component (struct demangle_component *comp)
+{
+
+  gdb_assert (comp != NULL);
+
+  switch (comp->type)
+    {
+    case DEMANGLE_COMPONENT_TEMPLATE:
+      /* If there is a template component remove this node by re-parenting the
+         the left child.  */
+      comp = d_left (comp);
+      break;
+    case DEMANGLE_COMPONENT_QUAL_NAME:
+      /* For a qualified name remove template components from the right
+         subtree.  */
+      d_right (comp) = cp_remove_template_params_component (d_right (comp));
+      break;
+    case DEMANGLE_COMPONENT_TYPED_NAME:
+      /* Template components of present will be in the left subtree. Remove
+         them.  */
+      d_left (comp) = cp_remove_template_params_component (d_left (comp));
+      break;
+    default:
+      break;
+    }
+
+  return comp;
+}
+
+/* Remove template parameters from the given name.  The returned string is
+   malloc'ed and must be properly saved and freed.  */
+
+char *
+cp_remove_template_params (const char *name)
+{
+  struct demangle_component *ret_comp;
+  char *ret = NULL;
+
+  if (name == NULL)
+    return NULL;
+
+  ret_comp = cp_demangled_name_to_comp (name, NULL);
+  if (ret_comp == NULL)
+    return NULL;
+
+  ret_comp = cp_remove_template_params_component(ret_comp);
+  ret = cp_comp_to_string (ret_comp, 10);
+
+  return ret;
+}
+
 /* Here are some random pieces of trivia to keep in mind while trying
    to take apart demangled names:
 
@@ -661,7 +727,7 @@ overload_list_add_symbol (struct symbol *sym, const char *oload_name)
       return;
 
   /* Get the demangled name without parameters */
-  sym_name = cp_remove_params (SYMBOL_NATURAL_NAME (sym));
+  sym_name = cp_remove_params (SYMBOL_SEARCH_NAME (sym));
   if (!sym_name)
     return;
 
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index ddc4c93..a2c8f7f 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -107,6 +107,10 @@ extern char *cp_func_name (const char *full_name);
 
 extern char *cp_remove_params (const char *demangled_name);
 
+extern int cp_name_has_template_parameters (const char *name);
+
+extern char *cp_remove_template_params (const char *name);
+
 extern struct symbol **make_symbol_overload_list (const char *,
 						  const char *);
 
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index ca608c5..c79ea88 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -8460,6 +8460,7 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
   if (name)
     {
       const char *linkagename;
+      const char* search_name = NULL;
 
       sym = (struct symbol *) obstack_alloc (&objfile->objfile_obstack,
 					     sizeof (struct symbol));
@@ -8471,6 +8472,26 @@ new_symbol (struct die_info *die, struct type *type, struct dwarf2_cu *cu)
       linkagename = dwarf2_physname (name, die, cu);
       SYMBOL_SET_NAMES (sym, linkagename, strlen (linkagename), 0, objfile);
 
+      /* For C++ if the name contains template parameters remove them, and set
+         the cleaned up name to be the search name.  */
+      if (cu->language == language_cplus && linkagename
+	  && cp_name_has_template_parameters (linkagename))
+	{
+	  char *tmp = cp_remove_template_params (linkagename);
+
+	  if (tmp != NULL && strcmp (tmp, linkagename) != 0)
+	    {
+	      search_name = obsavestring (tmp, strlen (tmp),
+	                                  &objfile->objfile_obstack);
+
+	      symbol_set_cplus_search_name (&sym->ginfo,
+	                                    objfile,
+	                                    search_name);
+	    }
+
+	  xfree (tmp);
+	}
+
       /* Default assumptions.
          Use the passed type or decode it from the die.  */
       SYMBOL_DOMAIN (sym) = VAR_DOMAIN;
diff --git a/gdb/symtab.c b/gdb/symtab.c
index f20baab..038eedc 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -370,6 +370,30 @@ symbol_init_cplus_specific (struct general_symbol_info *gsymbol,
       OBSTACK_ZALLOC (&objfile->objfile_obstack, struct cplus_specific);
 }
 
+/* Set the search name of the give GSYMBOL to name.  */
+void
+symbol_set_cplus_search_name (struct general_symbol_info *gsymbol,
+                                struct objfile *objfile,
+                                const char *name)
+{
+  if (gsymbol->language_specific.cplus_specific == NULL)
+    symbol_init_cplus_specific(gsymbol, objfile);
+
+  gsymbol->language_specific.cplus_specific->search_name = (char *) name;
+}
+
+/* Get the search name of the give GSYMBOL.  */
+
+char*
+symbol_get_cplus_search_name (const struct general_symbol_info *gsymbol)
+{
+  if (gsymbol->language_specific.cplus_specific != NULL
+      && gsymbol->language_specific.cplus_specific->search_name != NULL)
+    return gsymbol->language_specific.cplus_specific->search_name;
+
+  return symbol_natural_name (gsymbol);
+}
+
 \f
 /* Initialize the language dependent portion of a symbol
    depending upon the language for the symbol. */
@@ -656,7 +680,8 @@ symbol_set_names (struct general_symbol_info *gsymbol,
   gsymbol->name = (*slot)->mangled + lookup_len - len;
   if ((*slot)->demangled[0] != '\0')
     {
-      symbol_init_cplus_specific (gsymbol, objfile);
+      if (gsymbol->language_specific.cplus_specific == NULL)
+	symbol_init_cplus_specific (gsymbol, objfile);
       symbol_set_cplus_demangled_name (gsymbol, (*slot)->demangled);
     }
   else
@@ -716,6 +741,8 @@ symbol_search_name (const struct general_symbol_info *gsymbol)
 {
   if (gsymbol->language == language_ada)
     return gsymbol->name;
+  else if (gsymbol->language == language_cplus)
+    return symbol_get_cplus_search_name (gsymbol);
   else
     return symbol_natural_name (gsymbol);
 }
@@ -1049,8 +1076,8 @@ lookup_symbol (const char *name, const struct block *block,
 	       domain_enum domain, int *is_a_field_of_this)
 {
   return lookup_symbol_in_language (name, block, domain,
-				    current_language->la_language,
-				    is_a_field_of_this);
+                                    current_language->la_language,
+                                    is_a_field_of_this);
 }
 
 /* Behave like lookup_symbol except that NAME is the natural name
@@ -1616,6 +1643,18 @@ find_main_filename (void)
   return (NULL);
 }
 
+/* Return 1 if NAME matches SYM's template name.  */
+
+static int
+symbol_matches_template_name (const char *name, struct symbol *sym)
+{
+  const char *template_name = SYMBOL_NATURAL_NAME(sym);
+  if (template_name == NULL)
+    return 0;
+
+  return strcmp_iw (template_name, name) == 0;
+}
+
 /* Search BLOCK for symbol NAME in DOMAIN.
 
    Note that if NAME is the demangled form of a C++ symbol, we will fail
@@ -1637,12 +1676,28 @@ lookup_block_symbol (const struct block *block, const char *name,
 
   if (!BLOCK_FUNCTION (block))
     {
+    
+    const char* template_name = NULL;
+
+    if (cp_name_has_template_parameters (name))
+      {
+        template_name = name;
+        name = cp_remove_template_params (name);
+      }
+
       for (sym = dict_iter_name_first (BLOCK_DICT (block), name, &iter);
 	   sym != NULL;
 	   sym = dict_iter_name_next (name, &iter))
 	{
+	  /* For C++ if the name being searched for contains template
+	     parameters check the template name of the symbol.  */
+	  if (template_name != NULL 
+	      && !symbol_matches_template_name (template_name, sym))
+	    continue;
+
 	  if (symbol_matches_domain (SYMBOL_LANGUAGE (sym),
-				     SYMBOL_DOMAIN (sym), domain))
+	                             SYMBOL_DOMAIN (sym), domain))
+	 
 	    return sym;
 	}
       return NULL;
diff --git a/gdb/symtab.h b/gdb/symtab.h
index 8ae799b..ff04b2e 100644
--- a/gdb/symtab.h
+++ b/gdb/symtab.h
@@ -81,6 +81,11 @@ struct cplus_specific
 {
   /* This is in fact used for C++, Java, and Objective C.  */
   char *demangled_name;
+
+  /* If the symbol name contains template parameters this is the name with
+     these parameters removed to be used during search.  Otherwise this
+     should be NULL, and the regular symbol name should be used.  */
+  char *search_name;
 };
 
 /* Define a structure for the information that is common to all symbol types,
@@ -177,6 +182,15 @@ extern CORE_ADDR symbol_overlayed_address (CORE_ADDR, struct obj_section *);
 extern char *
 symbol_get_cplus_demangled_name (const struct general_symbol_info *gsymbol);
 
+/* Set the template name of the give GSYMBOL to name.  */
+extern void
+symbol_set_cplus_search_name (struct general_symbol_info *gsymbol,
+                              struct objfile *objfile,
+                              const char *name);
+
+extern char*
+symbol_get_cplus_search_name (const struct general_symbol_info *gsymbol);
+
 /* Initializes the language dependent portion of a symbol
    depending upon the language for the symbol. */
 #define SYMBOL_INIT_LANGUAGE_SPECIFIC(symbol,language) \
@@ -265,6 +279,7 @@ extern char *symbol_demangled_name (const struct general_symbol_info *symbol);
    (symbol_search_name (&(symbol)->ginfo))
 extern char *symbol_search_name (const struct general_symbol_info *);
 
+
 /* Analogous to SYMBOL_MATCHES_NATURAL_NAME, but uses the search
    name.  */
 #define SYMBOL_MATCHES_SEARCH_NAME(symbol, name)			\
diff --git a/gdb/testsuite/gdb.cp/temp-op.cc b/gdb/testsuite/gdb.cp/temp-op.cc
new file mode 100644
index 0000000..6839f98
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/temp-op.cc
@@ -0,0 +1,105 @@
+class A
+{
+};
+
+template<typename T>
+int foo (T, char)
+{
+  T t;
+  return 11;
+}
+
+template<typename T, typename T2>
+int foo2 (T, T2, char)
+{
+  T t;
+  return 11;
+}
+
+namespace C
+{
+  namespace D {
+    template<typename T, typename T2>
+    int foo3 (T, T2, char)
+    {
+      T t;
+      return 11;
+    }
+  }
+}
+int operator<< (A, int)
+{
+  return 12;
+}
+
+int operator< (A, int)
+{
+  return 13;
+}
+
+int operator<= (A, int)
+{
+  return 14;
+}
+
+template<typename T>
+int operator==(T, char)
+{
+  return 15;
+}
+
+int operator==(A, int)
+{
+  return 16;
+}
+
+template<typename T>
+class B{
+  T t;
+public:
+  int operator==(int)
+    {
+      return 17;
+    }
+};
+
+int operator== (B<int>, char){
+  return 18;
+}
+
+template <class T, int>
+class Outer{
+ public:
+  template <class T2, int>
+    class Inner{
+  public:
+    template <int>
+      class ReallyInner{};
+  };
+};
+
+
+int main ()
+{
+  A a;
+  
+  foo (a, 'a');
+  foo (a, 1);
+  foo2 (a, a, 'a');
+  C::D::foo3 (a, a, 'a');
+
+  a << 22;
+  a <  22;
+  a <= 22;
+
+  a == 'a';
+  a == 1;
+
+  B<int> b;
+  b == 1;
+  b == 'a';
+
+  Outer<int, 23>::Inner<long, 27>::ReallyInner<5> oir;
+  
+  return 0;
+}
\ No newline at end of file
diff --git a/gdb/testsuite/gdb.cp/temp-op.exp b/gdb/testsuite/gdb.cp/temp-op.exp
new file mode 100644
index 0000000..db76cd5
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/temp-op.exp
@@ -0,0 +1,71 @@
+# Copyright 2008 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile temp-op
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+
+# Get things started.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+############################################
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+# Test that a templated function can be found
+# with out specification of template arguments
+gdb_test "p foo(a, 'a')"         "= 11"
+
+# Test that function names with '<' in their names
+# are not mistaken for templates
+gdb_test "p a << 22"         "= 12"
+gdb_test "p a <  22"         "= 13"
+gdb_test "p a <= 22"         "= 14"
+
+# Test that a template operator can be correctly
+# evaluated
+gdb_test "p a == 'a'"        "= 15"
+
+# Test that overload resolution is still correctly
+# performed.
+gdb_test "p a == 1"          "= 16"
+
+# Test calling a member function of a template class
+gdb_test "p b == 1"          "= 17"
+gdb_test "p b == 'a'"        "= 18"
+
+# Test that printing the a template name without
+# template parameters does not return an arbitrary match
+
+gdb_test "p foo" "No symbol \"foo\" in current context"
+gdb_test "ptype B" "No symbol \"foo\" in current context"
diff --git a/gdb/valops.c b/gdb/valops.c
index 6648bdb..5e30263 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2429,7 +2429,7 @@ find_overload_match (struct type **arg_types, int nargs,
 
       if (fsym)
         {
-          qualified_name = SYMBOL_NATURAL_NAME (fsym);
+          qualified_name = SYMBOL_SEARCH_NAME (fsym);
 
           /* If we have a function with a C++ name, try to extract just
 	     the function part.  Do not try this for non-functions (e.g.

Re: Template function support

[Keith Seitz]

On 06/10/2010 11:43 AM, sami wagiaalla wrote:
> My idea was to remove <int> from foo<int>() and store it just as plain
> foo. This would allow for normal overload resolution to take place. The
> patch attached to this message demonstrates that approach.

Just one quick question: Did you consider the opposite approach, i.e., 
adding "<int>" (since we can deduce that from the argument type)?

Keith

Re: Template function support

[Tom Tromey]

>>>>> "Keith" == Keith Seitz <keiths@redhat.com> writes:

>> My idea was to remove <int> from foo<int>() and store it just as plain
>> foo. This would allow for normal overload resolution to take place. The
>> patch attached to this message demonstrates that approach.

Keith> Just one quick question: Did you consider the opposite approach, i.e.,
Keith> adding "<int>" (since we can deduce that from the argument type)?

You mean adding that at lookup time?

I think that is hard because the lookup is a complicated process, not
just adding the argument type names.

Tom

Re: Template function support

[Keith Seitz]

On 06/10/2010 12:11 PM, Tom Tromey wrote:
>>>>>> "Keith" == Keith Seitz<keiths@redhat.com>  writes:
> Keith>  Just one quick question: Did you consider the opposite approach, i.e.,
> Keith>  adding "<int>" (since we can deduce that from the argument type)?
>
> You mean adding that at lookup time?

Yeah... I forgot about the complications arising from implicit type 
conversions and other argument complications (templates?).

I wonder if having DW_TAG_template_*_parameter and a better symbol table 
layout would facilitate things like this? [I know a better symbol table 
layout would help in many other areas involving lookups.]

Keith

Re: Template function support

[Tom Tromey]

Keith> I wonder if having DW_TAG_template_*_parameter and a better symbol
Keith> table layout would facilitate things like this? [I know a better
Keith> symbol table layout would help in many other areas involving lookups.]

I think there are 2 issues.

First, we need some way to get from the unadorned name to the various
instantiations.

Second, once we have an instantiation, it would be handy if the
parameters were represented as symbols attached to the instantiation's
symbol.  I plan to do this once Sami's patch to deal with the
c++-specific stuff in general_symbol_info goes in.

In the long run maybe we want to move to a multi-level symbol table.
This isn't totally clear to me -- it would be more conventional, but I
am having trouble at the moment thinking of ways that our current
approach prevents us from doing what we want.

Tom

Re: Template function support

[sami wagiaalla]

> In the long run maybe we want to move to a multi-level symbol table.
> This isn't totally clear to me -- it would be more conventional, but I
> am having trouble at the moment thinking of ways that our current
> approach prevents us from doing what we want.
>

Ditto

Re: Template function support

[sami wagiaalla]

On 06/10/2010 03:00 PM, Keith Seitz wrote:
> On 06/10/2010 11:43 AM, sami wagiaalla wrote:
>> My idea was to remove <int> from foo<int>() and store it just as plain
>> foo. This would allow for normal overload resolution to take place. The
>> patch attached to this message demonstrates that approach.
>
> Just one quick question: Did you consider the opposite approach, i.e.,
> adding "<int>" (since we can deduce that from the argument type)?
>

No, it did not even occur to me. The problem is that we don't know if 
the sought symbol is a template instance, and in the case of overload 
resolution the candidates can be both template instances and regular 
functions.

Re: Template function support

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> I thought I would wright a summery of what I have been up to since
Sami> this work has taken a while now.

Thanks!

Sami> To fix this problem I plan to take a suggestion made by Tom which is
Sami> to add a meta variable corresponding to a group of instance of the
Sami> same template. This variable will enable the consumer of the lookup
Sami> (printing code, evaluation code, overload resolution code, or
Sami> breakpoint code) to do something sensible when this type of symbol is
Sami> encountered.

I thought perhaps you'd have a new LOC_TEMPLATE, then add a new symbol
with that class.  Then when you do a lookup for "foo" you can tell that
it is a "phony" symbol that represents a template.  Finally, the
symbol's value union could have a new field that points to all the
instantiations.

Alternatively such a symbol could just store a type that has a new
TYPE_CODE_.  I'm not sure whether this would be any better.

I'm not sure that this is sane and/or easily implemented.

Sami> +  return c != NULL && strchr (name, '>') > c;

I don't think you really need that second clause there.

I didn't look too closely at all of it.  Make sure that psymbol changes
don't poison the bcache, and that lookups that find a name like this
don't do the wrong thing elsewhere in gdb.

Tom

Re: Template function support

[sami wagiaalla]

> I thought perhaps you'd have a new LOC_TEMPLATE, then add a new symbol
> with that class.  Then when you do a lookup for "foo" you can tell that
> it is a "phony" symbol that represents a template.  Finally, the
> symbol's value union could have a new field that points to all the
> instantiations.
>
> Alternatively such a symbol could just store a type that has a new
> TYPE_CODE_.  I'm not sure whether this would be any better.
>
> I'm not sure that this is sane and/or easily implemented.
>

Thanks for the pointers! I don't know which way I'll go. The TYPE_CODE 
approach was the only thing I came up with, but I will look into both.

> Sami>  +  return c != NULL&&  strchr (name, '>')>  c;
>
> I don't think you really need that second clause there.
>

This is to avoid matching things like operator< operator<=, and 
operator<<. Come to think of it the second clause should be

strchr (c, '>') != NULL

> I didn't look too closely at all of it.  Make sure that psymbol changes
> don't poison the bcache, and that lookups that find a name like this
> don't do the wrong thing elsewhere in gdb.
>

Will do. I am not sure I understand this mechanism well, but I'll figure 
it out.


Thanks,
   Sami

Re: Template function support

[Tom Tromey]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> This is to avoid matching things like operator< operator<=, and
Sami> operator<<. Come to think of it the second clause should be

Sami> strchr (c, '>') != NULL

Actually I think strchr (c + 1, ...)

Tom