lookup_name_info::make_ignore_params

A few places in the completion code look for a "(" to find a
function's parameter list, in order to strip it, because psymtabs (and
gdb index) don't include parameter info in the symbol names.

See compare_symbol_name and
default_collect_symbol_completion_matches_break_on.

This is too naive.  Consider:

 ns_overload2_test::([TAB]

We'd want to complete that to:
 ns_overload2_test::(anonymous namespace)::struct_overload2_test

Or:

 b (anonymous namespace)::[TAB]

That currently completes to:

 b (anonymous namespace)

Which is obviously broken.  This patch makes that work.

Also, the current compare_symbol_name hack means that while this
works:

 "b function([TAB]"  ->  "b function()"

This does not:

 "b function ([TAB]"

This patch fixes that.  Whitespace "ignoring" now Just Works, i.e.,
assuming a symbol named "function(int, long)", this:
 b function (   int , lon[TAB]
completes to:
 b function (   int , long)

To address all of this, this patch builds on top of the rest of the
series, and pushes the responsibility of stripping parameters from a
lookup name to the new lookup_name_info object, where we can apply
per-language rules.  Also note that we now only make a version of the
lookup name with parameters stripped out where it's actually required
to do that, in the psymtab and GDB index code.

For C++, the right way to strip parameters is with "cp_remove_params",
which uses a real parser (cp-name-parser.y) to split the name into a
component tree and then discards parameters.

The trouble for completion is that in that case we have an incomplete
name, like "foo::func(int" and thus cp_remove_params throws an error.

This patch sorts that by adding a cp_remove_params_if_any variant of
cp_remove_params that tries removing characters from the end of the
string until cp_remove_params works.  So cp_remove_params_if_any
behaves like this:

With a complete name:

   "foo::func(int)"  => foo::func(int)  # cp_remove_params_1 succeeds the first time.

With an incomplete name:

   "foo::func(int"  => NULL             # cp_remove_params fails the first time.
   "foo::func(in"   => NULL             # and again...
   "foo::func(i"    => NULL             # and again...
   "foo::func("     => NULL             # and again...
   "foo::func"      => "foo::func"      # success!

Note that even if this approach removes significant rightmost
characters, it's still OK, because this parameter stripping is only
necessary for psymtabs and gdb index, where we're determining whether
to expand a symbol table.  Say cp_remove_params_if_any returned
"foo::" above for "foo::func(int".  That'd cause us to expand more
symtabs than ideal (because we'd expand all symtabs with symbols that
start with "foo::", not just "foo::func"), but then when we actually
look for completion matches, we'd still use the original lookup name,
with parameter information ["foo::func(int"], and thus we'll return no
false positive to the user.  Whether the stripping works as intended
and doesn't strip too much is thus covered by a unit test instead of a
testsuite test.

The "if_any" part of the name refers to the fact that while
cp_remove_params returns NULL if the input name has no parameters in
the first place, like:

   "foo::func"      => NULL         # cp_remove_params

cp_remove_params_if_any still returns the function name:

   "foo::func"      => "foo::func"  # cp_remove_params_if_any

gdb/ChangeLog:
2017-11-08  Pedro Alves  <palves@redhat.com>

	* Makefile.in (SUBDIR_UNITTESTS_SRCS): Add
	unittests/lookup_name_info-selftests.c.
	(SUBDIR_UNITTESTS_OBS): Add lookup_name_info-selftests.o.
	* cp-support.c: Include "selftest.h".
	(cp_remove_params_1): Rename from cp_remove_params.  Add
	'require_param' parameter, and handle it.
	(cp_remove_params): Reimplement.
	(cp_remove_params_if_any): New.
	(selftests::quote): New.
	(selftests::check_remove_params): New.
	(selftests::test_cp_remove_params): New.
	(_initialize_cp_support): Install
	selftests::test_cp_remove_params.
	* cp-support.h (cp_remove_params_if_any): Declare.
	* dwarf2read.c :Include "selftest.h".
	(dw2_expand_symtabs_matching_symbol): Use
	lookup_name_info::make_ignore_params.
	(selftests::dw2_expand_symtabs_matching::mock_mapped_index)
	(selftests::dw2_expand_symtabs_matching::string_or_null)
	(selftests::dw2_expand_symtabs_matching::check_match)
	(selftests::dw2_expand_symtabs_matching::test_symbols)
	(selftests::dw2_expand_symtabs_matching::run_test): New.
	(_initialize_dwarf2_read): Register
	selftests::dw2_expand_symtabs_matching::run_test.
	* psymtab.c (psym_expand_symtabs_matching): Use
	lookup_name_info::make_ignore_params.
	* symtab.c (demangle_for_lookup_info::demangle_for_lookup_info):
	If the lookup name wants to ignore parameters, strip them.
	(compare_symbol_name): Remove sym_text/sym_text_len parameters and
	code handling '('.
	(completion_list_add_name): Don't pass down sym_text/sym_text_len.
	(default_collect_symbol_completion_matches_break_on): Don't try to
	strip parameters.
	* symtab.h (lookup_name_info::lookup_name_info): Add
	'ignore_parameters' parameter.
	(lookup_name_info::ignore_parameters)
	(lookup_name_info::make_ignore_params): New methods.
	(lookup_name_info::m_ignore_parameters): New field.
	* unittests/lookup_name_info-selftests.c: New file.

1 files changed, 182 insertions, 5 deletions

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index defe509..1cab69b 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -36,6 +36,7 @@
 #include <signal.h>
 #include "gdb_setjmp.h"
 #include "safe-ctype.h"
+#include "selftest.h"
 
 #define d_left(dc) (dc)->u.s_binary.left
 #define d_right(dc) (dc)->u.s_binary.right
@@ -830,12 +831,14 @@ cp_func_name (const char *full_name)
   return ret.release ();
 }
 
-/* DEMANGLED_NAME is the name of a function, including parameters and
-   (optionally) a return type.  Return the name of the function without
-   parameters or return type, or NULL if we can not parse the name.  */
+/* Helper for cp_remove_params.  DEMANGLED_NAME is the name of a
+   function, including parameters and (optionally) a return type.
+   Return the name of the function without parameters or return type,
+   or NULL if we can not parse the name.  If REQUIRE_PARAMS is false,
+   then tolerate a non-existing or unbalanced parameter list.  */
 
-gdb::unique_xmalloc_ptr<char>
-cp_remove_params (const char *demangled_name)
+static gdb::unique_xmalloc_ptr<char>
+cp_remove_params_1 (const char *demangled_name, bool require_params)
 {
   bool done = false;
   struct demangle_component *ret_comp;
@@ -871,10 +874,56 @@ cp_remove_params (const char *demangled_name)
   /* What we have now should be a function.  Return its name.  */
   if (ret_comp->type == DEMANGLE_COMPONENT_TYPED_NAME)
     ret = cp_comp_to_string (d_left (ret_comp), 10);
+  else if (!require_params
+	   && (ret_comp->type == DEMANGLE_COMPONENT_NAME
+	       || ret_comp->type == DEMANGLE_COMPONENT_QUAL_NAME
+	       || ret_comp->type == DEMANGLE_COMPONENT_TEMPLATE))
+    ret = cp_comp_to_string (ret_comp, 10);
 
   return ret;
 }
 
+/* DEMANGLED_NAME is the name of a function, including parameters and
+   (optionally) a return type.  Return the name of the function
+   without parameters or return type, or NULL if we can not parse the
+   name.  */
+
+gdb::unique_xmalloc_ptr<char>
+cp_remove_params (const char *demangled_name)
+{
+  return cp_remove_params_1 (demangled_name, true);
+}
+
+/* See cp-support.h.  */
+
+gdb::unique_xmalloc_ptr<char>
+cp_remove_params_if_any (const char *demangled_name, bool completion_mode)
+{
+  /* Trying to remove parameters from the empty string fails.  If
+     we're completing / matching everything, avoid returning NULL
+     which would make callers interpret the result as an error.  */
+  if (demangled_name[0] == '\0' && completion_mode)
+    return gdb::unique_xmalloc_ptr<char> (xstrdup (""));
+
+  gdb::unique_xmalloc_ptr<char> without_params
+    = cp_remove_params_1 (demangled_name, false);
+
+  if (without_params == NULL && completion_mode)
+    {
+      std::string copy = demangled_name;
+
+      while (!copy.empty ())
+	{
+	  copy.pop_back ();
+	  without_params = cp_remove_params_1 (copy.c_str (), false);
+	  if (without_params != NULL)
+	    break;
+	}
+    }
+
+  return without_params;
+}
+
 /* Here are some random pieces of trivia to keep in mind while trying
    to take apart demangled names:
 
@@ -1600,6 +1649,129 @@ cp_get_symbol_name_matcher (const lookup_name_info &lookup_name)
   return cp_fq_symbol_name_matches;
 }
 
+#if GDB_SELF_TEST
+
+namespace selftests {
+
+/* If non-NULL, return STR wrapped in quotes.  Otherwise, return a
+   "<null>" string (with no quotes).  */
+
+static std::string
+quote (const char *str)
+{
+  if (str != NULL)
+    return std::string (1, '\"') + str + '\"';
+  else
+    return "<null>";
+}
+
+/* Check that removing parameter info out of NAME produces EXPECTED.
+   COMPLETION_MODE indicates whether we're testing normal and
+   completion mode.  FILE and LINE are used to provide better test
+   location information in case ithe check fails.  */
+
+static void
+check_remove_params (const char *file, int line,
+		      const char *name, const char *expected,
+		      bool completion_mode)
+{
+  gdb::unique_xmalloc_ptr<char> result
+    = cp_remove_params_if_any (name, completion_mode);
+
+  if ((expected == NULL) != (result == NULL)
+      || (expected != NULL
+	  && strcmp (result.get (), expected) != 0))
+    {
+      error (_("%s:%d: make-paramless self-test failed: (completion=%d) "
+	       "\"%s\" -> %s, expected %s"),
+	     file, line, completion_mode, name,
+	     quote (result.get ()).c_str (), quote (expected).c_str ());
+    }
+}
+
+/* Entry point for cp_remove_params unit tests.  */
+
+static void
+test_cp_remove_params ()
+{
+  /* Check that removing parameter info out of NAME produces EXPECTED.
+     Checks both normal and completion modes.  */
+#define CHECK(NAME, EXPECTED)						\
+  do									\
+    {									\
+      check_remove_params (__FILE__, __LINE__, NAME, EXPECTED, false);	\
+      check_remove_params (__FILE__, __LINE__, NAME, EXPECTED, true);	\
+    }									\
+  while (0)
+
+  /* Similar, but used when NAME is incomplete -- i.e., is has
+     unbalanced parentheses.  In this case, looking for the exact name
+     should fail / return empty.  */
+#define CHECK_INCOMPL(NAME, EXPECTED)					\
+  do									\
+    {									\
+      check_remove_params (__FILE__, __LINE__, NAME, NULL, false);	\
+      check_remove_params (__FILE__, __LINE__, NAME, EXPECTED, true);	\
+    }									\
+  while (0)
+
+  CHECK ("function()", "function");
+  CHECK_INCOMPL ("function(", "function");
+  CHECK ("function() const", "function");
+
+  CHECK ("(anonymous namespace)::A::B::C",
+	 "(anonymous namespace)::A::B::C");
+
+  CHECK ("A::(anonymous namespace)",
+	 "A::(anonymous namespace)");
+
+  CHECK_INCOMPL ("A::(anonymou", "A");
+
+  CHECK ("A::foo<int>()",
+	 "A::foo<int>");
+
+  CHECK_INCOMPL ("A::foo<int>(",
+		 "A::foo<int>");
+
+  CHECK ("A::foo<(anonymous namespace)::B>::func(int)",
+	 "A::foo<(anonymous namespace)::B>::func");
+
+  CHECK_INCOMPL ("A::foo<(anonymous namespace)::B>::func(in",
+		 "A::foo<(anonymous namespace)::B>::func");
+
+  CHECK_INCOMPL ("A::foo<(anonymous namespace)::B>::",
+		 "A::foo<(anonymous namespace)::B>");
+
+  CHECK_INCOMPL ("A::foo<(anonymous namespace)::B>:",
+		 "A::foo<(anonymous namespace)::B>");
+
+  CHECK ("A::foo<(anonymous namespace)::B>",
+	 "A::foo<(anonymous namespace)::B>");
+
+  CHECK_INCOMPL ("A::foo<(anonymous namespace)::B",
+		 "A::foo");
+
+  /* Shouldn't this parse?  Looks like a bug in
+     cp_demangled_name_to_comp.  See PR c++/22411.  */
+#if 0
+  CHECK ("A::foo<void(int)>::func(int)",
+	 "A::foo<void(int)>::func");
+#else
+  CHECK_INCOMPL ("A::foo<void(int)>::func(int)",
+		 "A::foo");
+#endif
+
+  CHECK_INCOMPL ("A::foo<void(int",
+		 "A::foo");
+
+#undef CHECK
+#undef CHECK_INCOMPL
+}
+
+} // namespace selftests
+
+#endif /* GDB_SELF_CHECK */
+
 /* Don't allow just "maintenance cplus".  */
 
 static  void
@@ -1682,4 +1854,9 @@ display the offending symbol."),
 			   &maintenance_set_cmdlist,
 			   &maintenance_show_cmdlist);
 #endif
+
+#if GDB_SELF_TEST
+  selftests::register_test ("cp_remove_params",
+			    selftests::test_cp_remove_params);
+#endif
 }