Re: [RFC] Koenig lookup patch 3

[Sami Wagiaalla <swagiaal at redhat dot com>]

> Tom> there were
> Tom> some formatting issues and whatnot -- these have to be fixed but are
> Tom> not very important overall.
>
> This comment still applies.  Please fix the formatting problems.
> I can list them if you want.

Please do. I fixed what I could find, I hope there aren't any any more.

As for comments regarding correctness of the lookup, I have change the 
code to just insert
the extra functions found through ADL into the overload set during the 
search of
the deepest namespace.

This patch does not implement all searches needed to perform complete 
ADL. Here is an
excerpt from the spec with supported functionality marked. I have 
supported the low hanging fruit, and will work on the medium hanging
fruit.

— If T is a fundamental type, its associated sets of namespaces and
  classes are both empty [supported :)].

— If T is a class type (including unions), its associated classes are:
  the class itself [supported]; the class of which it is a member[not
  supported], if any; and its direct and indirect base classes
  [supported]. Its associated namespaces are the namespaces in which its
  associated classes are defined.

— If T is an enumeration type, its associated namespace is the namespace
  in which it is defined [supported]. If it is class member , its
  associated class is the member’s class [supported]; else it has no
  associated class.

— If T is a pointer to U or an array of U, its associated namespaces and
  classes are those associated with U [supported].

— If T is a function type, its associated namespaces and classes are
  those associated with the function parameter types and those
  associated with the return type [not supported].

— If T is a pointer to a member function of a class X, its associated
  namespaces and classes are those associated with the function
  parameter types and return type, together with those associated with
  X [not supported].

— If T is a pointer to a data member of class X, its associated
  namespaces and classes are those associated with the member type
  together with those associated with X [not supported].

— If T is a class template specialization its associated namespaces and
  classes are the namespace in which the template is defined; for member
  templates, the member template’s class; the namespaces and classes
  associated with the types of the template arguments provided for
  template type parameters (excluding template template parameters); the
  namespaces in which any template template arguments are defined; and
  the classes in which any member templates used as template template
  arguments are defined. [ Note: non-type template arguments do not
  contribute to the set of associated namespaces. — end note ]
  [not supported :D]




Here is the patch:







2009-10-08  Sami Wagiaalla  <swagiaal@redhat.com>

	* cp-support.c (make_symbol_overload_list_namespace): New function.
	(make_symbol_overload_list_using): Moved namespace checking code to
	make_symbol_overload_list_namespace.
	(make_symbol_overload_list_adl): New function.
	* parse.c (operator_length_standard): Added length information for
	OP_ADL_FUNC.
	* expression.h: Added OP_ADL_FUNC.
	* c-exp.y: Created token UNKOWN_NAME.
	Created grammer rules for UNKOWN_NAME, and adl_function.
	* eval.c (evaluate_subexp_standard): Added handling for for OP_ADL_FUNC.



diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 5123042..d215a9c 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -184,6 +184,7 @@ static int parse_number (char *, int, int, YYSTYPE *);

 %token <sval> STRING
 %token <ssym> NAME /* BLOCKNAME defined below to give it higher 
precedence. */
+%token <ssym> UNKNOWN_NAME
 %token <voidval> COMPLETE
 %token <tsym> TYPENAME
 %type <sval> name string_exp
@@ -384,6 +385,30 @@ exp	:	exp '('
 			  write_exp_elt_opcode (OP_FUNCALL); }
 	;

+exp	:	adl_func '('
+			/* This is to save the value of arglist_len
+			   being accumulated by an outer function call.  */
+			{ start_arglist (); }
+		arglist ')'	%prec ARROW
+			{
+			  write_exp_elt_opcode (OP_FUNCALL);
+			  write_exp_elt_longcst ((LONGEST) end_arglist ());
+			  write_exp_elt_opcode (OP_FUNCALL);
+			}
+	;
+
+adl_func	:	UNKNOWN_NAME
+			{
+			  /* This could potentially be a an argument defined
+			     lookup function (Koenig).  */
+			  write_exp_elt_opcode (OP_ADL_FUNC);
+			  write_exp_elt_block (expression_context_block);
+			  write_exp_elt_sym (NULL); /* Place holder */
+			  write_exp_string ($1.stoken);
+			  write_exp_elt_opcode (OP_ADL_FUNC);
+			}
+		;
+
 lcurly	:	'{'
 			{ start_arglist (); }
 	;
