Re: [RFC] Koenig lookup patch 2

[Sami Wagiaalla <swagiaal@redhat.com>]

Tom Tromey wrote:
>>>>>> "Sami" == Sami Wagiaalla <swagiaal@redhat.com> writes:
> 
> Sami>     Recommit: Added support for ADL function lookup. Patch 2.
> 
> Thanks.
> 
> Sami>        * c-exp.y: Created token UNKOWN_NAME.
> 
> FYI, a typo in the ChangeLog -- missing an "N".
> 
> A couple quick notes.  I don't have time to pick nits... there were
> some formatting issues and whatnot -- these have to be fixed but are
> not very important overall.
> 
> Sami> @@ -2083,9 +2086,37 @@ find_overload_match (struct value **argvec, int nargs,
> 
> With the current code is there a need to have the values here?
> Or could this be reverted to the trunk's approach?
> 

This was my attempt of making the code a little more compact. It makes
sense that you should be able to perform an overload resolution with
types not values. It is just that this code:

/* 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]);

...is repeated before every call to the function.

I had also rolled the Koenig lookup into this loop in this function
but that is a cheat since Koenig is not really an overload.

> Sami> +  for (ix = 1; ix <= nargs; ix++){
> Sami>      arg_types[ix - 1] = value_type (argvec[ix]);
> 
> BTW, my earlier note about needing the formal types was in error.
> I think GDB really only deals in formal types, unless it does special
> work to find the dynamic type.
> 

Cool.

> Sami> +        if(cindex != NULL){
> Sami> +          prefix_len = (int)(cindex - type_name) - 1;
> Sami> +          prefix = alloca(prefix_len+1);
> Sami> +          strncpy(prefix, type_name, prefix_len);
> Sami> +          prefix[prefix_len] = '\0';
> Sami> +
> Sami> +          concatenated_name = alloca (strlen (prefix) + 1 + strlen (name) + 1);
> Sami> +          strcpy(concatenated_name, prefix);
> Sami> +          strcat(concatenated_name, "::");
> Sami> +          strcat(concatenated_name, name);
> 
> This could be reduced to a single allocation.
> 

I have eliminated the unneeded allocation and also found a
better way to figure out the prefix. Please see patch below.

> Sami> +          fsym = lookup_symbol(concatenated_name,NULL, VAR_DOMAIN, (int *) NULL);
> 
> Does this really do the right thing in the case where the call has
> multiple arguments, each of which has a type from a different
> namespace?  I don't understand how those would get added to the
> overload set.

The overload resolution is performed by the remainder of the function
(find_overloaded_match) this is just to give a starting point. However,
as you suspected, find_overload_match assumes that all the overload
choices are within the same namespace. This is corrected in this patch.

     Added support for ADL function lookup. Patch 3.

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

     	* valops.c (find_adl_match): 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..24b8481 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 dependet
+			     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,11 @@ yylex ()
      if (in_parse_field && *lexptr == '\0')
        saw_name_at_eof = 1;

+    if (sym == NULL && !lookup_minimal_symbol (tmp, NULL, NULL) && !is_a_field_of_this)
+      {
+        return UNKNOWN_NAME;
+      }
+
      return NAME;
    }
  }
diff --git a/gdb/eval.c b/gdb/eval.c
index 1d35571..8125594 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,29 @@ 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_adl_match (arg_types, nargs, func_name, &symp);
+
+          /* 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..8f3bd98 100644
--- a/gdb/testsuite/gdb.cp/namespace-koenig.cc
+++ b/gdb/testsuite/gdb.cp/namespace-koenig.cc
@@ -31,6 +31,27 @@ 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; }
+}
+
+//------------
  int
  main()
  {
@@ -42,5 +63,19 @@ main()
    second(0, 0, c, 0, 0);
    A::first(b.c);

-  return first(0, 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);
+
+  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..69df315 100644
--- a/gdb/testsuite/gdb.cp/namespace-koenig.exp
+++ b/gdb/testsuite/gdb.cp/namespace-koenig.exp
@@ -57,11 +57,10 @@ 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"
diff --git a/gdb/valops.c b/gdb/valops.c
index 202dcce..8a804b5 100644
--- a/gdb/valops.c
+++ b/gdb/valops.c
@@ -2021,6 +2021,42 @@ value_find_oload_method_list (struct value **argp, char *method,
  			   basetype, boffset);
  }

+/* Performs an argument defined (Koenig) lookup. */
+void
+find_adl_match (struct type **arg_types, int nargs,char *name,
+               struct symbol **symp)
+{
+  char *type_name;
+  int cindex;
+  int prefix_len;
+  char *concatenated_name;
+  int i;
+  struct symbol *fsym;
+
+  for (i = 1; i <= nargs; i++){
+    type_name = TYPE_NAME (arg_types[i - 1]);
+
+    prefix_len = cp_entire_prefix_len(type_name);
+
+    if( prefix_len != 0){
+
+      concatenated_name = alloca (prefix_len + 2 + strlen (name) + 1);
+      strncpy(concatenated_name, type_name, prefix_len);
+      concatenated_name[prefix_len] = '\0';
+      strcat(concatenated_name, "::");
+      strcat(concatenated_name, name);
+
+      fsym = lookup_symbol(concatenated_name,NULL, VAR_DOMAIN, (int *) NULL);
+
+      if ( fsym &&
+           find_overload_match(arg_types, nargs, name, 0,
+                              0, NULL, fsym,NULL, symp,NULL) == 0)
+        return;
+
+    }
+  }
+}
+
  /* Given an array of argument types (ARGTYPES) (which includes an
     entry for "this" in the case of C++ methods), the number of
     arguments NARGS, the NAME of a function whether it's a method or
diff --git a/gdb/value.h b/gdb/value.h
index aa43365..c37eb4d 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -444,6 +444,9 @@ extern struct fn_field *value_find_oload_method_list (struct value **, char *,
  						      int, int *,
  						      struct type **, int *);

+extern void find_adl_match (struct type **arg_types, int nargs, char *name,
+                           struct symbol **symp);
+
  extern int find_overload_match (struct type **arg_types, int nargs,
  				char *name, int method, int lax,
  				struct value **objp, struct symbol *fsym,