public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] Test and support all cpp operator types
@ 2010-05-11 20:38 sami wagiaalla
  2010-05-13  0:00 ` Tom Tromey
  0 siblings, 1 reply; 6+ messages in thread
From: sami wagiaalla @ 2010-05-11 20:38 UTC (permalink / raw)
  To: gdb-patches

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

This patch adds testing and support for the following types of operator
lookup:
  - non-member operators.
  - imported operators (using directive, anonymous namespaces).
  - ADL operators.

If there are more, please let me know.

It also tests for overload resolution in the presence of combination of
the above.

In the implementation of this I have abandoned the use of
value_struct_elt and used find_overload_match instead. This eliminated
the duplicate implementation of eval-time lookup and brought the
benefits of overload resolution to operator evaluation.

Operators have the unique situation where it is not possible to tell
from the expression whether the intent is a member or non-member
operator eg:

class A {

   int operator==(int){
     return 0;
   }
}

int operator==(char){
   return 1;
}

A a;

(gdb) p a == 1

'find_overload_match' search is mutually exclusive; it performs either
method or non-method search if performed. So it had to be changed to
support the case above; I added a new search mode which would look for
both candidates and compare them to find the best result.

Thanks,
Sami

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

commit 49803a15a3df378b8441102cc5316704b6f96cb9
Author: Sami Wagiaalla <swagiaal@redhat.com>
Date:   Tue May 11 15:39:00 2010 -0400

    test and support all cpp operator types
    
    2010-05-11  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	* value.h: Created oload_search_type enum.
    	(find_overload_match): use oload_search_type enum.
    	* valops.c (find_overload_match): Support combined member and
    	non-member search.
    	(oload_method_static): Verify index is a proper value.
    	* valarith.c (value_user_defined_cpp_op): Search for and handle
    	both member and non-member operators.
    	(value_user_defined_cpp_op): New function.
    	(value_user_defined_op): New function.
    	(value_x_unop): Use value_user_defined_op.
    	(value_x_binop): Ditto.
    	* cp-support.c (make_symbol_overload_list_using): Added block
    	iteration.
    	Add check for namespace aliases and imported declarations.
    
    2010-05-11  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	* gdb.cp/koenig.exp: Test for ADL operators.
    	* gdb.cp/koenig.cc: Added ADL operators.
    	* gdb.cp/operator.exp: New test.
    	* gdb.cp/operator.cc: New test.

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 8f447ca..87805c9 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -800,21 +800,27 @@ make_symbol_overload_list_using (const char *func_name,
 				 const char *namespace)
 {
   const struct using_direct *current;
+  const struct block *block;
 
   /* First, go through the using directives.  If any of them apply,
      look in the appropriate namespaces for new functions to match
      on.  */
 
-  for (current = block_using (get_selected_block (0));
-       current != NULL;
-       current = current->next)
-    {
-      if (strcmp (namespace, current->import_dest) == 0)
-	{
-	  make_symbol_overload_list_using (func_name,
-					   current->import_src);
-	}
-    }
+  for (block = get_selected_block (0);
+       block != NULL;
+       block = BLOCK_SUPERBLOCK(block))
+    for (current = block_using (block);
+	current != NULL;
+	current = current->next)
+      {
+        /* If this is a namespace alias or imported declaration ignore it.  */
+        if (current->alias != NULL || current->declaration !=NULL)
+          continue;
+
+        if (strcmp (namespace, current->import_dest) == 0)
+            make_symbol_overload_list_using (func_name, current->import_src);
+
+      }
 
   /* Now, add names for this namespace.  */
   make_symbol_overload_list_namespace (func_name, namespace);
diff --git a/gdb/testsuite/gdb.cp/koenig.cc b/gdb/testsuite/gdb.cp/koenig.cc
index 6cfa3f5..3d4c7fb 100644
--- a/gdb/testsuite/gdb.cp/koenig.cc
+++ b/gdb/testsuite/gdb.cp/koenig.cc
@@ -175,6 +175,43 @@ typedef O::A TOA;
 typedef TOA  TTOA;
 
 //------------
+
+namespace P {
+  class Q{
+  public:
+    int operator== (int){
+      return 24;
+    }
+
+    int operator== (float){
+      return 25;
+    }
+
+    int operator+ (float){
+      return 26;
+    }
+
+  };
+
+  int operator!= (Q, int){
+    return 27;
+  }
+
+  int operator!= (Q, double){
+    return 28;
+  }
+
+  int operator+ (Q, int){
+    return 29;
+  }
+
+  int operator++ (Q){
+    return 30;
+  }
+
+}
+
+//------------
 int
 main ()
 {
@@ -238,6 +275,16 @@ main ()
   TTOA ttoa;
   foo (ttoa, 'a');
 
+  P::Q q;
+  q == 5;
+  q == 5.0f;
+  q != 5;
+  q != 5.0f;
+  q + 5;
+  q + 5.0f;
+
+  ++q;
+
   return first (0, c) + foo (eo) +
          foo (eo, eo) + foo (eo, eo, 1)  +
          foo (fo, eo) + foo (1  ,fo, eo) +
diff --git a/gdb/testsuite/gdb.cp/koenig.exp b/gdb/testsuite/gdb.cp/koenig.exp
index b13ffbc..37c770b 100644
--- a/gdb/testsuite/gdb.cp/koenig.exp
+++ b/gdb/testsuite/gdb.cp/koenig.exp
@@ -107,3 +107,22 @@ gdb_test "p M::N::bar('a')" "= 22"
 
 #test that lookup supports typedef
 gdb_test "p foo(ttoa, 'a')" "= 23"
+
+# test lookup of namespace user-defined operators
+# and overload resolution:
+
+# within class
+gdb_test "p q == 5" "= 24"
+gdb_test "p q == 5.0f" "= 25"
+
+# within namespace
+gdb_test "p q != 5" "= 27"
+gdb_test "p q != 5.0f" "= 28"
+
+# across namespace and class
+gdb_test "p q + 5.0f" "= 26"
+gdb_test "p q + 5" "= 29"
+
+# some unary operators for good measure
+# Cannot resolve function operator++ to any overloaded instance
+gdb_test "p ++q" "= 30"
diff --git a/gdb/testsuite/gdb.cp/operator.cc b/gdb/testsuite/gdb.cp/operator.cc
new file mode 100644
index 0000000..6063eb1
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/operator.cc
@@ -0,0 +1,148 @@
+class A {};
+
+int operator== (A, int) {
+  return 11;
+}
+
+int operator== (A, char) {
+  return 12;
+}
+
+//------------------
+
+namespace B{
+  class C {};
+
+  int operator== (C, int) {
+    return 22;
+  }
+
+  int operator== (C, char) {
+    return 23;
+  }
+}
+
+//------------------
+
+class D {};
+namespace {
+  int operator==(D,int) {
+    return 33;
+  }
+
+  int operator==(D,char) {
+    return 34;
+  }
+}
+
+int operator==(D,float) {
+  return 35;
+}
+
+//------------------
+
+class E {};
+namespace F {
+  int operator== (E, int){
+    return 44;
+  }
+
+  int operator== (E, char){
+    return 45;
+  }
+}
+
+int operator== (E, float){
+  return 46;
+}
+
+using namespace F;
+
+//-----------------
+
+class G {
+public:
+  int operator== (int) {
+    return 55;
+  }
+};
+
+int operator== (G, char){
+  return 56;
+}
+
+//------------------
+
+class H {};
+namespace I {
+  int operator== (H, int){
+    return 66;
+  }
+}
+
+namespace ALIAS = I;
+
+//------------------
+
+class J {};
+
+namespace K {
+  int i;
+  int operator== (J, int){
+    return 77;
+  }
+}
+
+using K::i;
+
+//------------------
+
+class L{};
+namespace M{
+  int operator== (L, int){
+    return 88;
+  }
+}
+
+namespace N{
+  using namespace M;
+}
+
+using namespace N;
+
+//------------------
+
+int main () {
+  A a;
+  a == 1;
+  a == 'a';
+
+  B::C bc;
+  bc == 1;
+  bc == 'a';
+
+  D d;
+  d == 1;
+  d == 'a';
+  d == 1.0f;
+
+  E e;
+  e == 1;
+  e == 'a';
+  e == 1.0f;
+
+  G g;
+  g == 1;
+  g == 'a';
+
+  H h;
+  I::operator== (h,1);
+
+  J j;
+  K::operator== (j,1);
+
+  L l;
+  l == 1;
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/operator.exp b/gdb/testsuite/gdb.cp/operator.exp
new file mode 100644
index 0000000..b9bf157
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/operator.exp
@@ -0,0 +1,73 @@
+# 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 operator
+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 global operator
+gdb_test "p a == 1" "= 11" "global operator"
+gdb_test "p a == 'a'" "= 12" "global operator overload"
+
+# Test ADL operator
+gdb_test "p bc == 1" "= 22" "ADL operator"
+gdb_test "p bc == 'a'" "= 23" "ADL operator overload"
+
+# Test operator imported from anonymous namespace
+gdb_test "p d == 1" "= 33" "anonymous namespace operator"
+gdb_test "p d == 'a'" "= 34" "anonymous namespace operator overload"
+gdb_test "p d == 1.0f" "= 35" "anonymous namespace operator overload float"
+
+# Test operator imported by using directive
+gdb_test "p e == 1" "= 44" "imported operator"
+gdb_test "p e == 'a'" "= 45" "imported operator overload"
+gdb_test "p e == 1.0f" "= 46" "imported operator overload float"
+
+# Test member operator
+gdb_test "p g == 1" "= 55" "member operator"
+gdb_test "p g == 'a'" "= 56" "member operator overload"
+
+# Test that operators are not wrongly imported
+# by import declarations and namespace aliases
+gdb_test "p h == 1" "Cannot resolve function operator== to any overloaded instance" "namespace alias"
+gdb_test "p j == 1" "Cannot resolve function operator== to any overloaded instance" "imported declaration"
+
+# Test that indirectly imported operators work
+gdb_test "p l == 1" "= 88"
\ No newline at end of file
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 4efe936..2dbcb77 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -31,6 +31,7 @@
 #include "dfp.h"
 #include <math.h>
 #include "infcall.h"
+#include "exceptions.h"
 
 /* Define whether or not the C operator '/' truncates towards zero for
    differently signed operands (truncation direction is undefined in C). */
@@ -318,6 +319,67 @@ unop_user_defined_p (enum exp_opcode op, struct value *arg1)
     }
 }
 
+/* Try to find an operator named OPERATOR which takes NARGS arguments
+   specified in ARGS.  If the operator found is a static member operator
+   *STATIC_MEMFUNP will be set to 1, and otherwise 0.
+   The search if performed through find_overload_match which will handle
+   member operators, non member operators, operators imported implicitly or
+   explicitly, and perform correct overload resolution in all of the above
+   situations or combinations thereof.  */
+
+static struct value *
+value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
+                           int *static_memfuncp)
+{
+
+  struct symbol *symp = NULL;
+  struct value *valp = NULL;
+  struct type **arg_types;
+  int i;
+
+  arg_types = (struct type **) alloca (nargs * (sizeof (struct type *)));
+  /* Prepare list of argument types for overload resolution */
+  for (i = 0; i < nargs; i++)
+    arg_types[i] = value_type (args[i]);
+
+  find_overload_match (arg_types, nargs, operator, 2 /* could be method */,
+                       0 /* strict match */, &args[0], /* objp */
+                       NULL /* pass NULL symbol since symbol is unknown */,
+                       &valp, &symp, static_memfuncp, 0);
+
+  if (valp)
+    return valp;
+
+  if (symp)
+    {
+      /* This is a non member function and does not
+         expect a reference as its first argument
+         rather the explicit structure.  */
+      args[0] = value_ind (args[0]);
+      return value_of_variable (symp, 0);
+    }
+
+  return NULL;
+}
+
+/* Lookup user defined operator NAME.  Return a value representing the
+   function, otherwise return NULL.  */
+
+static struct value *
+value_user_defined_op (struct value **argp, struct value **args, char *name,
+                       int *static_memfuncp, int nargs)
+{
+  struct value *result = NULL;
+
+  if (current_language->la_language == language_cplus)
+    result = value_user_defined_cpp_op (args, nargs, name, static_memfuncp);
+  else
+    result = value_struct_elt (argp, args, name, static_memfuncp,
+                               "structure");
+
+  return result;
+}
+
 /* We know either arg1 or arg2 is a structure, so try to find the right
    user defined function.  Create an argument vector that calls 
    arg1.operator @ (arg1,arg2) and return that value (where '@' is any
@@ -458,7 +520,8 @@ value_x_binop (struct value *arg1, struct value *arg2, enum exp_opcode op,
       error (_("Invalid binary operation specified."));
     }
 
-  argvec[0] = value_struct_elt (&arg1, argvec + 1, tstr, &static_memfuncp, "structure");
+  argvec[0] = value_user_defined_op (&arg1, argvec + 1, tstr,
+                                     &static_memfuncp, 2);
 
   if (argvec[0])
     {
@@ -555,7 +618,8 @@ value_x_unop (struct value *arg1, enum exp_opcode op, enum noside noside)
       error (_("Invalid unary operation specified."));
     }
 
-  argvec[0] = value_struct_elt (&arg1, argvec + 1, tstr, &static_memfuncp, "structure");
+  argvec[0] = value_user_defined_op (&arg1, argvec + 1, tstr,
+                                     &static_memfuncp, nargs);
 
   if (argvec[0])
     {
diff --git a/gdb/valops.c b/gdb/valops.c
index 6f5f684..4d38128 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2266,7 +2266,6 @@ value_find_oload_method_list (struct value **argp, const char *method,
   struct type *t;
 
   t = check_typedef (value_type (*argp));
-
   /* Code snarfed from value_struct_elt.  */
   while (TYPE_CODE (t) == TYPE_CODE_PTR || TYPE_CODE (t) == TYPE_CODE_REF)
     {
@@ -2293,6 +2292,16 @@ value_find_oload_method_list (struct value **argp, const char *method,
    matches on the argument types according to the overload resolution
    rules.
 
+   METHOD can be one of three valuse:
+     NON_METHOD for non-member functions.
+     METHOD: for member functions.
+     BOTH: used for overload resolution of operators where the
+       candidates are expected to be either member or non member
+       functions. In this case the first argument ARGTYPES
+       (representing 'this') is expected to be a reference to the
+       target object, and will be dereferenced when attempting the
+       non-member search.
+
    In the case of class methods, the parameter OBJ is an object value
    in which to search for overloaded methods.
 
@@ -2320,16 +2329,21 @@ value_find_oload_method_list (struct value **argp, const char *method,
 
 int
 find_overload_match (struct type **arg_types, int nargs, 
-		     const char *name, int method, int lax, 
-		     struct value **objp, struct symbol *fsym,
+		     const char *name, enum oload_search_type method,
+		     int lax, struct value **objp, struct symbol *fsym,
 		     struct value **valp, struct symbol **symp, 
 		     int *staticp, const int no_adl)
 {
   struct value *obj = (objp ? *objp : NULL);
+
   /* Index of best overloaded function.  */
-  int oload_champ;
+  int func_oload_champ = -1;
+  int method_oload_champ = -1;
+
   /* The measure for the current best match.  */
-  struct badness_vector *oload_champ_bv = NULL;
+  struct badness_vector *method_badness = NULL;
+  struct badness_vector *func_badness = NULL;
+
   struct value *temp = obj;
   /* For methods, the list of overloaded methods.  */
   struct fn_field *fns_ptr = NULL;
@@ -2345,9 +2359,11 @@ find_overload_match (struct type **arg_types, int nargs,
   const char *obj_type_name = NULL;
   const char *func_name = NULL;
   enum oload_classification match_quality;
+  enum oload_classification method_match_quality = INCOMPATIBLE;
+  enum oload_classification func_match_quality = INCOMPATIBLE;
 
   /* Get the list of overloaded methods or functions.  */
-  if (method)
+  if (method == METHOD || method == BOTH)
     {
       gdb_assert (obj);
 
@@ -2370,26 +2386,50 @@ find_overload_match (struct type **arg_types, int nargs,
 	    }
 	}
 
+      /* Retrieve the list of methods with the name NAME.  */
       fns_ptr = value_find_oload_method_list (&temp, name, 
 					      0, &num_fns, 
 					      &basetype, &boffset);
-      if (!fns_ptr || !num_fns)
+
+      /* If this is a method only search, and no methods were found
+         the search has faild.  */
+      if (method == METHOD && (!fns_ptr || !num_fns))
 	error (_("Couldn't find method %s%s%s"),
 	       obj_type_name,
 	       (obj_type_name && *obj_type_name) ? "::" : "",
 	       name);
+
       /* If we are dealing with stub method types, they should have
 	 been resolved by find_method_list via
 	 value_find_oload_method_list above.  */
-      gdb_assert (TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL);
-      oload_champ = find_oload_champ (arg_types, nargs, method, 
-				      num_fns, fns_ptr, 
-				      oload_syms, &oload_champ_bv);
+      if (fns_ptr)
+	{
+	  gdb_assert (TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL);
+	  method_oload_champ = find_oload_champ (arg_types, nargs, method,
+	                                         num_fns, fns_ptr,
+	                                         oload_syms, &method_badness);
+
+	  method_match_quality =
+	      classify_oload_match (method_badness, nargs,
+	                            oload_method_static (method, fns_ptr,
+	                                                 method_oload_champ));
+
+	  make_cleanup (xfree, method_badness);
+	}
+
     }
-  else
+
+  if (method == NON_METHOD || method == BOTH)
     {
+
       const char *qualified_name = NULL;
 
+      /* If the the overload match is being search for both
+         as a method and non member function, the first argument
+         must now be dereferenced.  */
+      if (method == BOTH)
+	arg_types[0] = TYPE_TARGET_TYPE (arg_types[0]);
+
       if (fsym)
         {
           qualified_name = SYMBOL_NATURAL_NAME (fsym);
@@ -2432,30 +2472,67 @@ find_overload_match (struct type **arg_types, int nargs,
           return 0;
         }
 
-      make_cleanup (xfree, oload_syms);
-      make_cleanup (xfree, oload_champ_bv);
+      func_oload_champ = find_oload_champ_namespace (arg_types, nargs,
+                                                     func_name,
+                                                     qualified_name,
+                                                     &oload_syms,
+                                                     &func_badness,
+                                                     no_adl);
+
+      if (func_oload_champ >= 0)
+	func_match_quality = classify_oload_match (func_badness, nargs, 0);
 
-      oload_champ = find_oload_champ_namespace (arg_types, nargs,
-						func_name,
-						qualified_name,
-						&oload_syms,
-						&oload_champ_bv,
-						no_adl);
+      make_cleanup (xfree, oload_syms);
+      make_cleanup (xfree, func_badness);
     }
 
   /* Did we find a match ?  */
-  if (oload_champ == -1)
+  if (method_oload_champ == -1 && func_oload_champ == -1)
     error (_("No symbol \"%s\" in current context."), name);
 
-  /* Check how bad the best match is.  */
-  match_quality =
-    classify_oload_match (oload_champ_bv, nargs,
-			  oload_method_static (method, fns_ptr,
-					       oload_champ));
+  /* If we have found both a method match and a function
+     match, find out which one is better, and calculate match
+     quality.  */
+  if (method_oload_champ >= 0 && func_oload_champ >= 0)
+    {
+      switch (compare_badness (func_badness, method_badness))
+        {
+	  case 0: /* Top two contenders are equally good.  */
+	    /* FIXME: GDB does not support the general ambiguous
+	     case.  All candidates should be collected and presented
+	     the the user.  */
+	    error (_("Ambiguous overload resolution"));
+	    break;
+	  case 1: /* Incomparable top contenders.  */
+	    /* This is an error incompatible candidates
+	       should not have been proposed.  */
+	    error (_("Internal error: incompatible overload candidates proposed"));
+	    break;
+	  case 2: /* Function champion.  */
+	    method_oload_champ = -1;
+	    match_quality = func_match_quality;
+	    break;
+	  case 3: /* Method champion.  */
+	    func_oload_champ = -1;
+	    match_quality = method_match_quality;
+	    break;
+	  default:
+	    error (_("Internal error: unexpected overload comparison result"));
+	    break;
+        }
+    }
+  else
+    {
+      /* We have either a method match or a function match.  */
+      if (method_oload_champ >= 0)
+	match_quality = method_match_quality;
+      else
+	match_quality = func_match_quality;
+    }
 
   if (match_quality == INCOMPATIBLE)
     {
-      if (method)
+      if (method == METHOD)
 	error (_("Cannot resolve method %s%s%s to any overloaded instance"),
 	       obj_type_name,
 	       (obj_type_name && *obj_type_name) ? "::" : "",
@@ -2466,7 +2543,7 @@ find_overload_match (struct type **arg_types, int nargs,
     }
   else if (match_quality == NON_STANDARD)
     {
-      if (method)
+      if (method == METHOD)
 	warning (_("Using non-standard conversion to match method %s%s%s to supplied arguments"),
 		 obj_type_name,
 		 (obj_type_name && *obj_type_name) ? "::" : "",
@@ -2476,20 +2553,21 @@ find_overload_match (struct type **arg_types, int nargs,
 		 func_name);
     }
 
-  if (method)
+  if (staticp != NULL)
+    *staticp = oload_method_static (method, fns_ptr, method_oload_champ);
+
+  if (method_oload_champ >= 0)
     {
-      if (staticp != NULL)
-	*staticp = oload_method_static (method, fns_ptr, oload_champ);
-      if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, oload_champ))
-	*valp = value_virtual_fn_field (&temp, fns_ptr, oload_champ, 
+      if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ))
+	*valp = value_virtual_fn_field (&temp, fns_ptr, method_oload_champ,
 					basetype, boffset);
       else
-	*valp = value_fn_field (&temp, fns_ptr, oload_champ, 
+	*valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
 				basetype, boffset);
     }
   else
     {
-      *symp = oload_syms[oload_champ];
+      *symp = oload_syms[func_oload_champ];
     }
 
   if (objp)
@@ -2778,7 +2856,8 @@ find_oload_champ (struct type **arg_types, int nargs, int method,
 static int
 oload_method_static (int method, struct fn_field *fns_ptr, int index)
 {
-  if (method && TYPE_FN_FIELD_STATIC_P (fns_ptr, index))
+  if (method && fns_ptr && index >= 0
+      && TYPE_FN_FIELD_STATIC_P (fns_ptr, index))
     return 1;
   else
     return 0;
diff --git a/gdb/value.h b/gdb/value.h
index 57b4dd7..a7ed135 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -447,8 +447,10 @@ extern struct fn_field *value_find_oload_method_list (struct value **,
 						      int, int *,
 						      struct type **, int *);
 
+enum oload_search_type { NON_METHOD, METHOD, BOTH };
 extern int find_overload_match (struct type **arg_types, int nargs,
-				const char *name, int method, int lax,
+				const char *name,
+				enum oload_search_type method, int lax,
 				struct value **objp, struct symbol *fsym,
 				struct value **valp, struct symbol **symp,
 				int *staticp, const int no_adl);



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] Test and support all cpp operator types
  2010-05-11 20:38 [PATCH] Test and support all cpp operator types sami wagiaalla
@ 2010-05-13  0:00 ` Tom Tromey
  2010-05-18 20:49   ` [PATCH 1/2] " sami wagiaalla
  2010-05-18 22:31   ` [PATCH 2/2] " sami wagiaalla
  0 siblings, 2 replies; 6+ messages in thread
