[RFA] handling of 'operator' in cp_find_first_component

[RFA] handling of 'operator' in cp_find_first_component

[David Carlton]

The function cp_find_first_component assumes that the string
'operator' as part of an operator name can only occur at the start of
a component.  Unfortunately, this isn't true: I've recently run into
situations where there's a templated function whose name demangles to
something like

  int operator<< <int>(char)

In particular, the return type is part of the demangled name (I'm not
entirely sure why, but that's a separate issue), so 'operator' occurs
after a space.

This patch modifies cp_find_first_component to accept that; it looks
for the string 'operator' either at the beginning of a name or after a
few other characters.  I've also added some test cases for this.

Tested on GCC 3.2, DWARF 2, i686-pc-linux-gnu; no new regressions.  OK
to commit?

David Carlton
carlton@bactrian.org

2003-04-18  David Carlton  <carlton@bactrian.org>

	* cp-support.c (cp_find_first_component): Accept 'operator' in
	more locations.

2003-04-18  David Carlton  <carlton@bactrian.org>

	* gdb.c++/maint.exp (test_first_component): Add tests for
	'operator' in more locations.

Index: cp-support.c
===================================================================
RCS file: /cvs/src/src/gdb/cp-support.c,v
retrieving revision 1.2
diff -u -p -r1.2 cp-support.c
--- cp-support.c	15 Apr 2003 23:07:11 -0000	1.2
+++ cp-support.c	18 Apr 2003 19:08:55 -0000
@@ -57,16 +57,7 @@ static void first_component_command (cha
      'foo' in an anonymous namespace gets demangled as "(anonymous
      namespace)::foo".
 
-   - And operator names can contain parentheses or angle brackets.
-     Fortunately, I _think_ that operator names can only occur in a
-     fairly restrictive set of locations (in particular, they have be
-     at depth 0, don't they?).  */
-
-/* NOTE: carlton/2003-02-21: Daniel Jacobowitz came up with an example
-   where operator names don't occur at depth 0.  Sigh.  (It involved a
-   template argument that was a pointer: I hadn't realized that was
-   possible.)  Handling such edge cases does not seem like a
-   high-priority problem to me.  */
+   - And operator names can contain parentheses or angle brackets.  */
 
 /* FIXME: carlton/2003-03-13: We have several functions here with
    overlapping functionality; can we combine them?  Also, do they
@@ -209,40 +200,14 @@ method_name_from_physname (const char *p
 unsigned int
 cp_find_first_component (const char *name)
 {
-  /* Names like 'operator<<' screw up the recursion, so let's
-     special-case them.  I _hope_ they can only occur at the start of
-     a component.  */
-
   unsigned int index = 0;
-
-  if (strncmp (name, "operator", LENGTH_OF_OPERATOR) == 0)
-    {
-      index += LENGTH_OF_OPERATOR;
-      while (isspace(name[index]))
-	++index;
-      switch (name[index])
-	{
-	case '<':
-	  if (name[index + 1] == '<')
-	    index += 2;
-	  else
-	    index += 1;
-	  break;
-	case '>':
-	case '-':
-	  if (name[index + 1] == '>')
-	    index += 2;
-	  else
-	    index += 1;
-	  break;
-	case '(':
-	  index += 2;
-	  break;
-	default:
-	  index += 1;
-	  break;
-	}
-    }
+  /* Operator names can show up in unexpected places.  Since these can
+     contain parentheses or angle brackets, they can screw up the
+     recursion.  But not every string 'operator' is part of an
+     operater name: e.g. you could have a variable 'cooperator'.  So
+     this variable tells us whether or not we should treat the string
+     'operator' as starting an operator.  */
+  int operator_possible = 1;
 
   for (;; ++index)
     {
@@ -261,6 +226,7 @@ cp_find_first_component (const char *nam
 	      gdb_assert (name[index] == ':');
 	      index += 2;
 	    }
+	  operator_possible = 1;
 	  break;
 	case '(':
 	  /* Similar comment as to '<'.  */
@@ -272,13 +238,63 @@ cp_find_first_component (const char *nam
 	      gdb_assert (name[index] == ':');
 	      index += 2;
 	    }
+	  operator_possible = 1;
 	  break;
 	case '>':
 	case ')':
 	case '\0':
 	case ':':
 	  return index;
+	case 'o':
+	  /* Operator names can screw up the recursion.  */
+	  if (operator_possible
+	      && strncmp (name + index, "operator", LENGTH_OF_OPERATOR) == 0)
+	    {
+	      index += LENGTH_OF_OPERATOR;
+	      while (isspace(name[index]))
+		++index;
+	      switch (name[index])
+		{
+		  /* Skip over one less than the appropriate number of
+		     characters: the for loop will skip over the last
+		     one.  */
+		case '<':
+		  if (name[index + 1] == '<')
+		    index += 1;
+		  else
+		    index += 0;
+		  break;
+		case '>':
+		case '-':
+		  if (name[index + 1] == '>')
+		    index += 1;
+		  else
+		    index += 0;
+		  break;
+		case '(':
+		  index += 1;
+		  break;
+		default:
+		  index += 0;
+		  break;
+		}
+	    }
+	  operator_possible = 0;
+	  break;
+	case ' ':
+	case ',':
+	case '.':
+	case '&':
+	case '*':
+	  /* NOTE: carlton/2003-04-18: I'm not sure what the precise
+	     set of relevant characters are here: it's necessary to
+	     include any character that can show up before 'operator'
+	     in a demangled name, and it's safe to include any
+	     character that can't be part of an identifier's name.  */
+	  operator_possible = 1;
+	  break;
 	default:
+	  operator_possible = 0;
 	  break;
 	}
     }
Index: maint.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/maint.exp,v
retrieving revision 1.1
diff -u -p -r1.1 maint.exp
--- maint.exp	15 Apr 2003 23:07:11 -0000	1.1
+++ maint.exp	18 Apr 2003 19:10:32 -0000
@@ -63,6 +63,11 @@ proc test_first_component {} {
     test_single_component "foo(std::basic_streambuf<wchar_t,std::char_traits<wchar_t> >)"
     test_single_component "operator>(X::Y)"
 
+    # Operator names can show up in weird places.
+
+    test_single_component "int operator<< <char>()"
+    test_single_component "T<Cooperator>"
+
     gdb_test "maint cp first_component foo::bar" "foo"
     gdb_test "maint cp first_component foo::bar::baz" "foo"
     gdb_test "maint cp first_component C<A>::bar" "C<A>"

Re: [RFA] handling of 'operator' in cp_find_first_component

[Daniel Berlin]

On Friday, April 18, 2003, at 03:17  PM, David Carlton wrote:

> The function cp_find_first_component assumes that the string
> 'operator' as part of an operator name can only occur at the start of
> a component.  Unfortunately, this isn't true: I've recently run into
> situations where there's a templated function whose name demangles to
> something like
>
>   int operator<< <int>(char)
>
> In particular, the return type is part of the demangled name (I'm not
> entirely sure why, but that's a separate issue),

Templated functions always have the return type in the mangled (and 
thus, demangled) names.

We've actually been through this before, they are supposed to be there.
see http://sources.redhat.com/ml/gdb/2001-06/msg00227.html
(I quote the relevant part of the ABI standard)

Re: [RFA] handling of 'operator' in cp_find_first_component

[Daniel Jacobowitz]

On Fri, Apr 18, 2003 at 12:17:54PM -0700, David Carlton wrote:
> The function cp_find_first_component assumes that the string
> 'operator' as part of an operator name can only occur at the start of
> a component.  Unfortunately, this isn't true: I've recently run into
> situations where there's a templated function whose name demangles to
> something like
> 
>   int operator<< <int>(char)
> 
> In particular, the return type is part of the demangled name (I'm not
> entirely sure why, but that's a separate issue), so 'operator' occurs
> after a space.
> 
> This patch modifies cp_find_first_component to accept that; it looks
> for the string 'operator' either at the beginning of a name or after a
> few other characters.  I've also added some test cases for this.
> 
> Tested on GCC 3.2, DWARF 2, i686-pc-linux-gnu; no new regressions.  OK
> to commit?

First of all, think with me for a moment here.

As Daniel Berlin pointed out, the demangler is behaving correctly.  The
return type is part of the mangled signature in this case, and must be.
However, for GDB's purposes, is it _necessary_ or _useful_ to have said
mangled name?  If not, for another project I already have a
cplus_demangle wrapper which plays with the resulting names.  Adapting
it to detect and remove return types would probably be easy.

I am not strong in this area of C++, but as far as I know this should
suffice; the demangled name of such a function will always include the
template parameters; and the template-id must be unique in the program
(right?  Not sure if the ODR comes into play here or not) so either
there is only one possible return value or the template parameters
uniquely identify the return value.

i.e. you can't have a program with
  int foo<int> (int)
and
  long foo<int> (int)
in it!


That said, I still think your patch is OK; on the condition that it
handles the test case the comment you're removing refers to.  Something
like:
  int foo<operator() (B&)>
or whatever it was in the right syntax.  Does it, and can you add that
to the maint.exp tests?

> 
> David Carlton
> carlton@bactrian.org
> 
> 2003-04-18  David Carlton  <carlton@bactrian.org>
> 
> 	* cp-support.c (cp_find_first_component): Accept 'operator' in
> 	more locations.
> 
> 2003-04-18  David Carlton  <carlton@bactrian.org>
> 
> 	* gdb.c++/maint.exp (test_first_component): Add tests for
> 	'operator' in more locations.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] handling of 'operator' in cp_find_first_component

[Daniel Berlin]

On Monday, April 21, 2003, at 11:21  PM, Daniel Jacobowitz wrote:

>
> i.e. you can't have a program with
>   int foo<int> (int)
> and
>   long foo<int> (int)
> in it!


Stop right here.

This is not quite right, since the second example is invalid.
You can, however,  have (geez i haven't written C++ templates in a 
while. it took me a few compiles to get this right):

template <class blah> blah foo (int a)
{
         return a;
}
int main(void)
{
         int q = foo<int>(5);
         long r = foo<long>(5);
}

Note the only real difference here is in return type.
It also definitely generates two functions:
root@dberlin root]# nm -C a.o
          U __gxx_personality_v0
00000000 T main
00000000 W int foo<int>(int)
00000000 W long foo<long>(int)
[root@dberlin root]#

There is no way to write exactly what you want in valid C++ (IIRC), so 
i'm not sure if this program demonstrates what you think can't be done 
or not.

To answer whether you need the return type, let's add two 
specializations here and make it worse:
template <> long foo (int a)
{
	return 9;
}
template <> int foo (int a)
{
	return 10;
}

Now, without the return type in the template, you would *never* know 
which function to call from a debugger if someone does p foo(5) 
(However, i'll note the only case you will know is when someone does 
something like p a=foo(5), where "a" is a program variable of type int 
or long).
With the return type,  you could at least present a useful list to the 
user and ask.
Unless you want the list to have to re-demangle names so it can get the 
return type out again, or something ugly like that.

--Dan

Re: [RFA] handling of 'operator' in cp_find_first_component

[Daniel Jacobowitz]

On Tue, Apr 22, 2003 at 10:33:05AM -0400, Daniel Berlin wrote:
> 
> On Monday, April 21, 2003, at 11:21  PM, Daniel Jacobowitz wrote:
> 
> >
> >i.e. you can't have a program with
> >  int foo<int> (int)
> >and
> >  long foo<int> (int)
> >in it!
> 
> 
> Stop right here.
> 
> This is not quite right, since the second example is invalid.
> You can, however,  have (geez i haven't written C++ templates in a 
> while. it took me a few compiles to get this right):

I don't think it's invalid on its own.  I think I'm just not being
clear.  It's something like:

template <typename T> int foo(T);
template <typename T> long foo(T);
template <> int foo (int) { return 1;}
template <> long foo (int) { return 2;}

Not that that's valid, but one of them would be "int foo<int> (int)"
and the other would be "long foo<int> (int)".

> template <class blah> blah foo (int a)
> {
>         return a;
> }
> int main(void)
> {
>         int q = foo<int>(5);
>         long r = foo<long>(5);
> }
> 
> Note the only real difference here is in return type.
> It also definitely generates two functions:
> root@dberlin root]# nm -C a.o
>          U __gxx_personality_v0
> 00000000 T main
> 00000000 W int foo<int>(int)
> 00000000 W long foo<long>(int)
> [root@dberlin root]#
> 
> There is no way to write exactly what you want in valid C++ (IIRC), so 
> i'm not sure if this program demonstrates what you think can't be done 
> or not.
> 
> To answer whether you need the return type, let's add two 
> specializations here and make it worse:
> template <> long foo (int a)
> {
> 	return 9;
> }
> template <> int foo (int a)
> {
> 	return 10;
> }
> 
> Now, without the return type in the template, you would *never* know 
> which function to call from a debugger if someone does p foo(5) 
> (However, i'll note the only case you will know is when someone does 
> something like p a=foo(5), where "a" is a program variable of type int 
> or long).
> With the return type,  you could at least present a useful list to the 
> user and ask.
> Unless you want the list to have to re-demangle names so it can get the 
> return type out again, or something ugly like that.

Sure we can.  We look through our list of functions whose name is foo. 
We find that there's a foo<int>(int) returning int and a foo<long>(int)
returning long.  Note that they have unique names because one's
foo<int>(int) and the other's foo<long>(int).  We then ask the user
which one he wanted.

That's what I'm suggesting; I'm just not sure if there's a valid way to
get both "int foo<int>(int)" and "long foo<int>(int)" into the same
program.

In the light of the morning, maybe it's not such a good idea after all.
-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer

Re: [RFA] handling of 'operator' in cp_find_first_component

[David Carlton]

On Mon, 21 Apr 2003 23:21:52 -0400, Daniel Jacobowitz <drow@mvista.com> said:

> As Daniel Berlin pointed out, the demangler is behaving correctly.
> The return type is part of the mangled signature in this case, and
> must be.

I'm actually curious why it must be, but that's another issue; it
simply is the case that it's part of the mangled signature, so the
demangler is behaving correctly.

> However, for GDB's purposes, is it _necessary_ or _useful_ to have
> said mangled name?  If not, for another project I already have a
> cplus_demangle wrapper which plays with the resulting names.
> Adapting it to detect and remove return types would probably be
> easy.

I suspect that it's not useful (I'll talk more about that downthread);
whether or not it's useful, I'm pretty sure that, right now, it's
actively harmful.  We have code in GDB that deals with function names;
that code assumes that the function names doesn't have certain
characteristics.  So if the demangler is returning "int foo<int>(int)"
then I bet that "break foo<int>" doesn't work for us.

