Handle custom completion match prefix / LCD
authorPedro Alves <palves@redhat.com>
Wed, 29 Nov 2017 19:33:23 +0000 (19:33 +0000)
committerPedro Alves <palves@redhat.com>
Wed, 29 Nov 2017 19:33:23 +0000 (19:33 +0000)
A following patch will add support for wild matching for C++ symbols,
making completing on "b push_ba" on a C++ program complete to
std::vector<...>::push_back, std::string::push_back etc., like:

 (gdb) b push_ba[TAB]
 std::vector<...>::push_back(....)
 std::string<...>::push_back(....)

Currently, we compute the "lowest common denominator" between all
completion candidates (what the input line is adjusted to) as the
common prefix of all matches.  That's problematic with wild matching
as above, as then we'd end up with TAB changing the input line to
"b std::", losing the original input, like:

 (gdb) b push_ba[TAB]
 std::vector<...>::push_back(....)
 std::string<...>::push_back(....)
 (gdb) b std::

while obviously we'd want it to adjust itself to "b push_back(" instead:

 (gdb) b push_ba[TAB]
 std::vector<...>::push_back(....)
 std::string<...>::push_back(....)
 (gdb) b push_back(

This patch adds the core code necessary to support this, though
nothing really makes use of it yet in this patch.

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

* ada-lang.c (ada_lookup_name_info::matches): Change type of
parameter from completion_match to completion_match_result.
Adjust.
(do_wild_match, do_full_match, ada_symbol_name_matches): Likewise.
* completer.c (completion_tracker::maybe_add_completion): Add
match_for_lcd parameter and use it.
(completion_tracker::add_completion): Likewise.
* completer.h (class completion_match_for_lcd): New class.
(completion_match_result::match_for_lcd): New field.
(completion_match_result::set_match): New method.
(completion_tracker): Add comments.
(completion_tracker::add_completion): Add match_for_lcd parameter.
(completion_tracker::reset_completion_match_result): Reset
match_for_lcd too.
(completion_tracker::maybe_add_completion): Add match_for_lcd
parameter.
(completion_tracker::m_lowest_common_denominator_unique): Extend
comments.
* cp-support.c (cp_symbol_name_matches_1)
(cp_fq_symbol_name_matches): Change type of parameter from
completion_match to completion_match_result.  Adjust.
* language.c (default_symbol_name_matcher): Change type of
parameter from completion_match to completion_match_result.
Adjust.
* language.h (completion_match_for_lcd): Forward declare.
(default_symbol_name_matcher): Change type of parameter from
completion_match to completion_match_result.
* symtab.c (compare_symbol_name): Adjust.
(completion_list_add_name): Pass the match_for_lcd to the tracker.
* symtab.h (ada_lookup_name_info::matches): Change type of
parameter from completion_match to completion_match_result.
(symbol_name_matcher_ftype): Likewise, and update comments.

gdb/ChangeLog
gdb/ada-lang.c
gdb/completer.c
gdb/completer.h
gdb/cp-support.c
gdb/language.c
gdb/language.h
gdb/symtab.c
gdb/symtab.h

index c8f8e2671ac38100c3773c2caab288192d70bf51..0c9d089103184cdab9c3da1f920f2cef4296beb0 100644 (file)
@@ -1,3 +1,38 @@
+2017-11-29  Pedro Alves  <palves@redhat.com>
+
+       * ada-lang.c (ada_lookup_name_info::matches): Change type of
+       parameter from completion_match to completion_match_result.
+       Adjust.
+       (do_wild_match, do_full_match, ada_symbol_name_matches): Likewise.
+       * completer.c (completion_tracker::maybe_add_completion): Add
+       match_for_lcd parameter and use it.
+       (completion_tracker::add_completion): Likewise.
+       * completer.h (class completion_match_for_lcd): New class.
+       (completion_match_result::match_for_lcd): New field.
+       (completion_match_result::set_match): New method.
+       (completion_tracker): Add comments.
+       (completion_tracker::add_completion): Add match_for_lcd parameter.
+       (completion_tracker::reset_completion_match_result): Reset
+       match_for_lcd too.
+       (completion_tracker::maybe_add_completion): Add match_for_lcd
+       parameter.
+       (completion_tracker::m_lowest_common_denominator_unique): Extend
+       comments.
+       * cp-support.c (cp_symbol_name_matches_1)
+       (cp_fq_symbol_name_matches): Change type of parameter from
+       completion_match to completion_match_result.  Adjust.
+       * language.c (default_symbol_name_matcher): Change type of
+       parameter from completion_match to completion_match_result.
+       Adjust.
+       * language.h (completion_match_for_lcd): Forward declare.
+       (default_symbol_name_matcher): Change type of parameter from
+       completion_match to completion_match_result.
+       * symtab.c (compare_symbol_name): Adjust.
+       (completion_list_add_name): Pass the match_for_lcd to the tracker.
+       * symtab.h (ada_lookup_name_info::matches): Change type of
+       parameter from completion_match to completion_match_result.
+       (symbol_name_matcher_ftype): Likewise, and update comments.
+
 2017-11-29  Pedro Alves  <palves@redhat.com>
 
        * linespec.c (minsym_found, add_minsym): Use msymbol_is_function.
index 3265c211fba208272865e476cd7f5f3a42f6dfc9..38d1ce6cf64752e00ed96c3eb0ba58c546d039e9 100644 (file)
@@ -6357,7 +6357,7 @@ bool
 ada_lookup_name_info::matches
   (const char *sym_name,
    symbol_name_match_type match_type,
-   completion_match *comp_match) const
+   completion_match_result *comp_match_res) const
 {
   bool match = false;
   const char *text = m_encoded_name.c_str ();
@@ -6415,15 +6415,12 @@ ada_lookup_name_info::matches
   if (!match)
     return false;
 
-  if (comp_match != NULL)
+  if (comp_match_res != NULL)
     {
-      std::string &match_str = comp_match->storage ();
+      std::string &match_str = comp_match_res->match.storage ();
 
       if (!m_encoded_p)
-       {
-         match_str = ada_decode (sym_name);
-         comp_match->set_match (match_str.c_str ());
-       }
+       match_str = ada_decode (sym_name);
       else
        {
          if (m_verbatim_p)
@@ -6431,8 +6428,9 @@ ada_lookup_name_info::matches
          else
            match_str = sym_name;
 
-         comp_match->set_match (match_str.c_str ());
        }
+
+      comp_match_res->set_match (match_str.c_str ());
     }
 
   return true;
@@ -13925,7 +13923,7 @@ static const struct exp_descriptor ada_exp_descriptor = {
 static bool
 do_wild_match (const char *symbol_search_name,
               const lookup_name_info &lookup_name,
-              completion_match *match)
+              completion_match_result *comp_match_res)
 {
   return wild_match (symbol_search_name, ada_lookup_name (lookup_name));
 }
@@ -13935,7 +13933,7 @@ do_wild_match (const char *symbol_search_name,
 static bool
 do_full_match (const char *symbol_search_name,
               const lookup_name_info &lookup_name,
-              completion_match *match)
+              completion_match_result *comp_match_res)
 {
   return full_match (symbol_search_name, ada_lookup_name (lookup_name));
 }
@@ -14005,11 +14003,11 @@ ada_lookup_name_info::ada_lookup_name_info (const lookup_name_info &lookup_name)
 static bool
 ada_symbol_name_matches (const char *symbol_search_name,
                         const lookup_name_info &lookup_name,
-                        completion_match *match)
+                        completion_match_result *comp_match_res)
 {
   return lookup_name.ada ().matches (symbol_search_name,
                                     lookup_name.match_type (),
-                                    match);
+                                    comp_match_res);
 }
 
 /* Implement the "la_get_symbol_name_matcher" language_defn method for
index f9ece5913a217f94ef5b07854775cdaed64f5701..fd82b8603145bab694af27f11de35e3cd84893b1 100644 (file)
@@ -1502,7 +1502,9 @@ completion_tracker::~completion_tracker ()
 /* See completer.h.  */
 
 bool