From: Tom Tromey @ 2010-05-13  0:00 UTC (permalink / raw)
  To: sami wagiaalla; +Cc: gdb-patches

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

Sami> This patch adds testing and support for the following types of operator
Sami> lookup:
Sami>  - non-member operators.
Sami>  - imported operators (using directive, anonymous namespaces).
Sami>  - ADL operators.

I like the general approach of this patch.

I have a few comments.

Sami> @@ -800,21 +800,27 @@ make_symbol_overload_list_using (const char *func_name,
Sami>  				 const char *namespace)
[...]
Sami> -  for (current = block_using (get_selected_block (0));
Sami> -       current != NULL;
Sami> -       current = current->next)
Sami> -    {
Sami> -      if (strcmp (namespace, current->import_dest) == 0)
Sami> -	{
Sami> -	  make_symbol_overload_list_using (func_name,
Sami> -					   current->import_src);
Sami> -	}
Sami> -    }
Sami> +  for (block = get_selected_block (0);
Sami> +       block != NULL;
Sami> +       block = BLOCK_SUPERBLOCK(block))
Sami> +    for (current = block_using (block);
Sami> +	current != NULL;
Sami> +	current = current->next)
Sami> +      {
[...]

This part seems a little weird to me.
make_symbol_overload_list_using calls make_symbol_overload_list_namespace,
which calls make_symbol_overload_list_qualified, which itself
starts with get_selected_block and iterates over the superblocks.

It seems to me that only one such iteration should be needed.
I don't have a test case but it seems like this could cause incorrect
results in some corner case.

Also, missing space after BLOCK_SUPERBLOCK.

Sami> +        /* If this is a namespace alias or imported declaration ignore it.  */
Sami> +        if (current->alias != NULL || current->declaration !=NULL)

Missing space before NULL.

Sami> +        if (strcmp (namespace, current->import_dest) == 0)
Sami> +            make_symbol_overload_list_using (func_name, current->import_src);
Sami> +
Sami> +      }
 