I would be tempted to put the code to not give return types in the
demangler itself, though, rather than in a post-processor: that seems
to me to be more reliable and more efficient.

> I am not strong in this area of C++, but as far as I know this should
> suffice; the demangled name of such a function will always include the
> template parameters; and the template-id must be unique in the program
> (right?  Not sure if the ODR comes into play here or not) so either
> there is only one possible return value or the template parameters
> uniquely identify the return value.

> i.e. you can't have a program with
>   int foo<int> (int)
> and
>   long foo<int> (int)
> in it!

That's my attitude, too.  Template parameters and argument types can
distinguish functions; return types can't.  At least I assume so; on
the other hand, presumably the ABI authors had a reason for including
return types in the demangled name in this situation.  But even if
there are some strange boundary cases where it's necessary to
disambiguate functions, I still think that, right now, GDB is much
more likely to be hurt than to be helped by having return types in
there.

> That said, I still think your patch is OK; on the condition that it
> handles the test case the comment you're removing refers to.  Something
> like:
>   int foo<operator() (B&)>
> or whatever it was in the right syntax.  Does it, and can you add that
> to the maint.exp tests?

It should handle them, but you're right, I should include such
examples in the maint.exp tests.  I'll go off and generate some
examples first before checking it in.

David Carlton
carlton@math.stanford.edu

Re: [RFA] handling of 'operator' in cp_find_first_component

[David Carlton]

On Tue, 22 Apr 2003 10:33:05 -0400, Daniel Berlin <dberlin@dberlin.org> said:

> To answer whether you need the return type, let's add two
> specializations here and make it worse:

> template <> long foo (int a)
> {
> 	return 9;
> }
> template <> int foo (int a)
> {
> 	return 10;
> }

Yeah, but that's illegal, isn't it?  You can't have two functions that
differ only in return type: otherwise, how would the compiler know
which one to use in a call to foo?

I tried it out in GCC; the above doesn't compile (I guess templates
with 0 parameters aren't legal), but when I compiled the following
file:

template <typename T> long foo(int a)
{
  return 9;
}
template <typename T> int foo (int a)
{
  return 10;
}

I got:

jackfruit$ g++ -c foo.cpp
foo.cpp:6: new declaration `template<class T> int foo(int)'
foo.cpp:2: ambiguates old declaration `template<class T> long int foo(int)'

Except that I don't understand C++ as well as I could: according to
<http://gcc.gnu.org/ml/gcc-patches/2000-02/msg00879.html>, there are
cases involving templated functions where the compiler is allowed to
disambiguate based on return type.  The above isn't one of them, and
even if I modify them to return objects of completely different types,
I still get a similar error message.

Hmm.  I'm confused.  When I play around with this further, life is
getting more bizarre: GCC happily compiles the following:

class C {};

template <typename T> int foo(int a)
{
  return C();
}
template <typename T> T foo (T a)
{
  return 10;
}

even though the first function is returning an object of the wrong
type!  Am I going crazy, or is that just a bug?  The first function
sure doesn't compile if I remove the template part.

At any rate, obviously I should try to read the standard or Stroustrup
to understand this better.

David Carlton
carlton@math.stanford.edu

Re: [RFA] handling of 'operator' in cp_find_first_component

[David Carlton]

On 23 Apr 2003 16:27:03 -0700, David Carlton <carlton@math.stanford.edu> said:

> Hmm.  I'm confused.  When I play around with this further, life is
> getting more bizarre: GCC happily compiles the following:

> class C {};

> template <typename T> int foo(int a)
> {
>   return C();
> }
> template <typename T> T foo (T a)
> {
>   return 10;
> }

> even though the first function is returning an object of the wrong
> type!  Am I going crazy, or is that just a bug?

No, it's not a bug: it's just that the error isn't signalled until the
template is instantiated, which I wasn't doing.  Silly me.

David Carlton
carlton@math.stanford.edu

Re: [RFA] handling of 'operator' in cp_find_first_component

[Daniel Berlin]

On Wednesday, April 23, 2003, at 07:27  PM, David Carlton wrote:

> On Tue, 22 Apr 2003 10:33:05 -0400, Daniel Berlin 
> <dberlin@dberlin.org> said:
>
>> To answer whether you need the return type, let's add two
>> specializations here and make it worse:
>
>> template <> long foo (int a)
>> {
>> 	return 9;
>> }
>> template <> int foo (int a)
>> {
>> 	return 10;
>> }
>
> Yeah, but that's illegal, isn't it?  You can't have two functions that
> differ only in return type: otherwise, how would the compiler know
> which one to use in a call to foo?
>
> I tried it out in GCC; the above doesn't compile (I guess templates
> with 0 parameters aren't legal),

It's not a template, it's a specialization, i just didn't paste the 
right thing.
template <> int foo<int> (int a)
{
         return 5;
}
template <> long foo<long> (int a)
{
         return 9;
}
is what that should be.

You can have two templates that only differ in return type, and they 
become, when instantiated, two functions that only differ in return 
type.
--Dan

Re: [RFA] handling of 'operator' in cp_find_first_component

[David Carlton]

On 23 Apr 2003 16:02:06 -0700, David Carlton <carlton@math.stanford.edu> said:
> On Mon, 21 Apr 2003 23:21:52 -0400, Daniel Jacobowitz <drow@mvista.com> said:

>> That said, I still think your patch is OK; on the condition that it
>> handles the test case the comment you're removing refers to.  Something
>> like:
>> int foo<operator() (B&)>
>> or whatever it was in the right syntax.  Does it, and can you add that
>> to the maint.exp tests?

> It should handle them, but you're right, I should include such
> examples in the maint.exp tests.  I'll go off and generate some
> examples first before checking it in.

I've checked it in with more examples, as you requested.  I can get
the demangler to produce this:

  jackfruit$ c++filt _Z3fooIXadL_Zls1CS0_EEEiv
  int foo<&(operator<<(C, C))>()

Just to be on the safe side, I added variants of that without the
extra parens and even without the &, but with a comment that I'd only
seen the first one in the wild.  I'm including the new maint.exp patch
below; I didn't change the cp-support.c patch.

David Carlton
carlton@math.stanford.edu

2003-04-23  David Carlton  <carlton@bactrian.org>

	* gdb.c++/maint.exp (test_first_component): Add tests for
	'operator' in more locations.

Index: maint.exp
===================================================================
RCS file: /cvs/src/src/gdb/testsuite/gdb.c++/maint.exp,v
retrieving revision 1.1
diff -u -p -r1.1 maint.exp
--- maint.exp   15 Apr 2003 23:07:11 -0000      1.1
+++ maint.exp   23 Apr 2003 23:43:31 -0000
@@ -63,6 +63,18 @@ proc test_first_component {} {
     test_single_component "foo(std::basic_streambuf<wchar_t,std::char_traits<wchar_t> >)"
     test_single_component "operator>(X::Y)"

+    # Operator names can show up in weird places.
+
+    test_single_component "int operator<< <char>()"
+    test_single_component "T<Cooperator>"
+
+    # NOTE: carlton/2003-04-23: I've only seen the first of these
+    # produced by the demangler, but I'm including two more just to be
+    # on the safe side.
+    test_single_component "int foo<&(operator<<(C, C))>()"
+    test_single_component "int foo<&operator<<(C, C)>()"
+    test_single_component "int foo<operator<<(C, C)>()"
+
     gdb_test "maint cp first_component foo::bar" "foo"
     gdb_test "maint cp first_component foo::bar::baz" "foo"
     gdb_test "maint cp first_component C<A>::bar" "C<A>"