-completion_tracker::maybe_add_completion (gdb::unique_xmalloc_ptr<char> name)
+completion_tracker::maybe_add_completion
+  (gdb::unique_xmalloc_ptr<char> name,
+   completion_match_for_lcd *match_for_lcd)
 {
   void **slot;
 
@@ -1515,7 +1517,13 @@ completion_tracker::maybe_add_completion (gdb::unique_xmalloc_ptr<char> name)
   slot = htab_find_slot (m_entries_hash, name.get (), INSERT);
   if (*slot == HTAB_EMPTY_ENTRY)
     {
-      const char *match_for_lcd_str = name.get ();
+      const char *match_for_lcd_str = NULL;
+
+      if (match_for_lcd != NULL)
+       match_for_lcd_str = match_for_lcd->finish ();
+
+      if (match_for_lcd_str == NULL)
+       match_for_lcd_str = name.get ();
 
       recompute_lowest_common_denominator (match_for_lcd_str);
 
@@ -1529,9 +1537,10 @@ completion_tracker::maybe_add_completion (gdb::unique_xmalloc_ptr<char> name)
 /* See completer.h.  */
 
 void
-completion_tracker::add_completion (gdb::unique_xmalloc_ptr<char> name)
+completion_tracker::add_completion (gdb::unique_xmalloc_ptr<char> name,
+                                   completion_match_for_lcd *match_for_lcd)
 {
-  if (!maybe_add_completion (std::move (name)))
+  if (!maybe_add_completion (std::move (name), match_for_lcd))
     throw_error (MAX_COMPLETIONS_REACHED_ERROR, _("Max completions reached."));
 }
 
index 38fee6bd62341666f1b0ea007d25352e1e92de12..f756412dc48b695ad80c2af09c7bfd3135dafbee 100644 (file)
@@ -116,12 +116,56 @@ private:
   std::string m_storage;
 };
 
+/* The result of a successful completion match, but for least common
+   denominator (LCD) computation.  Some completers provide matches
+   that don't start with the completion "word".  E.g., completing on
+   "b push_ba" on a C++ program usually completes to
+   std::vector<...>::push_back, std::string::push_back etc.  In such
+   case, the symbol comparison routine will set the LCD match to point
+   into the "push_back" substring within the symbol's name string.  */
+
+class completion_match_for_lcd
+{
+public:
+  /* Set the match for LCD.  See m_match's description.  */
+  void set_match (const char *match)
+  { m_match = match; }
+
+  /* Get the resulting LCD, after a successful match.  */
+  const char *finish ()
+  { return m_match; }
+
+  /* Prepare for another completion matching sequence.  */
+  void clear ()
+  { m_match = NULL; }
+
+private:
+  /* The completion match result for LCD.  This is usually either a
+     pointer into to a substring within a symbol's name, or to the
+     storage of the pairing completion_match object.  */
+  const char *m_match;
+};
+
 /* Convenience aggregate holding info returned by the symbol name
    matching routines (see symbol_name_matcher_ftype).  */
 struct completion_match_result
 {
   /* The completion match candidate.  */
   completion_match match;
+
+  /* The completion match, for LCD computation purposes.  */
+  completion_match_for_lcd match_for_lcd;
+
+  /* Convenience that sets both MATCH and MATCH_FOR_LCD.  M_FOR_LCD is
+     optional.  If not specified, defaults to M.  */
+  void set_match (const char *m, const char *m_for_lcd = NULL)
+  {
+    match.set_match (m);
+    if (m_for_lcd == NULL)
+      match_for_lcd.set_match (m);
+    else
+      match_for_lcd.set_match (m_for_lcd);
+  }
 };
 
 /* The final result of a completion that is handed over to either
@@ -188,6 +232,21 @@ public:
      that necessitates the time consuming expansion of many symbol
      tables.
 
+   - The completer's idea of least common denominator (aka the common
+     prefix) between all completion matches to hand over to readline.
+     Some completers provide matches that don't start with the
+     completion "word".  E.g., completing on "b push_ba" on a C++
+     program usually completes to std::vector<...>::push_back,
+     std::string::push_back etc.  If all matches happen to start with
+     "std::", then readline would figure out that the lowest common
+     denominator is "std::", and thus would do a partial completion
+     with that.  I.e., it would replace "push_ba" in the input buffer
+     with "std::", losing the original "push_ba", which is obviously
+     undesirable.  To avoid that, such completers pass the substring
+     of the match that matters for common denominator computation as
+     MATCH_FOR_LCD argument to add_completion.  The end result is
+     passed to readline in gdb_rl_attempted_completion_function.
+
    - The custom word point to hand over to readline, for completers
      that parse the input string in order to dynamically adjust
      themselves depending on exactly what they're completing.  E.g.,
@@ -205,7 +264,8 @@ public:
   /* Add the completion NAME to the list of generated completions if
      it is not there already.  If too many completions were already
      found, this throws an error.  */
-  void add_completion (gdb::unique_xmalloc_ptr<char> name);
+  void add_completion (gdb::unique_xmalloc_ptr<char> name,
+                      completion_match_for_lcd *match_for_lcd = NULL);
 
   /* Add all completions matches in LIST.  Elements are moved out of
      LIST.  */
@@ -268,6 +328,7 @@ public:
 
     /* Clear any previous match.  */
     res.match.clear ();
+    res.match_for_lcd.clear ();
     return m_completion_match_result;
   }
 