@@ -795,7 +820,7 @@ variable:	name_not_typename
 			}
 	;

-space_identifier : '@' NAME
+space_identifier : '@' UNKNOWN_NAME
 		{ push_type_address_space (copy_name ($2.stoken));
 		  push_type (tp_space_identifier);
 		}
@@ -1091,10 +1116,12 @@ name	:	NAME { $$ = $1.stoken; }
 	|	BLOCKNAME { $$ = $1.stoken; }
 	|	TYPENAME { $$ = $1.stoken; }
 	|	NAME_OR_INT  { $$ = $1.stoken; }
+	|	UNKNOWN_NAME  { $$ = $1.stoken; }
 	;

 name_not_typename :	NAME
 	|	BLOCKNAME
+	|	UNKNOWN_NAME
 /* These would be useful if name_not_typename was useful, but it is just
    a fake for "variable", so these cause reduce/reduce conflicts because
    the parser can't tell whether NAME_OR_INT is a name_not_typename 
(=variable,
@@ -2016,6 +2043,9 @@ yylex ()
     if (in_parse_field && *lexptr == '\0')
       saw_name_at_eof = 1;

+    if (sym == NULL && !lookup_minimal_symbol (tmp, NULL, NULL))
+      return UNKNOWN_NAME;
+
     return NAME;
   }
 }
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index bf42636..f69bcd9 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -47,7 +47,7 @@ static void demangled_name_complaint (const char *name);

 /* Functions/variables related to overload resolution.  */

-static int sym_return_val_size;
+static int sym_return_val_size = -1;
 static int sym_return_val_index;
 static struct symbol **sym_return_val;

@@ -57,6 +57,12 @@ static void overload_list_add_symbol (struct symbol *sym,
 static void make_symbol_overload_list_using (const char *func_name,
 					     const char *namespace);

+static void make_symbol_overload_list_namespace (const char *func_name,
+                                                 const char *namespace);
+
+static void make_symbol_overload_list_adl_namespace (struct type *type,
+                                                     const char 
*func_name);
+
 static void make_symbol_overload_list_qualified (const char *func_name);

 static void read_in_psymtabs (const char *oload_name);
@@ -697,6 +703,59 @@ make_symbol_overload_list (const char *func_name,
   return sym_return_val;
 }

+/* Adds the the overload list overload candidates for FUNC_NAME found 
through
+   argument dependent lookup.  */
+
+struct symbol **
+make_symbol_overload_list_adl (struct type **arg_types, int nargs,
+                               const char *func_name )
+{
+  int i;
+
+  gdb_assert (sym_return_val_size != -1);
+
+  for (i = 1; i <= nargs; i++)
+    make_symbol_overload_list_adl_namespace (arg_types[i - 1], func_name );
+
+  return sym_return_val;
+}
+
+/* Search the namespace of the given type and namespace of and public base
+   types.  */
+static void
+make_symbol_overload_list_adl_namespace (struct type *type, const char 
*func_name )
+{
+  char* namespace;
+  char* type_name;
+  int i, prefix_len;
+
+  if (TYPE_CODE (type) == TYPE_CODE_PTR
+      || TYPE_CODE (type) == TYPE_CODE_REF)
+    return make_symbol_overload_list_adl_namespace(TYPE_TARGET_TYPE 
(type), func_name);
+
+  type_name = TYPE_NAME (type);
+
+  prefix_len = cp_entire_prefix_len(type_name);
+
+  if (prefix_len != 0)
+    {
+      namespace = alloca (prefix_len + 1);
+      strncpy(namespace, type_name, prefix_len);
+      namespace[prefix_len] = '\0';
+
+      make_symbol_overload_list_namespace (func_name, namespace);
+    }
+
+  /* Check public base type */
+  if (TYPE_CODE(type) == TYPE_CODE_CLASS)
+    for (i = 0; i < TYPE_N_BASECLASSES (type); i++)
+      {
+        if(BASETYPE_VIA_PUBLIC (type, i))
+          make_symbol_overload_list_adl_namespace (TYPE_BASECLASS(type, 
i), func_name );
+      }
+
+}
+
 /* This applies the using directives to add namespaces to search in,
    and then searches for overloads in all of those namespaces.  It
    adds the symbols found to sym_return_val.  Arguments are as in
@@ -704,7 +763,7 @@ make_symbol_overload_list (const char *func_name,

 static void
 make_symbol_overload_list_using (const char *func_name,
-				 const char *namespace)
+                                 const char *namespace)
 {
   const struct using_direct *current;

@@ -724,7 +783,16 @@ make_symbol_overload_list_using (const char *func_name,
     }

   /* Now, add names for this namespace.  */
-
+  make_symbol_overload_list_namespace (func_name, namespace);
+}
+
+/* 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)
+{
+
   if (namespace[0] == '\0')
     {
       make_symbol_overload_list_qualified (func_name);
diff --git a/gdb/cp-support.h b/gdb/cp-support.h
index 4ce85b5..0e4a378 100644
--- a/gdb/cp-support.h
+++ b/gdb/cp-support.h
@@ -84,6 +84,10 @@ extern char *cp_remove_params (const char 
*demangled_name);
 extern struct symbol **make_symbol_overload_list (const char *,
 						  const char *);

+extern struct symbol **make_symbol_overload_list_adl (struct type 
**arg_types,
+                                                      int nargs,
+                                                      const char 
*func_name);
+
 extern struct type *cp_lookup_rtti_type (const char *name,
 					 struct block *block);

diff --git a/gdb/eval.c b/gdb/eval.c
index 1d35571..bb07c03 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -704,6 +704,7 @@ evaluate_subexp_standard (struct type *expect_type,
       return value_from_decfloat (exp->elts[pc + 1].type,
 				  exp->elts[pc + 2].decfloatconst);

+    case OP_ADL_FUNC:
     case OP_VAR_VALUE:
       (*pos) += 3;
       if (noside == EVAL_SKIP)
@@ -1362,6 +1363,17 @@ evaluate_subexp_standard (struct type *expect_type,
 	  /* Now, say which argument to start evaluating from */
 	  tem = 2;
 	}
+      else if (op == OP_ADL_FUNC)
+        {
+          /* Save the function position and move pos so that the arguments
+             can be evaluated. */
+          int func_name_len;
+          save_pos1 = *pos;
+          tem = 1;
+
+          func_name_len = longest_to_int (exp->elts[save_pos1 + 
3].longconst);
+          (*pos) += 6 + BYTES_TO_EXP_ELEM (func_name_len + 1);
+        }
       else
 	{
 	  /* Non-method function call */
@@ -1393,6 +1405,32 @@ evaluate_subexp_standard (struct type *expect_type,
       /* signal end of arglist */
       argvec[tem] = 0;

+      if (op == OP_ADL_FUNC)
+        {
+          struct symbol *symp;
+          char *func_name;
+          int  name_len;
+          int string_pc = save_pos1 + 3;
+
+          name_len = longest_to_int (exp->elts[string_pc].longconst);
+          func_name = (char*) alloca (name_len+1);
+          strcpy (func_name, &exp->elts[string_pc + 1].string);
+
+          /* Prepare list of argument types for overload resolution */
+          arg_types = (struct type **) alloca (nargs * (sizeof (struct 
type *)));
+          for (ix = 1; ix <= nargs; ix++)
+            arg_types[ix - 1] = value_type (argvec[ix]);
+
+          find_overload_match (arg_types, nargs, func_name,
+                              0 /* not method */ , 0 /* strict match */ ,
+                              NULL, NULL /* pass NULL symbol to signal 
ADL lookup */ ,
+                              NULL, &symp, NULL);
+
+          /* Now fix the expression being evaluated */
+          exp->elts[save_pos1+2].symbol = symp;
+          argvec[0] = evaluate_subexp_with_coercion (exp, &save_pos1, 
noside);
+        }
+
       if (op == STRUCTOP_STRUCT || op == STRUCTOP_PTR)
 	{
 	  int static_memfuncp;
diff --git a/gdb/expprint.c b/gdb/expprint.c
index 89bae03..fd2e1ce 100644
--- a/gdb/expprint.c
+++ b/gdb/expprint.c
@@ -799,6 +799,8 @@ op_name_standard (enum exp_opcode opcode)
       return "OP_TYPE";
     case OP_LABELED:
       return "OP_LABELED";
+    case OP_ADL_FUNC:
+      return "OP_ADL_FUNC";
     }
 }

diff --git a/gdb/expression.h b/gdb/expression.h
index 12163e3..ec8df4d 100644
--- a/gdb/expression.h
+++ b/gdb/expression.h
@@ -334,6 +334,10 @@ enum exp_opcode
        Then comes another OP_DECFLOAT.  */
     OP_DECFLOAT,

+    /* OP_ADL_FUNC specifies that the argument is to be looked up in an
+       Argument Dependent manner (keonig lookup) */
+    OP_ADL_FUNC,
+
      /* First extension operator.  Individual language modules define
         extra operators they need as constants with values
         OP_LANGUAGE_SPECIFIC0 + k, for k >= 0, using a separate
diff --git a/gdb/parse.c b/gdb/parse.c
index eee1f8e..afa6a43 100644
--- a/gdb/parse.c
+++ b/gdb/parse.c
@@ -811,6 +811,13 @@ operator_length_standard (struct expression *expr, 
int endpos,
       args = 1;
       break;

+    case OP_ADL_FUNC:
+      oplen = longest_to_int (expr->elts[endpos - 2].longconst);
+      oplen = 4 + BYTES_TO_EXP_ELEM (oplen + 1);
+      oplen++;
+      oplen++;
+      break;
+
     case OP_LABELED:
     case STRUCTOP_STRUCT:
     case STRUCTOP_PTR:
diff --git a/gdb/testsuite/gdb.cp/namespace-koenig.cc 
b/gdb/testsuite/gdb.cp/namespace-koenig.cc
index fad833e..0c5140a 100644
--- a/gdb/testsuite/gdb.cp/namespace-koenig.cc
+++ b/gdb/testsuite/gdb.cp/namespace-koenig.cc
@@ -7,19 +7,19 @@ namespace A
   };

   int
-  first(C c)
+  first (C c)
   {
     return 11;
   }

   int
-  first(int a, C c)
+  first (int a, C c)
   {
     return 22;
   }

   int
-  second(int a, int b, C cc, int c, int d)
+  second (int a, int b, C cc, int c, int d)
   {
     return 33;
   }
@@ -31,16 +31,115 @@ struct B
   A::C c;
 };

+//------------
+
+namespace E
+{
+  class O{};
+  int foo (O o){return 1; }
+  int foo (O o, O o2){return 2; }
+  int foo (O o, O o2, int i){return 3; }
+}
+
+namespace F
+{
+  class O{};
+  int foo (       O fo, ::E::O eo){ return 4;}
+  int foo (int i, O fo, ::E::O eo){ return 5;}
+}
+
+namespace G
+{
+  class O{};
+  int foo (O go, ::F::O fo, ::E::O eo){ return 6; }
+}
+
+//------------
+
+namespace H
+{
+  class O{};
+  int foo (O){ return 7;}
+}
+
+namespace I
+{
+  class O: public H::O {};
+  class X: H::O{};
+}
+
+//------------
+
+namespace J
+{
+  union U{};
+  struct S{};
+  enum E{};
+
+  class A{
+  public:
+    class B{};
+  };
+
+  class C{};
+
+  int foo (U){ return 8;}
+  int foo (S){ return 9;}
+  int foo (E){ return 10;}
+  int foo (A::B){ return 11;}
+  int foo (A*){ return 12;}
+  int foo (A**){ return 13;}
+  int foo (C[]){ return 14;}
+
+}
+//------------
+
 int
-main()
+main ()
 {
   A::C c;
   B b;

-  A::first(c);
-  first(0, c);
-  second(0, 0, c, 0, 0);
-  A::first(b.c);
+  A::first (c);
+  first (0, c);
+  second (0, 0, c, 0, 0);
+  A::first (b.c);
+
+  E::O eo;
+  F::O fo;
+  G::O go;
+
+  foo (eo);
+  foo (eo, eo);
+  foo (eo, eo, 1);
+  foo (fo, eo);
+  foo (1  ,fo, eo);
+  foo (go, fo, eo);
+
+  I::O io;
+  I::X ix;
+
+  foo (io);
+//foo (ix);
+
+  J::U ju;
+  J::S js;
+  J::E je;
+  J::A::B jab;
+  J::A *jap;
+  J::A **japp;
+  J::C jca[3];
+
+  foo (ju);
+  foo (js);
+  foo (je);
+  foo (jab);
+  foo (jap);
+  foo (japp);
+  foo (jca);

-  return first(0, c);
+  return first (0, c) + foo (eo) +
+         foo (eo, eo) + foo (eo, eo, 1)  +
+         foo (fo, eo) + foo (1  ,fo, eo) +
+         foo (go, fo, eo);
 }
diff --git a/gdb/testsuite/gdb.cp/namespace-koenig.exp 
b/gdb/testsuite/gdb.cp/namespace-koenig.exp
index 060c8a5..caf432f 100644
--- a/gdb/testsuite/gdb.cp/namespace-koenig.exp
+++ b/gdb/testsuite/gdb.cp/namespace-koenig.exp
@@ -57,11 +57,31 @@ gdb_test "p first(0,c)" "= 22"
 # when the argument is an expression
 gdb_test "p first(b.c)" "= 11"

-
-
-
-
-
-
-
+# test that resolutions can be made across namespaces
+gdb_test "p foo(eo)"         "= 1"
+gdb_test "p foo(eo, eo)"     "= 2"
+gdb_test "p foo(eo, eo, 1)"  "= 3"
+gdb_test "p foo(fo, eo)"     "= 4"
+gdb_test "p foo(1 ,fo, eo)"  "= 5"
+gdb_test "p foo(go, fo, eo)" "= 6"
+
+#test that gdb fails gracefully
+gdb_test "p fake(eo)" "No symbol \"fake\" in current context."
+
+#test that namespaces of base classes are searched
+gdb_test "p foo(io)" "= 7"
+gdb_test "p foo(ix)" "No symbol \"foo\" in current context."
+
+#test for other types
+gdb_test "p foo(ju)" "= 8"
+gdb_test "p foo(js)" "= 9"
+gdb_test "p foo(je)" "= 10"
+
+#test for class members
+setup_xfail "*-*-*"
+gdb_test "p foo(jab)" "= 11"
+
+gdb_test "p foo(jap)" "= 12"
+gdb_test "p foo(japp)" "= 13"
+gdb_test "p foo(jca)" "= 14"

diff --git a/gdb/valops.c b/gdb/valops.c
index 202dcce..ace3fa7 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2108,12 +2108,25 @@ find_overload_match (struct type **arg_types, 
int nargs,
     }
   else
     {
-      const char *qualified_name = SYMBOL_CPLUS_DEMANGLED_NAME (fsym);
+      const char *qualified_name = NULL;

-      /* If we have a C++ name, try to extract just the function
-	 part.  */
-      if (qualified_name)
-	func_name = cp_func_name (qualified_name);
+      if (fsym)
+        {
+          qualified_name = SYMBOL_CPLUS_DEMANGLED_NAME (fsym);
+
+          /* If we have a C++ name, try to extract just the function
+             part.  */
+          if (qualified_name)
+            {
+              func_name = cp_func_name (qualified_name);
+              old_cleanups = make_cleanup (xfree, func_name);
+            }
+        }
+      else
+        {
+          func_name = name;
+          qualified_name = name;
+        }

       /* If there was no C++ name, this must be a C-style function.
 	 Just return the same symbol.  Do the same if cp_func_name
@@ -2124,7 +2137,6 @@ find_overload_match (struct type **arg_types, int 
nargs,
           return 0;
         }

-      old_cleanups = make_cleanup (xfree, func_name);
       make_cleanup (xfree, oload_syms);
       make_cleanup (xfree, oload_champ_bv);

@@ -2135,8 +2147,11 @@ find_overload_match (struct type **arg_types, int 
nargs,
 						&oload_champ_bv);
     }

-  /* Check how bad the best match is.  */
+  /* Did we find a match ?*/
+  if (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,
@@ -2302,6 +2317,12 @@ find_oload_champ_namespace_loop (struct type 
**arg_types, int nargs,
   new_namespace[namespace_len] = '\0';
   new_oload_syms = make_symbol_overload_list (func_name,
 					      new_namespace);
+
+  /* If we have reached the deepesst level perform argument
+     determined lookup.  */
+  if (!searched_deeper)
+      make_symbol_overload_list_adl(arg_types, nargs, func_name);
+
   while (new_oload_syms[num_fns])
     ++num_fns;

@@ -2334,7 +2355,6 @@ find_oload_champ_namespace_loop (struct type 
**arg_types, int nargs,
     }
   else
     {
-      gdb_assert (new_oload_champ != -1);
       *oload_syms = new_oload_syms;
       *oload_champ = new_oload_champ;
       *oload_champ_bv = new_oload_champ_bv;