Wrong indentation on the line after the 'if'.
Gratuitous blank line before the closing brace.

Sami> +# some unary operators for good measure
Sami> +# Cannot resolve function operator++ to any overloaded instance
Sami> +gdb_test "p ++q" "= 30"

It would be interesting to know if "q++" would call an overloaded
postfix operator++.  These have a hack in the C++ spec to differentiate
them from prefix ++.

I'm also curious to know if "ADL avoidance" works properly when a
qualified reference to "operator<whatever>" is used.  I didn't see a
test for that in this patch.  (If there is already one in the test
suite, then of course we don't need a new one...)

Sami> +++ b/gdb/testsuite/gdb.cp/operator.exp
[...]
Sami> +set prms_id 0
Sami> +set bug_id 0

I think Joel just nuked these from the rest of the test suite.

Sami> +set testfile operator
Sami> +set srcfile ${testfile}.cc
Sami> +set binfile ${objdir}/${subdir}/${testfile}
Sami> +if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
Sami> +    untested "Couldn't compile test program"
Sami> +    return -1
Sami> +}
Sami> +
Sami> +# Get things started.
Sami> +
Sami> +gdb_exit
Sami> +gdb_start
Sami> +gdb_reinitialize_dir $srcdir/$subdir
Sami> +gdb_load ${binfile}