@@ -290,10 +351,19 @@ private:
   /* Add the completion NAME to the list of generated completions if
      it is not there already.  If false is returned, too many
      completions were found.  */
-  bool maybe_add_completion (gdb::unique_xmalloc_ptr<char> name);
+  bool maybe_add_completion (gdb::unique_xmalloc_ptr<char> name,
+                            completion_match_for_lcd *match_for_lcd);
 
   /* Given a new match, recompute the lowest common denominator (LCD)
-     to hand over to readline.  */
+     to hand over to readline.  Normally readline computes this itself
+     based on the whole set of completion matches.  However, some
+     completers want to override readline, in order to be able to
+     provide a LCD that is not really a prefix of the matches, but the
+     lowest common denominator of some relevant substring of each
+     match.  E.g., "b push_ba" completes to
+     "std::vector<..>::push_back", "std::string::push_back", etc., and
+     in this case we want the lowest common denominator to be
+     "push_back" instead of "std::".  */
   void recompute_lowest_common_denominator (const char *new_match);
 
   /* Completion match outputs returned by the symbol name matching
@@ -348,8 +418,13 @@ private:
      See intro.  */
   char *m_lowest_common_denominator = NULL;
 
-  /* If true, the LCD is unique.  I.e., all completion candidates had
-     the same string.  */
+  /* If true, the LCD is unique.  I.e., all completions had the same
+     MATCH_FOR_LCD substring, even if the completions were different.
+     For example, if "break function<tab>" found "a::function()" and
+     "b::function()", the LCD will be "function()" in both cases and
+     so we want to tell readline to complete the line with
+     "function()", instead of showing all the possible
+     completions.  */
   bool m_lowest_common_denominator_unique = false;
 };
 
