gdb: replace NULL terminated array with array_view

After the previous commit, this commit updates the value_struct_elt
function to take an array_view rather than a NULL terminated array of
values.

The requirement for a NULL terminated array of values actually stems
from typecmp, so the change from an array to array_view needs to be
propagated through to this function.

While making this change I noticed that this fixes another bug, in
value_x_binop and value_x_unop GDB creates an array of values which
doesn't have a NULL at the end.  An array_view of this array is passed
to value_user_defined_op, which then unpacks the array_view and passed
the raw array to value_struct_elt, but only if the language is not
C++.

As value_x_binop and value_x_unop can only request member functions
with the names of C++ operators, then most of the time, assuming the
inferior is not a C++ program, then GDB will not find a matching
member function with the call to value_struct_elt, and so typecmp will
never be called, and so, GDB will avoid undefined behaviour.

However, it is worth remembering that, when GDB's language is set to
"auto", the current language is selected based on the language of the
current compilation unit.  As C++ programs usually link against libc,
which is written in C, then, if the inferior is stopped in libc GDB
will set the language to C.  And so, it is possible that we will end
up using value_struct_elt to try and lookup, and match, a C++
operator.  If this occurs then GDB will experience undefined
behaviour.

I have extended the test added in the previous commit to also cover
this case.

Finally, this commit changes the API from passing around a pointer to
an array to passing around a pointer to an array_view.  The reason for
this is that we need to be able to distinguish between the cases where
we call value_struct_elt with no arguments, i.e. we are looking up a
struct member, but we either don't have the arguments we want to pass
yet, or we don't expect there to be any need for GDB to use the
argument types to resolve any overloading; and the second case where
we call value_struct_elt looking for a function that takes no
arguments, that is, the argument list is empty.

NOTE: While writing this I realise that if we pass an array_view at
all then it will always have at least one item in it, the `this'
pointer for the object we are planning to call the method on.  So we
could, I guess, pass an empty array_view to indicate the case where we
don't know anything about the arguments, and when the array_view is
length 1 or more, it means we do have the arguments.  However, though
we could do this, I don't think this would be better, the length 0 vs
length 1 difference seems a little too subtle, I think that there's a
better solution...

I think a better solution would be to wrap the array_view in a
gdb::optional, this would make the whole, do we have an array view or
not question explicit.

I haven't done this as part of this commit as making that change is
much more extensive, every user of value_struct_elt will need to be
updated, and as this commit already contains a bug fix, I wanted to
keep the large refactoring in a separate commit, so, check out the
next commit for the use of gdb::optional.

gdb/ChangeLog:

	PR gdb/27994
	* eval.c (structop_base_operation::evaluate_funcall): Pass
	array_view instead of array to value_struct_elt.
	* valarith.c (value_user_defined_op): Likewise.
	* valops.c (typecmp): Change parameter type from array pointer to
	array_view.  Update header comment, and update body accordingly.
	(search_struct_method): Likewise.
	(value_struct_elt): Likewise.
	* value.h (value_struct_elt): Update declaration.

gdb/testsuite/ChangeLog:

	PR gdb/27994
	* gdb.cp/method-call-in-c.cc (struct foo_type): Add operator+=,
	change initial value of var member variable.
	(main): Make use of foo_type's operator+=.
	* gdb.cp/method-call-in-c.exp: Test use of operator+=.

1 files changed, 8 insertions, 11 deletions

diff --git a/gdb/eval.c b/gdb/eval.c
index ab070a3..5a72bf1 100644
--- a/gdb/eval.c
+++ b/gdb/eval.c
@@ -872,9 +872,9 @@ structop_base_operation::evaluate_funcall
      (struct type *expect_type, struct expression *exp, enum noside noside,
       const std::vector<operation_up> &args)
 {
-  /* Allocate space for the function call arguments.  Include space for a
-     `this' pointer at the start, and a trailing nullptr.  */
-  std::vector<value *> vals (args.size () + 2);
+  /* Allocate space for the function call arguments, Including space for a
+     `this' pointer at the start.  */
+  std::vector<value *> vals (args.size () + 1);
   /* First, evaluate the structure into vals[0].  */
   enum exp_opcode op = opcode ();
   if (op == STRUCTOP_STRUCT)
@@ -920,16 +920,13 @@ structop_base_operation::evaluate_funcall
 	}
     }
 
-  /* Evaluate the arguments, and add the trailing nullptr.  The '+ 1' here
-     is to allow for the `this' pointer we placed into vals[0].  */
+  /* Evaluate the arguments.  The '+ 1' here is to allow for the `this'
+     pointer we placed into vals[0].  */
   for (int i = 0; i < args.size (); ++i)
     vals[i + 1] = args[i]->evaluate_with_coercion (exp, noside);
-  vals[args.size () + 1] = nullptr;
 
-  /* The array view includes the `this' pointer, but not the trailing
-     nullptr.  */
-  gdb::array_view<value *> arg_view
-    = gdb::make_array_view (&vals[0], args.size () + 1);
+  /* The array view includes the `this' pointer.  */
+  gdb::array_view<value *> arg_view (vals);
 
   int static_memfuncp;
   value *callee;
@@ -950,7 +947,7 @@ structop_base_operation::evaluate_funcall
     {
       struct value *temp = vals[0];
 
-      callee = value_struct_elt (&temp, &vals[0], tstr,
+      callee = value_struct_elt (&temp, &arg_view, tstr,
 				 &static_memfuncp,
 				 op == STRUCTOP_STRUCT
 				 ? "structure" : "structure pointer");