There is some convenience proc that automates a lot of this now.
I forget what but I think it is on the wiki.

Sami> \ No newline at end of file

Please add one.

Sami> +static struct value *
Sami> +value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
Sami> +                           int *static_memfuncp)
[...]
Sami> +  find_overload_match (arg_types, nargs, operator, 2 /* could be method */,

This should use the new enum constant, not '2'.

I think this patch ought to update all other callers of
find_overload_match to use the constants.

Sami> +/* Lookup user defined operator NAME.  Return a value representing the
Sami> +   function, otherwise return NULL.  */
Sami> +
Sami> +static struct value *
Sami> +value_user_defined_op (struct value **argp, struct value **args, char *name,
Sami> +                       int *static_memfuncp, int nargs)

I think the error return here is odd.
value_struct_elt will throw, rather than return NULL.
Perhaps that is what value_user_defined_cpp_op ought to do as well.

Sami> @@ -2266,7 +2266,6 @@ value_find_oload_method_list (struct value **argp, const char *method,
Sami>    struct type *t;
 
Sami>    t = check_typedef (value_type (*argp));
Sami> -
Sami>    /* Code snarfed from value_struct_elt.  */

Don't change whitespace.

Sami> +   METHOD can be one of three valuse:

Typo, "values".

I haven't yet digested the rest of the changes to find_overload_match.

Sami> +enum oload_search_type { NON_METHOD, METHOD, BOTH };
Sami>  extern int find_overload_match (struct type **arg_types, int nargs,
Sami> -				const char *name, int method, int lax,
Sami> +				const char *name,
Sami> +				enum oload_search_type method, int lax,
Sami>  				struct value **objp, struct symbol *fsym,
Sami>  				struct value **valp, struct symbol **symp,
Sami>  				int *staticp, const int no_adl);

Blank line after the enum definition.
Normally I would insist on a comment for the enum as well, but I think
it is reasonably clear, especially given the intro comment to
find_overload_match.

Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Test and support all cpp operator types
  2010-05-13  0:00 ` Tom Tromey
@ 2010-05-18 20:49   ` sami wagiaalla
  2010-06-04 20:55     ` Tom Tromey
  2010-05-18 22:31   ` [PATCH 2/2] " sami wagiaalla
  1 sibling, 1 reply; 6+ messages in thread
From: sami wagiaalla @ 2010-05-18 20:49 UTC (permalink / raw)
  To: tromey; +Cc: gdb-patches

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


> Sami>  @@ -800,21 +800,27 @@ make_symbol_overload_list_using (const char *func_name,
> Sami>   				const char *namespace)
> [...]
> Sami>  -  for (current = block_using (get_selected_block (0));
> Sami>  -       current != NULL;
> Sami>  -       current = current->next)
> Sami>  -    {
> Sami>  -      if (strcmp (namespace, current->import_dest) == 0)
> Sami>  -	{
> Sami>  -	  make_symbol_overload_list_using (func_name,
> Sami>  -					   current->import_src);
> Sami>  -	}
> Sami>  -    }
> Sami>  +  for (block = get_selected_block (0);
> Sami>  +       block != NULL;
> Sami>  +       block = BLOCK_SUPERBLOCK(block))
> Sami>  +    for (current = block_using (block);
> Sami>  +	current != NULL;
> Sami>  +	current = current->next)
> Sami>  +      {
> [...]
>
> This part seems a little weird to me.
> make_symbol_overload_list_using calls make_symbol_overload_list_namespace,
> which calls make_symbol_overload_list_qualified, which itself
> starts with get_selected_block and iterates over the superblocks.
>
> It seems to me that only one such iteration should be needed.
> I don't have a test case but it seems like this could cause incorrect
> results in some corner case.
>

It is weird but it is due to the nature of the symbol tables. The first 
iteration, by make_symbol_overload_list_using, is iteration over the 
blocks' import statements. The Second iteration is actually searching 
for the symbol.

If we assume that names belonging to namespaces can only be in the 
static scope -since namespace symbols are flattened to the nearest 
parent- then we can separate the qualified and unqualified searches
and eliminate the nesting of the iterations.

Please see patch 2/2.

> Sami>  +# some unary operators for good measure
> Sami>  +# Cannot resolve function operator++ to any overloaded instance
> Sami>  +gdb_test "p ++q" "= 30"
>
> It would be interesting to know if "q++" would call an overloaded
> postfix operator++.  These have a hack in the C++ spec to differentiate
> them from prefix ++.
>

It does work. This is tested in userdef.exp.

> I'm also curious to know if "ADL avoidance" works properly when a
> qualified reference to "operator<whatever>" is used.  I didn't see a
> test for that in this patch.  (If there is already one in the test
> suite, then of course we don't need a new one...)
>

Yes it does. I have added a test case.

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

commit 493c3d32944b9b355158a46b300050ff2fdbfb59
Author: Sami Wagiaalla <swagiaal@redhat.com>
Date:   Tue May 11 15:39:00 2010 -0400

    test and support all cpp operator types
    
    2010-05-18  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	* value.h: Created oload_search_type enum.
    	(find_overload_match): Use oload_search_type enum.
    	* valops.c (find_overload_match): Support combined member and
    	non-member search.
    	* eval.c (evaluate_subexp_standard): Calls to
    	find_overload_match now use oload_search_type enum.
    	(oload_method_static): Verify index is a proper value.
    	* valarith.c (value_user_defined_cpp_op): Search for and handle
    	both member and non-member operators.
    	(value_user_defined_cpp_op): New function.
    	(value_user_defined_op): New function.
    	(value_x_unop): Use value_user_defined_op.
    	(value_x_binop): Ditto.
    	* cp-support.c (make_symbol_overload_list_using): Added block
    	iteration.
    	Add check for namespace aliases and imported declarations.
    
    2010-05-18  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	* gdb.cp/koenig.exp: Test for ADL operators.
    	* gdb.cp/koenig.cc: Added ADL operators.
    	* gdb.cp/operator.exp: New test.
    	* gdb.cp/operator.cc: New test.

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 8f447ca..0a0f777 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -800,21 +800,26 @@ make_symbol_overload_list_using (const char *func_name,
 				 const char *namespace)
 {
   const struct using_direct *current;
+  const struct block *block;
 
   /* First, go through the using directives.  If any of them apply,
      look in the appropriate namespaces for new functions to match
      on.  */
 
-  for (current = block_using (get_selected_block (0));
-       current != NULL;
-       current = current->next)
-    {
-      if (strcmp (namespace, current->import_dest) == 0)
-	{
-	  make_symbol_overload_list_using (func_name,
-					   current->import_src);
-	}
-    }
+  for (block = get_selected_block (0);
+       block != NULL;
+       block = BLOCK_SUPERBLOCK (block))
+    for (current = block_using (block);
+	current != NULL;
+	current = current->next)
+      {
+        /* If this is a namespace alias or imported declaration ignore it.  */
+        if (current->alias != NULL || current->declaration != NULL)
+          continue;
+
+        if (strcmp (namespace, current->import_dest) == 0)
+          make_symbol_overload_list_using (func_name, current->import_src);
+      }
 
   /* Now, add names for this namespace.  */
   make_symbol_overload_list_namespace (func_name, namespace);
diff --git a/gdb/eval.c b/gdb/eval.c
index 985e653..4beb1bd 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -1512,7 +1512,7 @@ evaluate_subexp_standard (struct type *expect_type,
             arg_types[ix - 1] = value_type (argvec[ix]);
 
           find_overload_match (arg_types, nargs, func_name,
-                               0 /* not method */ , 0 /* strict match */ ,
+                               NON_METHOD /* not method */ , 0 /* strict match */ ,
                                NULL, NULL /* pass NULL symbol since symbol is unknown */ ,
                                NULL, &symp, NULL, 0);
 
@@ -1549,7 +1549,7 @@ evaluate_subexp_standard (struct type *expect_type,
 		arg_types[ix - 1] = value_type (argvec[ix]);
 
 	      (void) find_overload_match (arg_types, nargs, tstr,
-				     1 /* method */ , 0 /* strict match */ ,
+	                                  METHOD /* method */ , 0 /* strict match */ ,
 					  &arg2 /* the object */ , NULL,
 					  &valp, NULL, &static_memfuncp, 0);
 
@@ -1618,8 +1618,8 @@ evaluate_subexp_standard (struct type *expect_type,
 		arg_types[ix - 1] = value_type (argvec[ix]);
 
 	      (void) find_overload_match (arg_types, nargs, NULL /* no need for name */ ,
-				 0 /* not method */ , 0 /* strict match */ ,
-		      NULL, function /* the function */ ,
+	                                  NON_METHOD /* not method */ , 0 /* strict match */ ,
+	                                  NULL, function /* the function */ ,
 					  NULL, &symp, NULL, no_adl);
 
 	      if (op == OP_VAR_VALUE)
diff --git a/gdb/testsuite/gdb.cp/koenig.cc b/gdb/testsuite/gdb.cp/koenig.cc
index 6cfa3f5..24b3ae7 100644
--- a/gdb/testsuite/gdb.cp/koenig.cc
+++ b/gdb/testsuite/gdb.cp/koenig.cc
@@ -175,6 +175,49 @@ typedef O::A TOA;
 typedef TOA  TTOA;
 
 //------------
+
+namespace P {
+  class Q{
+  public:
+    int operator== (int)
+      {
+        return 24;
+      }
+
+    int operator== (float)
+      {
+        return 25;
+      }
+
+    int operator+ (float)
+      {
+        return 26;
+      }
+
+  };
+
+  int operator!= (Q, int)
+    {
+      return 27;
+    }
+
+  int operator!= (Q, double)
+    {
+      return 28;
+    }
+
+  int operator+ (Q, int)
+    {
+      return 29;
+    }
+
+  int operator++ (Q)
+    {
+      return 30;
+    }
+}
+
+//------------
 int
 main ()
 {
@@ -238,6 +281,16 @@ main ()
   TTOA ttoa;
   foo (ttoa, 'a');
 
+  P::Q q;
+  q == 5;
+  q == 5.0f;
+  q != 5;
+  q != 5.0f;
+  q + 5;
+  q + 5.0f;
+
+  ++q;
+
   return first (0, c) + foo (eo) +
          foo (eo, eo) + foo (eo, eo, 1)  +
          foo (fo, eo) + foo (1  ,fo, eo) +
diff --git a/gdb/testsuite/gdb.cp/koenig.exp b/gdb/testsuite/gdb.cp/koenig.exp
index b13ffbc..12b4ddf 100644
--- a/gdb/testsuite/gdb.cp/koenig.exp
+++ b/gdb/testsuite/gdb.cp/koenig.exp
@@ -13,28 +13,12 @@
 # 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 koenig
 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"
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
     return -1
 }
 
-# Get things started.
-
-gdb_exit
-gdb_start
-gdb_reinitialize_dir $srcdir/$subdir
-gdb_load ${binfile}
-
 ############################################
 
 if ![runto_main] then {
@@ -107,3 +91,22 @@ gdb_test "p M::N::bar('a')" "= 22"
 
 #test that lookup supports typedef
 gdb_test "p foo(ttoa, 'a')" "= 23"
+
+# test lookup of namespace user-defined operators
+# and overload resolution:
+
+# within class
+gdb_test "p q == 5" "= 24"
+gdb_test "p q == 5.0f" "= 25"
+
+# within namespace
+gdb_test "p q != 5" "= 27"
+gdb_test "p q != 5.0f" "= 28"
+
+# across namespace and class
+gdb_test "p q + 5.0f" "= 26"
+gdb_test "p q + 5" "= 29"
+
+# some unary operators for good measure
+# Cannot resolve function operator++ to any overloaded instance
+gdb_test "p ++q" "= 30"
diff --git a/gdb/testsuite/gdb.cp/operator.cc b/gdb/testsuite/gdb.cp/operator.cc
new file mode 100644
index 0000000..cc925a0
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/operator.cc
@@ -0,0 +1,195 @@
+class A
+{
+};
+
+int operator== (A, int)
+{
+  return 11;
+}
+
+int operator== (A, char)
+{
+  return 12;
+}
+
+//------------------
+
+namespace B
+{
+  class C
+  {
+  };
+
+  int operator== (C, int)
+  {
+    return 22;
+  }
+
+  int operator== (C, char)
+  {
+    return 23;
+  }
+
+  namespace BD
+  {
+    int operator== (C, int)
+    {
+      return 24;
+    }
+  }
+}
+
+//------------------
+
+class D
+{
+};
+namespace
+{
+  int operator== (D, int)
+  {
+    return 33;
+  }
+
+  int operator== (D, char)
+  {
+    return 34;
+  }
+}
+
+int operator== (D, float)
+{
+  return 35;
+}
+
+//------------------
+
+class E
+{
+};
+namespace F
+{
+  int operator== (E, int)
+  {
+    return 44;
+  }
+
+  int operator== (E, char)
+  {
+    return 45;
+  }
+}
+
+int operator== (E, float)
+{
+  return 46;
+}
+
+using namespace F;
+
+//-----------------
+
+class G
+{
+public:
+  int operator== (int)
+  {
+    return 55;
+  }
+};
+
+int operator== (G, char)
+{
+  return 56;
+}
+
+//------------------
+
+class H
+{
+};
+namespace I
+{
+  int operator== (H, int)
+  {
+    return 66;
+  }
+}
+
+namespace ALIAS = I;
+
+//------------------
+
+class J
+{
+};
+
+namespace K
+{
+  int i;
+  int operator== (J, int)
+  {
+    return 77;
+  }
+}
+
+using K::i;
+
+//------------------
+
+class L
+{
+};
+namespace M
+{
+  int operator== (L, int)
+  {
+    return 88;
+  }
+}
+
+namespace N
+{
+  using namespace M;
+}
+
+using namespace N;
+
+//------------------
+
+int main ()
+{
+  A a;
+  a == 1;
+  a == 'a';
+
+  B::C bc;
+  bc == 1;
+  bc == 'a';
+  B::BD::operator== (bc,'a');
+
+  D d;
+  d == 1;
+  d == 'a';
+  d == 1.0f;
+
+  E e;
+  e == 1;
+  e == 'a';
+  e == 1.0f;
+
+  G g;
+  g == 1;
+  g == 'a';
+
+  H h;
+  I::operator== (h, 1);
+
+  J j;
+  K::operator== (j, 1);
+
+  L l;
+  l == 1;
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.cp/operator.exp b/gdb/testsuite/gdb.cp/operator.exp
new file mode 100644
index 0000000..ac89d2b
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/operator.exp
@@ -0,0 +1,58 @@
+# 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/>.
+
+set testfile operator
+set srcfile ${testfile}.cc
+if { [prepare_for_testing ${testfile}.exp ${testfile} ${srcfile} {debug c++}] } {
+    return -1
+}
+
+############################################
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+# Test global operator
+gdb_test "p a == 1" "= 11" "global operator"
+gdb_test "p a == 'a'" "= 12" "global operator overload"
+
+# Test ADL operator
+gdb_test "p bc == 1" "= 22" "ADL operator"
+gdb_test "p bc == 'a'" "= 23" "ADL operator overload"
+gdb_test "p B::BD::operator== (bc,'a')" "= 24" "Fully qualified explicit operator call"
+
+# Test operator imported from anonymous namespace
+gdb_test "p d == 1" "= 33" "anonymous namespace operator"
+gdb_test "p d == 'a'" "= 34" "anonymous namespace operator overload"
+gdb_test "p d == 1.0f" "= 35" "anonymous namespace operator overload float"
+
+# Test operator imported by using directive
+gdb_test "p e == 1" "= 44" "imported operator"
+gdb_test "p e == 'a'" "= 45" "imported operator overload"
+gdb_test "p e == 1.0f" "= 46" "imported operator overload float"
+
+# Test member operator
+gdb_test "p g == 1" "= 55" "member operator"
+gdb_test "p g == 'a'" "= 56" "member operator overload"
+
+# Test that operators are not wrongly imported
+# by import declarations and namespace aliases
+gdb_test "p h == 1" "Cannot resolve function operator== to any overloaded instance" "namespace alias"
+gdb_test "p j == 1" "Cannot resolve function operator== to any overloaded instance" "imported declaration"
+
+# Test that indirectly imported operators work
+gdb_test "p l == 1" "= 88"
diff --git a/gdb/valarith.c b/gdb/valarith.c
index 4efe936..fe6cae6 100644
--- a/gdb/valarith.c
+++ b/gdb/valarith.c
@@ -31,6 +31,7 @@
 #include "dfp.h"
 #include <math.h>
 #include "infcall.h"
+#include "exceptions.h"
 
 /* Define whether or not the C operator '/' truncates towards zero for
    differently signed operands (truncation direction is undefined in C). */
@@ -318,6 +319,67 @@ unop_user_defined_p (enum exp_opcode op, struct value *arg1)
     }
 }
 
+/* Try to find an operator named OPERATOR which takes NARGS arguments
+   specified in ARGS.  If the operator found is a static member operator
+   *STATIC_MEMFUNP will be set to 1, and otherwise 0.
+   The search if performed through find_overload_match which will handle
+   member operators, non member operators, operators imported implicitly or
+   explicitly, and perform correct overload resolution in all of the above
+   situations or combinations thereof.  */
+
+static struct value *
+value_user_defined_cpp_op (struct value **args, int nargs, char *operator,
+                           int *static_memfuncp)
+{
+
+  struct symbol *symp = NULL;
+  struct value *valp = NULL;
+  struct type **arg_types;
+  int i;
+
+  arg_types = (struct type **) alloca (nargs * (sizeof (struct type *)));
+  /* Prepare list of argument types for overload resolution */
+  for (i = 0; i < nargs; i++)
+    arg_types[i] = value_type (args[i]);
+
+  find_overload_match (arg_types, nargs, operator, BOTH /* could be method */,
+                       0 /* strict match */, &args[0], /* objp */
+                       NULL /* pass NULL symbol since symbol is unknown */,
+                       &valp, &symp, static_memfuncp, 0);
+
+  if (valp)
+    return valp;
+
+  if (symp)
+    {
+      /* This is a non member function and does not
+         expect a reference as its first argument
+         rather the explicit structure.  */
+      args[0] = value_ind (args[0]);
+      return value_of_variable (symp, 0);
+    }
+
+  error (_("Could not find %s."), operator);
+}
+
+/* Lookup user defined operator NAME.  Return a value representing the
+   function, otherwise return NULL.  */
+
+static struct value *
+value_user_defined_op (struct value **argp, struct value **args, char *name,
+                       int *static_memfuncp, int nargs)
+{
+  struct value *result = NULL;
+
+  if (current_language->la_language == language_cplus)
+    result = value_user_defined_cpp_op (args, nargs, name, static_memfuncp);
+  else
+    result = value_struct_elt (argp, args, name, static_memfuncp,
+                               "structure");
+
+  return result;
+}
+
 /* We know either arg1 or arg2 is a structure, so try to find the right
    user defined function.  Create an argument vector that calls 
    arg1.operator @ (arg1,arg2) and return that value (where '@' is any
@@ -458,7 +520,8 @@ value_x_binop (struct value *arg1, struct value *arg2, enum exp_opcode op,
       error (_("Invalid binary operation specified."));
     }
 
-  argvec[0] = value_struct_elt (&arg1, argvec + 1, tstr, &static_memfuncp, "structure");
+  argvec[0] = value_user_defined_op (&arg1, argvec + 1, tstr,
+                                     &static_memfuncp, 2);
 
   if (argvec[0])
     {
@@ -555,7 +618,8 @@ value_x_unop (struct value *arg1, enum exp_opcode op, enum noside noside)
       error (_("Invalid unary operation specified."));
     }
 
-  argvec[0] = value_struct_elt (&arg1, argvec + 1, tstr, &static_memfuncp, "structure");
+  argvec[0] = value_user_defined_op (&arg1, argvec + 1, tstr,
+                                     &static_memfuncp, nargs);
 
   if (argvec[0])
     {
diff --git a/gdb/valops.c b/gdb/valops.c
index 6f5f684..6648bdb 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2293,6 +2293,16 @@ value_find_oload_method_list (struct value **argp, const char *method,
    matches on the argument types according to the overload resolution
    rules.
 
+   METHOD can be one of three values:
+     NON_METHOD for non-member functions.
+     METHOD: for member functions.
+     BOTH: used for overload resolution of operators where the
+       candidates are expected to be either member or non member
+       functions. In this case the first argument ARGTYPES
+       (representing 'this') is expected to be a reference to the
+       target object, and will be dereferenced when attempting the
+       non-member search.
+
    In the case of class methods, the parameter OBJ is an object value
    in which to search for overloaded methods.
 
@@ -2320,16 +2330,20 @@ value_find_oload_method_list (struct value **argp, const char *method,
 
 int
 find_overload_match (struct type **arg_types, int nargs, 
-		     const char *name, int method, int lax, 
-		     struct value **objp, struct symbol *fsym,
+		     const char *name, enum oload_search_type method,
+		     int lax, struct value **objp, struct symbol *fsym,
 		     struct value **valp, struct symbol **symp, 
 		     int *staticp, const int no_adl)
 {
   struct value *obj = (objp ? *objp : NULL);
   /* Index of best overloaded function.  */
-  int oload_champ;
+  int func_oload_champ = -1;
+  int method_oload_champ = -1;
+
   /* The measure for the current best match.  */
-  struct badness_vector *oload_champ_bv = NULL;
+  struct badness_vector *method_badness = NULL;
+  struct badness_vector *func_badness = NULL;
+
   struct value *temp = obj;
   /* For methods, the list of overloaded methods.  */
   struct fn_field *fns_ptr = NULL;
@@ -2345,9 +2359,11 @@ find_overload_match (struct type **arg_types, int nargs,
   const char *obj_type_name = NULL;
   const char *func_name = NULL;
   enum oload_classification match_quality;
+  enum oload_classification method_match_quality = INCOMPATIBLE;
+  enum oload_classification func_match_quality = INCOMPATIBLE;
 
   /* Get the list of overloaded methods or functions.  */
-  if (method)
+  if (method == METHOD || method == BOTH)
     {
       gdb_assert (obj);
 
@@ -2370,10 +2386,13 @@ find_overload_match (struct type **arg_types, int nargs,
 	    }
 	}
 
+      /* Retrieve the list of methods with the name NAME.  */
       fns_ptr = value_find_oload_method_list (&temp, name, 
 					      0, &num_fns, 
 					      &basetype, &boffset);
-      if (!fns_ptr || !num_fns)
+      /* If this is a method only search, and no methods were found
+         the search has faild.  */
+      if (method == METHOD && (!fns_ptr || !num_fns))
 	error (_("Couldn't find method %s%s%s"),
 	       obj_type_name,
 	       (obj_type_name && *obj_type_name) ? "::" : "",
@@ -2381,15 +2400,33 @@ find_overload_match (struct type **arg_types, int nargs,
       /* If we are dealing with stub method types, they should have
 	 been resolved by find_method_list via
 	 value_find_oload_method_list above.  */
-      gdb_assert (TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL);
-      oload_champ = find_oload_champ (arg_types, nargs, method, 
-				      num_fns, fns_ptr, 
-				      oload_syms, &oload_champ_bv);
+      if (fns_ptr)
+	{
+	  gdb_assert (TYPE_DOMAIN_TYPE (fns_ptr[0].type) != NULL);
+	  method_oload_champ = find_oload_champ (arg_types, nargs, method,
+	                                         num_fns, fns_ptr,
+	                                         oload_syms, &method_badness);
+
+	  method_match_quality =
+	      classify_oload_match (method_badness, nargs,
+	                            oload_method_static (method, fns_ptr,
+	                                                 method_oload_champ));
+
+	  make_cleanup (xfree, method_badness);
+	}
+
     }
-  else
+
+  if (method == NON_METHOD || method == BOTH)
     {
       const char *qualified_name = NULL;
 
+      /* If the the overload match is being search for both
+         as a method and non member function, the first argument
+         must now be dereferenced.  */
+      if (method == BOTH)
+	arg_types[0] = TYPE_TARGET_TYPE (arg_types[0]);
+
       if (fsym)
         {
           qualified_name = SYMBOL_NATURAL_NAME (fsym);
@@ -2432,30 +2469,67 @@ find_overload_match (struct type **arg_types, int nargs,
           return 0;
         }
 
-      make_cleanup (xfree, oload_syms);
-      make_cleanup (xfree, oload_champ_bv);
+      func_oload_champ = find_oload_champ_namespace (arg_types, nargs,
+                                                     func_name,
+                                                     qualified_name,
+                                                     &oload_syms,
+                                                     &func_badness,
+                                                     no_adl);
 
-      oload_champ = find_oload_champ_namespace (arg_types, nargs,
-						func_name,
-						qualified_name,
-						&oload_syms,
-						&oload_champ_bv,
-						no_adl);
+      if (func_oload_champ >= 0)
+	func_match_quality = classify_oload_match (func_badness, nargs, 0);
+
+      make_cleanup (xfree, oload_syms);
+      make_cleanup (xfree, func_badness);
     }
 
   /* Did we find a match ?  */
-  if (oload_champ == -1)
+  if (method_oload_champ == -1 && func_oload_champ == -1)
     error (_("No symbol \"%s\" in current context."), name);
 
-  /* Check how bad the best match is.  */
-  match_quality =
-    classify_oload_match (oload_champ_bv, nargs,
-			  oload_method_static (method, fns_ptr,
-					       oload_champ));
+  /* If we have found both a method match and a function
+     match, find out which one is better, and calculate match
+     quality.  */
+  if (method_oload_champ >= 0 && func_oload_champ >= 0)
+    {
+      switch (compare_badness (func_badness, method_badness))
+        {
+	  case 0: /* Top two contenders are equally good.  */
+	    /* FIXME: GDB does not support the general ambiguous
+	     case.  All candidates should be collected and presented
+	     the the user.  */
+	    error (_("Ambiguous overload resolution"));
+	    break;
+	  case 1: /* Incomparable top contenders.  */
+	    /* This is an error incompatible candidates
+	       should not have been proposed.  */
+	    error (_("Internal error: incompatible overload candidates proposed"));
+	    break;
+	  case 2: /* Function champion.  */
+	    method_oload_champ = -1;
+	    match_quality = func_match_quality;
+	    break;
+	  case 3: /* Method champion.  */
+	    func_oload_champ = -1;
+	    match_quality = method_match_quality;
+	    break;
+	  default:
+	    error (_("Internal error: unexpected overload comparison result"));
+	    break;
+        }
+    }
+  else
+    {
+      /* We have either a method match or a function match.  */
+      if (method_oload_champ >= 0)
+	match_quality = method_match_quality;
+      else
+	match_quality = func_match_quality;
+    }
 
   if (match_quality == INCOMPATIBLE)
     {
-      if (method)
+      if (method == METHOD)
 	error (_("Cannot resolve method %s%s%s to any overloaded instance"),
 	       obj_type_name,
 	       (obj_type_name && *obj_type_name) ? "::" : "",
@@ -2466,7 +2540,7 @@ find_overload_match (struct type **arg_types, int nargs,
     }
   else if (match_quality == NON_STANDARD)
     {
-      if (method)
+      if (method == METHOD)
 	warning (_("Using non-standard conversion to match method %s%s%s to supplied arguments"),
 		 obj_type_name,
 		 (obj_type_name && *obj_type_name) ? "::" : "",
@@ -2476,21 +2550,20 @@ find_overload_match (struct type **arg_types, int nargs,
 		 func_name);
     }
 
-  if (method)
+  if (staticp != NULL)
+    *staticp = oload_method_static (method, fns_ptr, method_oload_champ);
+
+  if (method_oload_champ >= 0)
     {
-      if (staticp != NULL)
-	*staticp = oload_method_static (method, fns_ptr, oload_champ);
-      if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, oload_champ))
-	*valp = value_virtual_fn_field (&temp, fns_ptr, oload_champ, 
+      if (TYPE_FN_FIELD_VIRTUAL_P (fns_ptr, method_oload_champ))
+	*valp = value_virtual_fn_field (&temp, fns_ptr, method_oload_champ,
 					basetype, boffset);
       else
-	*valp = value_fn_field (&temp, fns_ptr, oload_champ, 
+	*valp = value_fn_field (&temp, fns_ptr, method_oload_champ,
 				basetype, boffset);
     }
   else
-    {
-      *symp = oload_syms[oload_champ];
-    }
+    *symp = oload_syms[func_oload_champ];
 
   if (objp)
     {
@@ -2778,7 +2851,8 @@ find_oload_champ (struct type **arg_types, int nargs, int method,
 static int
 oload_method_static (int method, struct fn_field *fns_ptr, int index)
 {
-  if (method && TYPE_FN_FIELD_STATIC_P (fns_ptr, index))
+  if (method && fns_ptr && index >= 0
+      && TYPE_FN_FIELD_STATIC_P (fns_ptr, index))
     return 1;
   else
     return 0;
diff --git a/gdb/value.h b/gdb/value.h
index 57b4dd7..7f71dc4 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -447,8 +447,11 @@ extern struct fn_field *value_find_oload_method_list (struct value **,
 						      int, int *,
 						      struct type **, int *);
 
+enum oload_search_type { NON_METHOD, METHOD, BOTH };
+
 extern int find_overload_match (struct type **arg_types, int nargs,
-				const char *name, int method, int lax,
+				const char *name,
+				enum oload_search_type method, int lax,
 				struct value **objp, struct symbol *fsym,
 				struct value **valp, struct symbol **symp,
 				int *staticp, const int no_adl);

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] Test and support all cpp operator types
  2010-05-13  0:00 ` Tom Tromey
  2010-05-18 20:49   ` [PATCH 1/2] " sami wagiaalla
@ 2010-05-18 22:31   ` sami wagiaalla
  2010-06-04 20:59     ` Tom Tromey
  1 sibling, 1 reply; 6+ messages in thread
From: sami wagiaalla @ 2010-05-18 22:31 UTC (permalink / raw)
  To: tromey; +Cc: gdb-patches

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

On 05/12/2010 06:21 PM, Tom Tromey wrote:
>>>>>> "Sami" == sami wagiaalla<swagiaal@redhat.com>  writes:
>
> Sami>  This patch adds testing and support for the following types of operator
> Sami>  lookup:
> Sami>   - non-member operators.
> Sami>   - imported operators (using directive, anonymous namespaces).
> Sami>   - ADL operators.
>
> I like the general approach of this patch.
>
> I have a few comments.
>
> Sami>  @@ -800,21 +800,27 @@ make_symbol_overload_list_using (const char *func_name,
> Sami>   				const char *namespace)
> [...]
> Sami>  -  for (current = block_using (get_selected_block (0));
> Sami>  -       current != NULL;
> Sami>  -       current = current->next)
> Sami>  -    {
> Sami>  -      if (strcmp (namespace, current->import_dest) == 0)
> Sami>  -	{
> Sami>  -	  make_symbol_overload_list_using (func_name,
> Sami>  -					   current->import_src);
> Sami>  -	}
> Sami>  -    }
> Sami>  +  for (block = get_selected_block (0);
> Sami>  +       block != NULL;
> Sami>  +       block = BLOCK_SUPERBLOCK(block))
> Sami>  +    for (current = block_using (block);
> Sami>  +	current != NULL;
> Sami>  +	current = current->next)
> Sami>  +      {
> [...]
>
> This part seems a little weird to me.
> make_symbol_overload_list_using calls make_symbol_overload_list_namespace,
> which calls make_symbol_overload_list_qualified, which itself
> starts with get_selected_block and iterates over the superblocks.
>
> It seems to me that only one such iteration should be needed.
> I don't have a test case but it seems like this could cause incorrect
> results in some corner case.
>

This patch fixes the above. It assumes that namespace names will belong 
to the static and global blocks. This enables us to separate the 
non-namespace iterative search from the namespace search.

[-- Attachment #2: search-cleanup.patch --]
[-- Type: text/plain, Size: 4386 bytes --]


    2010-05-18  Sami Wagiaalla  <swagiaal@redhat.com>
    
    	* cp-support.c (make_symbol_overload_list_namespace): Only search
    	static and global blocks.
    	(make_symbol_overload_list_block): New function.
    	(make_symbol_overload_list): Separate namespace search from block
    	search.
    	(make_symbol_overload_list_qualified): Use
    	make_symbol_overload_list_block.

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 0a0f777..e7ab6c5 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -693,6 +693,7 @@ make_symbol_overload_list (const char *func_name,
 			   const char *namespace)
 {
   struct cleanup *old_cleanups;
+  const char *name;
 
   sym_return_val_size = 100;
   sym_return_val_index = 0;
@@ -704,20 +705,54 @@ make_symbol_overload_list (const char *func_name,
 
   make_symbol_overload_list_using (func_name, namespace);
 
+  if (namespace[0] == '\0')
+    name = func_name;
+  else
+    {
+      char *concatenated_name
+	= alloca (strlen (namespace) + 2 + strlen (func_name) + 1);
+      strcpy (concatenated_name, namespace);
+      strcat (concatenated_name, "::");
+      strcat (concatenated_name, func_name);
+      name = concatenated_name;
+    }
+
+  make_symbol_overload_list_qualified (name);
+
   discard_cleanups (old_cleanups);
 
   return sym_return_val;
 }
 
+/* Add all symbols with a name matching NAME in BLOCK to the overload
+   list.  */
+
+static void
+make_symbol_overload_list_block (const char *name,
+                                 const struct block *block)
+{
+  struct dict_iterator iter;
+  struct symbol *sym;
+
+  const struct dictionary *dict = BLOCK_DICT (block);
+
+  for (sym = dict_iter_name_first (dict, name, &iter);
+       sym != NULL;
+       sym = dict_iter_name_next (name, &iter))
+    overload_list_add_symbol (sym, name);
+}
+
 /* Adds the function FUNC_NAME from NAMESPACE to the overload set.  */
 
 static void
 make_symbol_overload_list_namespace (const char *func_name,
                                      const char *namespace)
 {
+  const char *name;
+  const struct block *block = NULL;
 
   if (namespace[0] == '\0')
-    make_symbol_overload_list_qualified (func_name);
+    name = func_name;
   else
     {
       char *concatenated_name
@@ -725,8 +760,17 @@ make_symbol_overload_list_namespace (const char *func_name,
       strcpy (concatenated_name, namespace);
       strcat (concatenated_name, "::");
       strcat (concatenated_name, func_name);
-      make_symbol_overload_list_qualified (concatenated_name);
+      name = concatenated_name;
     }
+
+  /* look in the static block.  */
+  block = block_static_block (get_selected_block (0));
+  make_symbol_overload_list_block (name, block);
+
+  /* look in the global block.  */
+  block = block_global_block (block);
+  make_symbol_overload_list_block (name, block);
+
 }
 
 /* Search the namespace of the given type and namespace of and public base
@@ -852,16 +896,7 @@ make_symbol_overload_list_qualified (const char *func_name)
      complete on local vars.  */
 
   for (b = get_selected_block (0); b != NULL; b = BLOCK_SUPERBLOCK (b))
-    {
-      dict = BLOCK_DICT (b);
-
-      for (sym = dict_iter_name_first (dict, func_name, &iter);
-	   sym;
-	   sym = dict_iter_name_next (func_name, &iter))
-	{
-	  overload_list_add_symbol (sym, func_name);
-	}
-    }
+    make_symbol_overload_list_block (func_name, b);
 
   surrounding_static_block = block_static_block (get_selected_block (0));
 
@@ -872,14 +907,7 @@ make_symbol_overload_list_qualified (const char *func_name)
   {
     QUIT;
     b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
-    dict = BLOCK_DICT (b);
-
-    for (sym = dict_iter_name_first (dict, func_name, &iter);
-	 sym;
-	 sym = dict_iter_name_next (func_name, &iter))
-    {
-      overload_list_add_symbol (sym, func_name);
-    }
+    make_symbol_overload_list_block (func_name, b);
   }
 
   ALL_PRIMARY_SYMTABS (objfile, s)
@@ -889,14 +917,7 @@ make_symbol_overload_list_qualified (const char *func_name)
     /* Don't do this block twice.  */
     if (b == surrounding_static_block)
       continue;
-    dict = BLOCK_DICT (b);
-
-    for (sym = dict_iter_name_first (dict, func_name, &iter);
-	 sym;
-	 sym = dict_iter_name_next (func_name, &iter))
-    {
-      overload_list_add_symbol (sym, func_name);
-    }
+    make_symbol_overload_list_block (func_name, b);
   }
 }
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] Test and support all cpp operator types
  2010-05-18 20:49   ` [PATCH 1/2] " sami wagiaalla
@ 2010-06-04 20:55     ` Tom Tromey
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2010-06-04 20:55 UTC (permalink / raw)
  To: sami wagiaalla; +Cc: gdb-patches

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

Sami> It is weird but it is due to the nature of the symbol tables. The
Sami> first iteration, by make_symbol_overload_list_using, is iteration over
Sami> the blocks' import statements. The Second iteration is actually
Sami> searching for the symbol.

Sami> If we assume that names belonging to namespaces can only be in the
Sami> static scope -since namespace symbols are flattened to the nearest
Sami> parent- then we can separate the qualified and unqualified searches
Sami> and eliminate the nesting of the iterations.

Sami> Please see patch 2/2.

Thanks.

Sami>     2010-05-18  Sami Wagiaalla  <swagiaal@redhat.com>
Sami>     	* value.h: Created oload_search_type enum.
Sami>     	(find_overload_match): Use oload_search_type enum.
Sami>     	* valops.c (find_overload_match): Support combined member and
Sami>     	non-member search.
Sami>     	* eval.c (evaluate_subexp_standard): Calls to
Sami>     	find_overload_match now use oload_search_type enum.
Sami>     	(oload_method_static): Verify index is a proper value.
Sami>     	* valarith.c (value_user_defined_cpp_op): Search for and handle
Sami>     	both member and non-member operators.
Sami>     	(value_user_defined_cpp_op): New function.
Sami>     	(value_user_defined_op): New function.
Sami>     	(value_x_unop): Use value_user_defined_op.
Sami>     	(value_x_binop): Ditto.
Sami>     	* cp-support.c (make_symbol_overload_list_using): Added block
Sami>     	iteration.
Sami>     	Add check for namespace aliases and imported declarations.
>     2010-05-18  Sami Wagiaalla  <swagiaal@redhat.com>
Sami>     	* gdb.cp/koenig.exp: Test for ADL operators.
Sami>     	* gdb.cp/koenig.cc: Added ADL operators.
Sami>     	* gdb.cp/operator.exp: New test.
Sami>     	* gdb.cp/operator.cc: New test.

This is ok.  Thanks again.

Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] Test and support all cpp operator types
  2010-05-18 22:31   ` [PATCH 2/2] " sami wagiaalla
@ 2010-06-04 20:59     ` Tom Tromey
  0 siblings, 0 replies; 6+ messages in thread
From: Tom Tromey @ 2010-06-04 20:59 UTC (permalink / raw)
  To: sami wagiaalla; +Cc: gdb-patches

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

Sami> This patch fixes the above. It assumes that namespace names will
Sami> belong to the static and global blocks. This enables us to separate
Sami> the non-namespace iterative search from the namespace search.

Great, thanks.

One nit.

Sami> +  /* look in the static block.  */
Sami> +  block = block_static_block (get_selected_block (0));
Sami> +  make_symbol_overload_list_block (name, block);

Sami> +  /* look in the global block.  */

In both of these comments, the "L" in "look" should be capitalized.

Ok with this change.  Thanks again.

Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2010-06-04 20:59 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-05-11 20:38 [PATCH] Test and support all cpp operator types sami wagiaalla
2010-05-13  0:00 ` Tom Tromey
2010-05-18 20:49   ` [PATCH 1/2] " sami wagiaalla
2010-06-04 20:55     ` Tom Tromey
2010-05-18 22:31   ` [PATCH 2/2] " sami wagiaalla
2010-06-04 20:59     ` Tom Tromey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).