index 368112a529434e3850e408047d011cba699b7846..6c6825be90a960debadf2eaadb67d64b05dfd8a7 100644 (file)
@@ -1634,14 +1634,14 @@ cp_symbol_name_matches_1 (const char *symbol_search_name,
                          const char *lookup_name,
                          size_t lookup_name_len,
                          strncmp_iw_mode mode,
-                         completion_match *match)
+                         completion_match_result *comp_match_res)
 {
   if (strncmp_iw_with_mode (symbol_search_name,
                            lookup_name, lookup_name_len,
                            mode, language_cplus) == 0)
     {
-      if (match != NULL)
-       match->set_match (symbol_search_name);
+      if (comp_match_res != NULL)
+       comp_match_res->set_match (symbol_search_name);
       return true;
     }
 
@@ -1653,7 +1653,7 @@ cp_symbol_name_matches_1 (const char *symbol_search_name,
 static bool
 cp_fq_symbol_name_matches (const char *symbol_search_name,
                           const lookup_name_info &lookup_name,
-                          completion_match *match)
+                          completion_match_result *comp_match_res)
 {
   /* Get the demangled name.  */
   const std::string &name = lookup_name.cplus ().lookup_name ();
@@ -1664,7 +1664,7 @@ cp_fq_symbol_name_matches (const char *symbol_search_name,
 
   return cp_symbol_name_matches_1 (symbol_search_name,
                                   name.c_str (), name.size (),
-                                  mode, match);
+                                  mode, comp_match_res);
 }
 
 /* See cp-support.h.  */
index 2a1419cd3006139b81c52237bb8c93da00e50c7a..c05b703207ff10acfa0a9aa2a3a95493b1b4db4e 100644 (file)
@@ -704,7 +704,7 @@ default_get_string (struct value *value, gdb_byte **buffer, int *length,
 bool
 default_symbol_name_matcher (const char *symbol_search_name,
                             const lookup_name_info &lookup_name,
-                            completion_match *match)
+                            completion_match_result *comp_match_res)
 {
   const std::string &name = lookup_name.name ();
 
@@ -715,8 +715,8 @@ default_symbol_name_matcher (const char *symbol_search_name,
   if (strncmp_iw_with_mode (symbol_search_name, name.c_str (), name.size (),
                            mode, language_minimal) == 0)
     {
-      if (match != NULL)
-       match->set_match (symbol_search_name);
+      if (comp_match_res != NULL)
+       comp_match_res->set_match (symbol_search_name);
       return true;
     }
   else
index 34f569227beb3e45ee149d626aca34723eca8273..47ad8da05d632baafa78b5fb13c47ddad428264e 100644 (file)
@@ -37,6 +37,7 @@ struct type_print_options;
 struct lang_varobj_ops;
 struct parser_state;
 struct compile_instance;
+struct completion_match_for_lcd;
 
 #define MAX_FORTRAN_DIMS  7    /* Maximum number of F77 array dims.  */
 
@@ -629,7 +630,7 @@ void c_get_string (struct value *value, gdb_byte **buffer, int *length,
 extern bool default_symbol_name_matcher
   (const char *symbol_search_name,
    const lookup_name_info &lookup_name,
-   completion_match *match);
+   completion_match_result *comp_match_res);
 
 /* Get LANG's symbol_name_matcher method for LOOKUP_NAME.  Returns
    default_symbol_name_matcher if not set.  */
index a249a8d98eaeb3ebf091d3982a9c875137adb8b5..dd7434ed5093a580c2f77bf5a485cd65dd80e623 100644 (file)
@@ -4698,7 +4698,7 @@ compare_symbol_name (const char *symbol_name, language symbol_language,
   symbol_name_matcher_ftype *name_match
     = language_get_symbol_name_matcher (lang, lookup_name);
 
-  return name_match (symbol_name, lookup_name, &match_res.match);
+  return name_match (symbol_name, lookup_name, &match_res);
 }
 
 /*  See symtab.h.  */
@@ -4751,7 +4751,14 @@ completion_list_add_name (completion_tracker &tracker,
 
     gdb::unique_xmalloc_ptr<char> completion (newobj);
 
-    tracker.add_completion (std::move (completion));
+    /* Here we pass the match-for-lcd object to add_completion.  Some
+       languages match the user text against substrings of symbol
+       names in some cases.  E.g., in C++, "b push_ba" completes to
+       "std::vector::push_back", "std::string::push_back", etc., and
+       in this case we want the completion lowest common denominator
+       to be "push_back" instead of "std::".  */
+    tracker.add_completion (std::move (completion),
+                           &match_res.match_for_lcd);
   }
 }
 
index e5aae2a0601437ef42cd50ef2759295fee84753e..239a4794dfb99a19caceb4eed9f0d8d714ed0e00 100644 (file)
@@ -84,7 +84,7 @@ class ada_lookup_name_info final
      otherwise.  If non-NULL, store the matching results in MATCH.  */
   bool matches (const char *symbol_search_name,
                symbol_name_match_type match_type,
-               completion_match *match) const;
+               completion_match_result *comp_match_res) const;
 
   /* The Ada-encoded lookup name.  */
   const std::string &lookup_name () const
@@ -295,15 +295,21 @@ private:
 
    SYMBOL_SEARCH_NAME should be a symbol's "search" name.
 
-   On success and if non-NULL, MATCH is set to point to the symbol
-   name as should be presented to the user as a completion match list
-   element.  In most languages, this is the same as the symbol's
-   search name, but in some, like Ada, the display name is dynamically
-   computed within the comparison routine.  */
+   On success and if non-NULL, COMP_MATCH_RES->match is set to point
+   to the symbol name as should be presented to the user as a
+   completion match list element.  In most languages, this is the same
+   as the symbol's search name, but in some, like Ada, the display
+   name is dynamically computed within the comparison routine.
+
+   Also, on success and if non-NULL, COMP_MATCH_RES->match_for_lcd
+   points the part of SYMBOL_SEARCH_NAME that was considered to match
+   LOOKUP_NAME.  E.g., in C++, in linespec/wild mode, if the symbol is
+   "foo::function()" and LOOKUP_NAME is "function(", MATCH_FOR_LCD
+   points to "function()" inside SYMBOL_SEARCH_NAME.  */
 typedef bool (symbol_name_matcher_ftype)
   (const char *symbol_search_name,
    const lookup_name_info &lookup_name,
-   completion_match *match);
+   completion_match_result *comp_match_res);
 
 /* Some of the structures in this file are space critical.
    The space-critical structures are: