Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [RFA] Make symbol completion language-specific
@ 2007-12-28 13:07 Joel Brobecker
  2008-01-29 17:35 ` Daniel Jacobowitz
  0 siblings, 1 reply; 10+ messages in thread
From: Joel Brobecker @ 2007-12-28 13:07 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3637 bytes --]

The current symbol completion mechanism works really well for C, and
maybe other languages too, but it's not well suited for completing
Ada entity names.

As suggested by a comment inside symtab.c:make_symbol_completion_list,
I made this routine language-dependent by adding a new field inside
struct language_defn. I then renamed make_symbol_completion_list to
default_make_symbol_completion_list, and wrote a new
make_symbol_completion_list that called the new language method
using the current_language. All languages use the default completer,
except for Ada, for which I am providing a specific completer.

2007-12-28  Joel Brobecker  <brobecker@adacore.com>

        * language.h (struct language_defn): Add new field
        la_make_symbol_completion_list.
        * symtab.c (default_make_symbol_completion_list): Renames
        make_symbol_completion_list.
        (make_symbol_completion_list): New function.
        * symtab.h (default_make_symbol_completion_list): Add declaration.
        * langauge.c (unknown_language): Set la_make_symbol_completion_list.
        (auto_language, local_language): Likewise.
        * objc-lang.c (objc_language_defn): Likewise.
        * scm-lang.c (scm_language_defn): Likewise.
        * m2-lang.c (m2_language_defn): Likewise.
        * f-lang.c (f_language_defn): Likewise.
        * jv-lang.c (java_language_defn): Likewise.
        * p-lang.c (pascal_language_defn): Likewise.
        * c-lang.c (c_language_defn, cplus_language_defn, asm_language_defn)
        (minimal_language_defn): Likewise.
        * ada-lang.c (struct string_vector): New structure.
        (new_string_vector, string_vector_append, ada_unqualified_name)
        (add_angle_brackets, symbol_completion_match, symbol_completion_add)
        (ada_make_symbol_completion_list): New functions.
        (ada_language_defn): Set la_make_symbol_completion_list.
        * ada-lang.h (ada_make_symbol_completion_list): Remove declaration,
        this function is static.

Tested on x86-linux, no regression.
OK to commit?

I have also written a pretty extensive testcase that tests Ada-specific
completion.

2007-12-28  Joel Brobecker  <brobecker@adacore.com>

        * gdb.ada/complete/pck.ads, gdb.ada/complete/pck.adb,
        gdb.ada/complete/foo.adb: New files.
        * gdb.ada/complete.exp: New testcase.

Also tested on x86-linux.

A couple of remarks:

  1. How does someone verify that a GDB command does not return
     any output. Do we really have to do it "manually" (using
     gdb_send et al)?  Right now, there is a hole in my testcase
     regarding this, and I need to fix it before I commit it.

  2. I was getting tired of writing expected output regexps that
     were completely unreadable mostly because the output was matching
     more than one line that ended up concatenated inside the same
     string. So I wrote the following little helper function:

        proc multi_line { args } {
            return [join $args "\[\r\n\]*"]
        }

     This function allows me to do something like this:

        gdb_test "print variable" \
                 [multi_line "first_line" \
                             "second_line" \
                             "last_line" ] \
                 "print big variable"

     I think that this is far easier to read than:

        gdb_test "print variable" \
                 "first_line${eol}second_line${eol}last_line"
                 "print big variable"

     If there is some interest in using this trick more globally,
     we could provide something similar in gdb.exp so that all scripts
     can take advantage of it.

Thanks,
-- 
Joel

[-- Attachment #2: complete.diff --]
[-- Type: text/plain, Size: 20322 bytes --]

Index: ada-lang.c
===================================================================
--- ada-lang.c	(revision 12)
+++ ada-lang.c	(revision 13)
@@ -68,6 +68,17 @@
 #define TRUNCATION_TOWARDS_ZERO ((-5 / 2) == -2)
 #endif
 
+/* A structure that contains a vector of strings.
+   The main purpose of this type is to group the vector and its
+   associated parameters in one structure.  This makes it easier
+   to handle and pass around.  */
+
+struct string_vector
+{
+  char **array; /* The vector itself.  */
+  int index;    /* Index of the next available element in the array.  */
+  size_t size;  /* The number of entries allocated in the array.  */
+};
 
 static void extract_string (CORE_ADDR addr, char *buf);
 
@@ -315,6 +326,65 @@ static struct obstack symbol_list_obstac
 
                         /* Utilities */
 
+/* Create a new empty string_vector struct with an initial size of
+   INITIAL_SIZE.  */
+
+static struct string_vector
+new_string_vector (int initial_size)
+{
+  struct string_vector result;
+
+  result.array = (char **) xmalloc ((initial_size + 1) * sizeof (char *));
+  result.index = 0;
+  result.size = initial_size;
+
+  return result;
+}
+
+/* Add STR at the end of the given string vector SV.  If SV is already
+   full, its size is automatically increased (doubled).  */
+
+static void
+string_vector_append (struct string_vector *sv, char *str)
+{
+  if (sv->index >= sv->size)
+    GROW_VECT (sv->array, sv->size, sv->size * 2);
+
+  sv->array[sv->index] = str;
+  sv->index++;
+}
+
+/* Given DECODED_NAME a string holding a symbol name in its
+   decoded form (ie using the Ada dotted notation), returns
+   its unqualified name.  */
+
+static const char *
+ada_unqualified_name (const char *decoded_name)
+{
+  const char *result = strrchr (decoded_name, '.');
+
+  if (result != NULL)
+    result++;                   /* Skip the dot...  */
+  else
+    result = decoded_name;
+
+  return result;
+}
+
+/* Return a string starting with '<', followed by STR, and '>'.
+   The result is good until the next call.  */
+
+static char *
+add_angle_brackets (const char *str)
+{
+  static char *result = NULL;
+
+  xfree (result);
+  result = (char *) xmalloc ((strlen (str) + 3) * sizeof (char));
+
+  sprintf (result, "<%s>", str);
+  return result;
+}
 
 static char *
 ada_get_gdb_completer_word_break_characters (void)
@@ -5275,6 +5345,289 @@ ada_add_block_symbols (struct obstack *o
     }
 }
 \f
+
+                                /* Symbol Completion */
+
+/* If SYM_NAME is a completion candidate for TEXT, return this symbol
+   name in a form that's appropriate for the completion.  The result
+   does not need to be deallocated, but is only good until the next call.
+
+   TEXT_LEN is equal to the length of TEXT.
+   Perform a wild match if WILD_MATCH is set.
+   ENCODED should be set if TEXT represents the start of a symbol name
+   in its encoded form.  */
+
+static const char *
+symbol_completion_match (const char *sym_name,
+                         const char *text, int text_len,
+                         int wild_match, int encoded)
+{
+  char *result;
+  const int verbatim_match = (text[0] == '<');
+  int match = 0;
+
+  if (verbatim_match)
+    {
+      /* Strip the leading angle bracket.  */
+      text = text + 1;
+      text_len--;
+    }
+
+  /* First, test against the fully qualified name of the symbol.  */
+
+  if (strncmp (sym_name, text, text_len) == 0)
+    match = 1;
+
+  if (match && !encoded)
+    {
+      /* One needed check before declaring a positive match is to verify
+         that iff we are doing a verbatim match, the decoded version
+         of the symbol name starts with '<'.  Otherwise, this symbol name
+         is not a suitable completion.  */
+      const char *sym_name_copy = sym_name;
+      int has_angle_bracket;
+
+      sym_name = ada_decode (sym_name);
+      has_angle_bracket = (sym_name[0] == '<');
+      match = (has_angle_bracket == verbatim_match);
+      sym_name = sym_name_copy;
+    }
+
+  if (match && !verbatim_match)
+    {
+      /* When doing non-verbatim match, another check that needs to
+         be done is to verify that the potentially matching symbol name
+         does not include capital letters, because the ada-mode would
+         not be able to understand these symbol names without the
+         angle bracket notation.  */
+      const char *tmp;
+
+      for (tmp = sym_name; *tmp != '\0' && !isupper (*tmp); tmp++);
+      if (*tmp != '\0')
+        match = 0;
+    }
+
+  /* Second: Try wild matching...  */
+
+  if (!match && wild_match)
+    {
+      /* Since we are doing wild matching, this means that TEXT
+         may represent an unqualified symbol name.  We therefore must
+         also compare TEXT against the unqualified name of the symbol.  */
+      sym_name = ada_unqualified_name (ada_decode (sym_name));
+
+      if (strncmp (sym_name, text, text_len) == 0)
+        match = 1;
+    }
+
+  /* Finally: If we found a mach, prepare the result to return.  */
+
+  if (!match)
+    return NULL;
+
+  if (verbatim_match)
+    sym_name = add_angle_brackets (sym_name);
+
+  if (!encoded)
+    sym_name = ada_decode (sym_name);
+
+  return sym_name;
+}
+
+/* A companion function to ada_make_symbol_completion_list().
+   Check if SYM_NAME represents a symbol which name would be suitable
+   to complete TEXT (TEXT_LEN is the length of TEXT), in which case
+   it is appended at the end of the given string vector SV.
+
+   ORIG_TEXT is the string original string from the user command
+   that needs to be completed.  WORD is the entire command on which
+   completion should be performed.  These two parameters are used to
+   determine which part of the symbol name should be added to the
+   completion vector.
+   if WILD_MATCH is set, then wild matching is performed.
+   ENCODED should be set if TEXT represents a symbol name in its
+   encoded formed (in which case the completion should also be
+   encoded).  */
+
+static void
+symbol_completion_add (struct string_vector *sv,
+                       const char *sym_name,
+                       const char *text, int text_len,
+                       const char *orig_text, const char *word,
+                       int wild_match, int encoded)
+{
+  const char *match = symbol_completion_match (sym_name, text, text_len,
+                                               wild_match, encoded);
+  char *completion;
+
+  if (match == NULL)
+    return;
+
+  /* We found a match, so add the appropriate completion to the given
+     string vector.  */
+
+  if (word == orig_text)
+    {
+      completion = xmalloc (strlen (match) + 5);
+      strcpy (completion, match);
+    }
+  else if (word > orig_text)
+    {
+      /* Return some portion of sym_name.  */
+      completion = xmalloc (strlen (match) + 5);
+      strcpy (completion, match + (word - orig_text));
+    }
+  else
+    {
+      /* Return some of ORIG_TEXT plus sym_name.  */
+      completion = xmalloc (strlen (match) + (orig_text - word) + 5);
+      strncpy (completion, word, orig_text - word);
+      completion[orig_text - word] = '\0';
+      strcat (completion, match);
+    }
+
+  string_vector_append (sv, completion);
+}
+
+/* Return a list of possible symbol names completing TEXT0.  The list
+   is NULL terminated.  WORD is the entire command on which completion
+   is made.  */
+
+static char **
+ada_make_symbol_completion_list (char *text0, char *word)
+{
+  char *text;
+  int text_len;
+  int wild_match;
+  int encoded;
+  struct string_vector result = new_string_vector (128);
+  struct symbol *sym;
+  struct symtab *s;
+  struct partial_symtab *ps;
+  struct minimal_symbol *msymbol;
+  struct objfile *objfile;
+  struct block *b, *surrounding_static_block = 0;
+  int i;
+  struct dict_iterator iter;
+
+  if (text0[0] == '<')
+    {
+      text = xstrdup (text0);
+      make_cleanup (xfree, text);
+      text_len = strlen (text);
+      wild_match = 0;
+      encoded = 1;
+    }
+  else
+    {
+      text = xstrdup (ada_encode (text0));
+      make_cleanup (xfree, text);
+      text_len = strlen (text);
+      for (i = 0; i < text_len; i++)
+        text[i] = tolower (text[i]);
+
+      /* FIXME: brobecker/2003-09-17: When we get rid of ADA_RETAIN_DOTS,
+         we can restrict the wild_match check to searching "__" only.  */
+      wild_match = (strstr (text0, "__") == NULL
+                    && strchr (text0, '.') == NULL);
+      encoded = (strstr (text0, "__") != NULL);
+    }
+
+  /* First, look at the partial symtab symbols.  */
+  ALL_PSYMTABS (objfile, ps)
+  {
+    struct partial_symbol **psym;
+
+    /* If the psymtab's been read in we'll get it when we search
+       through the blockvector.  */
+    if (ps->readin)
+      continue;
+
+    for (psym = objfile->global_psymbols.list + ps->globals_offset;
+         psym < (objfile->global_psymbols.list + ps->globals_offset
+                 + ps->n_global_syms); psym++)
+      {
+        QUIT;
+        symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (*psym),
+                               text, text_len, text0, word,
+                               wild_match, encoded);
+      }
+
+    for (psym = objfile->static_psymbols.list + ps->statics_offset;
+         psym < (objfile->static_psymbols.list + ps->statics_offset
+                 + ps->n_static_syms); psym++)
+      {
+        QUIT;
+        symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (*psym),
+                               text, text_len, text0, word,
+                               wild_match, encoded);
+      }
+  }
+
+  /* At this point scan through the misc symbol vectors and add each
+     symbol you find to the list.  Eventually we want to ignore
+     anything that isn't a text symbol (everything else will be
+     handled by the psymtab code above).  */
+
+  ALL_MSYMBOLS (objfile, msymbol)
+  {
+    QUIT;
+    symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (msymbol),
+                           text, text_len, text0, word, wild_match, encoded);
+  }
+
+  /* Search upwards from currently selected frame (so that we can
+     complete on local vars.  */
+
+  for (b = get_selected_block (0); b != NULL; b = BLOCK_SUPERBLOCK (b))
+    {
+      if (!BLOCK_SUPERBLOCK (b))
+        surrounding_static_block = b;   /* For elmin of dups */
+
+      ALL_BLOCK_SYMBOLS (b, iter, sym)
+      {
+        symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (sym),
+                               text, text_len, text0, word,
+                               wild_match, encoded);
+      }
+    }
+
+  /* Go through the symtabs and check the externs and statics for
+     symbols which match.  */
+
+  ALL_SYMTABS (objfile, s)
+  {
+    QUIT;
+    b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
+    ALL_BLOCK_SYMBOLS (b, iter, sym)
+    {
+      symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (sym),
+                             text, text_len, text0, word,
+                             wild_match, encoded);
+    }
+  }
+
+  ALL_SYMTABS (objfile, s)
+  {
+    QUIT;
+    b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), STATIC_BLOCK);
+    /* Don't do this block twice.  */
+    if (b == surrounding_static_block)
+      continue;
+    ALL_BLOCK_SYMBOLS (b, iter, sym)
+    {
+      symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (sym),
+                             text, text_len, text0, word,
+                             wild_match, encoded);
+    }
+  }
+
+  /* Append the closing NULL entry.  */
+  string_vector_append (&result, NULL);
+
+  return (result.array);
+}
+
                                 /* Field Access */
 
 /* True if field number FIELD_NUM in struct or union type TYPE is supposed
@@ -10510,6 +10863,7 @@ const struct language_defn ada_language_
   0,                            /* c-style arrays */
   1,                            /* String lower bound */
   ada_get_gdb_completer_word_break_characters,
+  ada_make_symbol_completion_list,
   ada_language_arch_info,
   ada_print_array_index,
   default_pass_by_reference,
Index: ada-lang.h
===================================================================
--- ada-lang.h	(revision 12)
+++ ada-lang.h	(revision 13)
@@ -311,9 +311,6 @@ extern enum language ada_update_initial_
 
 extern void clear_ada_sym_cache (void);
 
-extern char **ada_make_symbol_completion_list (const char *text0,
-                                               const char *word);
-
 extern int ada_lookup_symbol_list (const char *, const struct block *,
                                    domain_enum, struct ada_symbol_info**);
 
Index: objc-lang.c
===================================================================
--- objc-lang.c	(revision 12)
+++ objc-lang.c	(revision 13)
@@ -517,6 +517,7 @@ const struct language_defn objc_language
   1,				/* C-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   c_language_arch_info,
   default_print_array_index,
   default_pass_by_reference,
Index: scm-lang.c
===================================================================
--- scm-lang.c	(revision 12)
+++ scm-lang.c	(revision 13)
@@ -262,6 +262,7 @@ const struct language_defn scm_language_
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   c_language_arch_info,
   default_print_array_index,
   default_pass_by_reference,
Index: m2-lang.c
===================================================================
--- m2-lang.c	(revision 12)
+++ m2-lang.c	(revision 13)
@@ -384,6 +384,7 @@ const struct language_defn m2_language_d
   0,				/* arrays are first-class (not c-style) */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   m2_language_arch_info,
   default_print_array_index,
   default_pass_by_reference,
Index: f-lang.c
===================================================================
--- f-lang.c	(revision 12)
+++ f-lang.c	(revision 13)
@@ -333,6 +333,7 @@ const struct language_defn f_language_de
   0,				/* arrays are first-class (not c-style) */
   1,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   f_language_arch_info,
   default_print_array_index,
   default_pass_by_reference,
Index: jv-lang.c
===================================================================
--- jv-lang.c	(revision 12)
+++ jv-lang.c	(revision 13)
@@ -1079,6 +1079,7 @@ const struct language_defn java_language
   0,				/* not c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   c_language_arch_info,
   default_print_array_index,
   default_pass_by_reference,
Index: language.c
===================================================================
--- language.c	(revision 12)
+++ language.c	(revision 13)
@@ -1197,6 +1197,7 @@ const struct language_defn unknown_langu
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   unknown_language_arch_info,	/* la_language_arch_info.  */
   default_print_array_index,
   default_pass_by_reference,
@@ -1232,6 +1233,7 @@ const struct language_defn auto_language
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   unknown_language_arch_info,	/* la_language_arch_info.  */
   default_print_array_index,
   default_pass_by_reference,
@@ -1266,6 +1268,7 @@ const struct language_defn local_languag
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   unknown_language_arch_info,	/* la_language_arch_info.  */
   default_print_array_index,
   default_pass_by_reference,
Index: language.h
===================================================================
--- language.h	(revision 12)
+++ language.h	(revision 13)
@@ -249,6 +249,11 @@ struct language_defn
     /* The list of characters forming word boundaries.  */
     char *(*la_word_break_characters) (void);
 
+    /* Should return a NULL terminated array of all symbols which
+       are possible completions for TEXT.  WORD is the entire command
+       on which the completion is being made.  */
+    char **(*la_make_symbol_completion_list) (char *text, char *word);
+
     /* The per-architecture (OS/ABI) language information.  */
     void (*la_language_arch_info) (struct gdbarch *,
 				   struct language_arch_info *);
Index: symtab.c
===================================================================
--- symtab.c	(revision 12)
+++ symtab.c	(revision 13)
@@ -3505,17 +3505,13 @@ language_search_unquoted_string (char *t
   return p;
 }
 
-
-/* Return a NULL terminated array of all symbols (regardless of class)
-   which begin by matching TEXT.  If the answer is no symbols, then
-   the return value is an array which contains only a NULL pointer.
-
-   Problem: All of the symbols have to be copied because readline frees them.
-   I'm not going to worry about this; hopefully there won't be that many.  */
-
 char **
-make_symbol_completion_list (char *text, char *word)
+default_make_symbol_completion_list (char *text, char *word)
 {
+  /* Problem: All of the symbols have to be copied because readline
+     frees them.  I'm not going to worry about this; hopefully there
+     won't be that many.  */
+
   struct symbol *sym;
   struct symtab *s;
   struct partial_symtab *ps;
@@ -3699,6 +3695,16 @@ make_symbol_completion_list (char *text,
   return (return_val);
 }
 
+/* Return a NULL terminated array of all symbols (regardless of class)
+   which begin by matching TEXT.  If the answer is no symbols, then
+   the return value is an array which contains only a NULL pointer.  */
+
+char **
+make_symbol_completion_list (char *text, char *word)
+{
+  return current_language->la_make_symbol_completion_list (text, word);
+}
+
 /* Like make_symbol_completion_list, but returns a list of symbols
    defined in a source file FILE.  */
 
Index: symtab.h
===================================================================
--- symtab.h	(revision 12)
+++ symtab.h	(revision 13)
@@ -1322,6 +1322,7 @@ extern void forget_cached_source_info (v
 
 extern void select_source_symtab (struct symtab *);
 
+extern char **default_make_symbol_completion_list (char *, char *);
 extern char **make_symbol_completion_list (char *, char *);
 
 extern char **make_file_symbol_completion_list (char *, char *, char *);
Index: p-lang.c
===================================================================
--- p-lang.c	(revision 12)
+++ p-lang.c	(revision 13)
@@ -423,6 +423,7 @@ const struct language_defn pascal_langua
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   pascal_language_arch_info,
   default_print_array_index,
   default_pass_by_reference,
Index: c-lang.c
===================================================================
--- c-lang.c	(revision 12)
+++ c-lang.c	(revision 13)
@@ -423,6 +423,7 @@ const struct language_defn c_language_de
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   c_language_arch_info,
   default_print_array_index,
   default_pass_by_reference,
@@ -535,6 +536,7 @@ const struct language_defn cplus_languag
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   cplus_language_arch_info,
   default_print_array_index,
   cp_pass_by_reference,
@@ -569,6 +571,7 @@ const struct language_defn asm_language_
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   c_language_arch_info, /* FIXME: la_language_arch_info.  */
   default_print_array_index,
   default_pass_by_reference,
@@ -608,6 +611,7 @@ const struct language_defn minimal_langu
   1,				/* c-style arrays */
   0,				/* String lower bound */
   default_word_break_characters,
+  default_make_symbol_completion_list,
   c_language_arch_info,
   default_print_array_index,
   default_pass_by_reference,

[-- Attachment #3: test-complete.diff --]
[-- Type: text/plain, Size: 8442 bytes --]

Index: gdb.ada/complete.exp
===================================================================
--- gdb.ada/complete.exp	(revision 0)
+++ gdb.ada/complete.exp	(revision 14)
@@ -0,0 +1,189 @@
+# Copyright 2005, 2007 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if $tracelevel then {
+    strace $tracelevel
+}
+
+load_lib "ada.exp"
+
+set testdir "complete"
+set testfile "${testdir}/foo"
+set srcfile ${srcdir}/${subdir}/${testfile}.adb
+set binfile ${objdir}/${subdir}/${testfile}
+
+file mkdir ${objdir}/${subdir}/${testdir}
+if {[gdb_compile_ada "${srcfile}" "${binfile}" executable [list debug ]] != "" } {
+  return -1
+}
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+set bp_location [gdb_get_line_number "START" ${testdir}/foo.adb]
+runto "foo.adb:$bp_location"
+
+set eol "\[\r\n\]*"
+
+# A convenience function that verifies that the "complete EXPR" command
+# returns the EXPECTED_OUTPUT.
+
+proc test_gdb_complete { expr expected_output } {
+    gdb_test "complete p $expr" \
+             "$expected_output" \
+             "complete p $expr"
+}
+
+# A convenience function that verifies that the "complete EXPR" command
+# does not genearte any output.
+
+proc test_gdb_no_completion { expr } {
+    # FIXME: brobecker/2007-12-27: How do you verify that the command
+    # output is actually really empty???  For now, the following does
+    # not verify this at all:
+    test_gdb_complete "$expr" ""
+}
+
+# A convenience function that joins all the arguments together,
+# with a regexp that matches zero-or-more end of lines in between
+# each argument.  This function is ideal to write the expected output
+# of a GDB command that generates more than a couple of lines, as
+# this allows us to write each line as a separate string, which is
+# easier to read by a human being.
+
+proc multi_line { args } {
+    return [join $args "\[\r\n\]*"]
+}
+# Try a global variable, only one match should be found:
+
+test_gdb_complete "my_glob" \
+                  "p my_global_variable"
+
+# A global variable, inside a nested package:
+
+test_gdb_complete "insi" \
+                  "p inside_variable"
+
+# A global variable inside a nested package, but only giving part of
+# the fully qualified name (top level package name missing):
+
+test_gdb_no_completion "inner.insi"
+
+# An incomplete nested package name, were lies a single symbol:
+test_gdb_complete "pck.inne" \
+                  "p pck.inner.inside_variable"
+
+# A fully qualified symbol name, mangled...
+test_gdb_complete "pck__inner__ins" \
+                  "p pck__inner__inside_variable"
+
+# A fully qualified symbol name...
+test_gdb_complete "pck.inner.ins" \
+                  "p pck.inner.inside_variable"
+
+# Make sure that "inside" is not returned as a possible completion
+# for "side"...
+test_gdb_no_completion "side"
+
+# Verify that "Exported_Capitalized" is not returned as a match for
+# "exported", since its symbol name contains capital letters.
+test_gdb_no_completion "exported"
+
+# check the "<...>" notation.
+test_gdb_complete "<Exported" \
+                  "p <Exported_Capitalized>"
+
+# A global symbol, created by the binder, that starts with __gnat...
+test_gdb_complete "__gnat_ada_main_progra" \
+                  "p __gnat_ada_main_program_name"
+
+# A global symbol, created by the binder, that starts with __gnat,
+# and using the '<' notation.
+test_gdb_complete "<__gnat_ada_main_prog" \
+                  "p <__gnat_ada_main_program_name>"
+
+# A local variable
+test_gdb_complete "some" \
+                  "p some_local_variable"
+
+# A local variable variable, but in a different procedure. No match
+# should be returned.
+test_gdb_no_completion "not_in_sco"
+
+# A fully qualified variable name that doesn't exist...
+test_gdb_no_completion "pck.ins"
+
+# A fully qualified variable name that does exist...
+test_gdb_complete "pck.my" \
+                  "p pck.my_global_variable"
+
+# A fully qualified package name
+test_gdb_complete "pck.inne" \
+                  "p pck.inner.inside_variable"
+
+# A fully qualified package name, with a dot at the end
+test_gdb_complete "pck.inner." \
+                  "p pck.inner.inside_variable"
+
+# Two matches, from the global scope:
+test_gdb_complete "local_ident" \
+                  [multi_line "p local_identical_one" \
+                              "p local_identical_two" ]
+
+# Two matches, from the global scope, but using fully qualified names:
+test_gdb_complete "pck.local_ident" \
+                  [multi_line "p pck.local_identical_one" \
+                              "p pck.local_identical_two" ]
+
+# Two matches, from the global scope, but using mangled fully qualified
+# names:
+test_gdb_complete "pck__local_ident" \
+                  [multi_line "p pck__local_identical_one" \
+                              "p pck__local_identical_two" ]
+
+# Two matches, one from the global scope, the other from the local scope:
+test_gdb_complete "external_ident" \
+                  [multi_line "p external_identical_one" \
+                              "p external_identical_two" ]
+
+# Complete on the name of package. 
+test_gdb_complete "pck" \
+                  [multi_line "(p pck\\.ad\[sb\])?" \
+                              "(p pck\\.ad\[sb\])?" \
+                              "p pck.external_identical_one" \
+                              "p pck.inner.inside_variable" \
+                              "p pck.local_identical_one" \
+                              "p pck.local_identical_two" \
+                              "p pck.my_global_variable" \
+                              "p pck.proc" ]
+
+# Complete on the name of a package followed by a dot:
+test_gdb_complete "pck." \
+                  [multi_line "(p pck\\.ad\[sb\])?" \
+                              "(p pck\\.ad\[sb\])?" \
+                              "p pck.external_identical_one" \
+                              "p pck.inner.inside_variable" \
+                              "p pck.local_identical_one" \
+                              "p pck.local_identical_two" \
+                              "p pck.my_global_variable" \
+                              "p pck.proc" ]
+
+# Complete a mangled symbol name, but using the '<...>' notation.
+test_gdb_complete "<pck__my" \
+                  "p <pck__my_global_variable>"
+
+
Index: gdb.ada/complete/pck.adb
===================================================================
--- gdb.ada/complete/pck.adb	(revision 0)
+++ gdb.ada/complete/pck.adb	(revision 14)
@@ -0,0 +1,9 @@
+package body Pck is
+
+   procedure Proc (I : Integer) is
+      Not_In_Scope : Integer := 77;
+   begin
+      Inner.Inside_Variable := Not_In_Scope + I;
+   end Proc;
+
+end Pck;
Index: gdb.ada/complete/pck.ads
===================================================================
--- gdb.ada/complete/pck.ads	(revision 0)
+++ gdb.ada/complete/pck.ads	(revision 14)
@@ -0,0 +1,19 @@
+package Pck is
+
+   My_Global_Variable : Integer := 1;
+
+   Exported_Capitalized : Integer := 2;
+   pragma Export (C, Exported_Capitalized, "Exported_Capitalized");
+
+   Local_Identical_One : Integer := 4;
+   Local_Identical_Two : Integer := 8;
+
+   External_Identical_One : Integer := 19;
+
+   package Inner is
+      Inside_Variable : Integer := 3;
+   end Inner;
+
+   procedure Proc (I : Integer);
+
+end Pck;
Index: gdb.ada/complete/foo.adb
===================================================================
--- gdb.ada/complete/foo.adb	(revision 0)
+++ gdb.ada/complete/foo.adb	(revision 14)
@@ -0,0 +1,10 @@
+with Pck; use Pck;
+
+procedure Foo is
+   Some_Local_Variable : Integer := 1;
+   External_Identical_Two : Integer := 74;
+begin
+   My_Global_Variable := Some_Local_Variable + 1; -- START
+   Proc (External_Identical_Two);
+end Foo;
+

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFA] Make symbol completion language-specific
  2007-12-28 13:07 [RFA] Make symbol completion language-specific Joel Brobecker
@ 2008-01-29 17:35 ` Daniel Jacobowitz
  2008-01-30 20:43   ` Joel Brobecker
  2008-02-04 23:15   ` Joel Brobecker
  0 siblings, 2 replies; 10+ messages in thread
From: Daniel Jacobowitz @ 2008-01-29 17:35 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Fri, Dec 28, 2007 at 04:28:25AM -0800, Joel Brobecker wrote:
>   1. How does someone verify that a GDB command does not return
>      any output. Do we really have to do it "manually" (using
>      gdb_send et al)?  Right now, there is a hole in my testcase
>      regarding this, and I need to fix it before I commit it.

Pretty much, though I think you could do it with gdb_test_multiple;
see how lib/mi-support.exp does it.  MI tests are anchored by default.
There may be more trouble doing it for the CLI, though, because
readline puts extra stuff in the output sometimes.

>   2. I was getting tired of writing expected output regexps that
>      were completely unreadable mostly because the output was matching
>      more than one line that ended up concatenated inside the same
>      string. So I wrote the following little helper function:
> 
>         proc multi_line { args } {
>             return [join $args "\[\r\n\]*"]
>         }
> 
>      This function allows me to do something like this:
> 
>         gdb_test "print variable" \
>                  [multi_line "first_line" \
>                              "second_line" \
>                              "last_line" ] \
>                  "print big variable"
> 
>      I think that this is far easier to read than:
> 
>         gdb_test "print variable" \
>                  "first_line${eol}second_line${eol}last_line"
>                  "print big variable"

Also see gdb_expect_list, which is similar.  I think that the way
you've written it lends to things being too far indented, which will
be hard to read...

> Index: ada-lang.c
> ===================================================================
> --- ada-lang.c	(revision 12)
> +++ ada-lang.c	(revision 13)
> @@ -68,6 +68,17 @@
>  #define TRUNCATION_TOWARDS_ZERO ((-5 / 2) == -2)
>  #endif
>  
> +/* A structure that contains a vector of strings.
> +   The main purpose of this type is to group the vector and its
> +   associated parameters in one structure.  This makes it easier
> +   to handle and pass around.  */
> +
> +struct string_vector
> +{
> +  char **array; /* The vector itself.  */
> +  int index;    /* Index of the next available element in the array.  */
> +  size_t size;  /* The number of entries allocated in the array.  */
> +};

We have a generic VEC nowadays.

You left the "should be language specific" FIXME :-)

And a general comment, I'm not thrilled at the amount of generic
symbol table code had to be duplicated in the Ada-specific files.  But
that's the status quo for ada-lang.c, anyway.

I have no objections, despite the above whining.


-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFA] Make symbol completion language-specific
  2008-01-29 17:35 ` Daniel Jacobowitz
@ 2008-01-30 20:43   ` Joel Brobecker
  2008-01-30 21:25     ` Daniel Jacobowitz
  2008-02-04 23:15   ` Joel Brobecker
  1 sibling, 1 reply; 10+ messages in thread
From: Joel Brobecker @ 2008-01-30 20:43 UTC (permalink / raw)
  To: gdb-patches

> >   1. How does someone verify that a GDB command does not return
> >      any output. Do we really have to do it "manually" (using
> >      gdb_send et al)?  Right now, there is a hole in my testcase
> >      regarding this, and I need to fix it before I commit it.
> 
> Pretty much, though I think you could do it with gdb_test_multiple;
> see how lib/mi-support.exp does it.  MI tests are anchored by default.
> There may be more trouble doing it for the CLI, though, because
> readline puts extra stuff in the output sometimes.

Will take a look, thanks. I suggest we add a new routine inside gdb.exp
that does check that a given command doesn't return any output. Would
that be ok?

> Also see gdb_expect_list, which is similar.  I think that the way
> you've written it lends to things being too far indented, which will
> be hard to read...

Thanks for the tip! I'll have a look as well.

> > +struct string_vector
> > +{
> > +  char **array; /* The vector itself.  */
> > +  int index;    /* Index of the next available element in the array.  */
> > +  size_t size;  /* The number of entries allocated in the array.  */
> > +};
> 
> We have a generic VEC nowadays.

Argh, right. I will rewrite the patch to use a VEC, remove the FIXME,
and resubmit.

Thanks,
-- 
Joel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFA] Make symbol completion language-specific
  2008-01-30 20:43   ` Joel Brobecker
@ 2008-01-30 21:25     ` Daniel Jacobowitz
  0 siblings, 0 replies; 10+ messages in thread
From: Daniel Jacobowitz @ 2008-01-30 21:25 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Wed, Jan 30, 2008 at 12:31:39PM -0800, Joel Brobecker wrote:
> Will take a look, thanks. I suggest we add a new routine inside gdb.exp
> that does check that a given command doesn't return any output. Would
> that be ok?

How about an anchored version of gdb_test?  We miss leading output,
which is also a problem.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFA] Make symbol completion language-specific
  2008-01-29 17:35 ` Daniel Jacobowitz
  2008-01-30 20:43   ` Joel Brobecker
@ 2008-02-04 23:15   ` Joel Brobecker
  2008-02-04 23:30     ` Daniel Jacobowitz
  1 sibling, 1 reply; 10+ messages in thread
From: Joel Brobecker @ 2008-02-04 23:15 UTC (permalink / raw)
  To: gdb-patches

Hi Daniel,

> > +struct string_vector
> > +{
> > +  char **array; /* The vector itself.  */
> > +  int index;    /* Index of the next available element in the array.  */
> > +  size_t size;  /* The number of entries allocated in the array.  */
> > +};
> 
> We have a generic VEC nowadays.

I just had a look, and I don't think the generic VEC integrates itself
well with the current infrastructure.  The type above works well,
because I can then directly pass the char **array back to the caller
of our completion routine (one of the callers is responsible for
freeing it - see completer.c:line_completion_function).

If I were to use a VEC, I would have to return a copy of the contents
of the VEC, which seems silly because I'd end up immediately destroying
an array that I just copied.

I propose the following:

  - I check the current patch in, as is (with an extra comment
    explaining why we're not using a VEC).

  - I can work on a follow-up patch that changes the inferface of
    the completer to use VECs instead of a NULL-terminated arrays.
    It's unclear how much benefit it's going to bring, but I could
    work on that relatively soon.

Thanks,
-- 
Joel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFA] Make symbol completion language-specific
  2008-02-04 23:15   ` Joel Brobecker
@ 2008-02-04 23:30     ` Daniel Jacobowitz
  2008-02-05 22:32       ` Joel Brobecker
  0 siblings, 1 reply; 10+ messages in thread
From: Daniel Jacobowitz @ 2008-02-04 23:30 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Mon, Feb 04, 2008 at 03:15:10PM -0800, Joel Brobecker wrote:
> I just had a look, and I don't think the generic VEC integrates itself
> well with the current infrastructure.  The type above works well,
> because I can then directly pass the char **array back to the caller
> of our completion routine (one of the callers is responsible for
> freeing it - see completer.c:line_completion_function).
> 
> If I were to use a VEC, I would have to return a copy of the contents
> of the VEC, which seems silly because I'd end up immediately destroying
> an array that I just copied.

It still saves you having to write your own push and pop.  But, hey,
you already have them... Personally I'd rather do the copy than have
my own set of accessors, but that's only my preference.

> I propose the following:
> 
>   - I check the current patch in, as is (with an extra comment
>     explaining why we're not using a VEC).
> 
>   - I can work on a follow-up patch that changes the inferface of
>     the completer to use VECs instead of a NULL-terminated arrays.
>     It's unclear how much benefit it's going to bring, but I could
>     work on that relatively soon.

I don't think the second step is even worthwhile.  Objection
withdrawn.

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFA] Make symbol completion language-specific
  2008-02-04 23:30     ` Daniel Jacobowitz
@ 2008-02-05 22:32       ` Joel Brobecker
  2008-02-06  2:40         ` [RFA/commit/Ada] Replace home-made string_vector struct with VEC Joel Brobecker
  0 siblings, 1 reply; 10+ messages in thread
From: Joel Brobecker @ 2008-02-05 22:32 UTC (permalink / raw)
  To: gdb-patches

> > If I were to use a VEC, I would have to return a copy of the contents
> > of the VEC, which seems silly because I'd end up immediately destroying
> > an array that I just copied.
> 
> It still saves you having to write your own push and pop.  But, hey,
> you already have them... Personally I'd rather do the copy than have
> my own set of accessors, but that's only my preference.

I don't have a strong preference either, but your point about having
to redefine some accessors is a good one.

I've committed the patch as is, and will do the VEC replacement as
a separate patch.

Thanks Daniel,
-- 
Joel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [RFA/commit/Ada] Replace home-made string_vector struct with VEC
  2008-02-05 22:32       ` Joel Brobecker
@ 2008-02-06  2:40         ` Joel Brobecker
  2008-02-06  2:48           ` Daniel Jacobowitz
  2008-02-07 19:14           ` Joel Brobecker
  0 siblings, 2 replies; 10+ messages in thread
From: Joel Brobecker @ 2008-02-06  2:40 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1834 bytes --]

Hi Daniel,

> I've committed the patch as is, and will do the VEC replacement as
> a separate patch.

I gave your suggestion a try, and here is what I got. I like using
the VEC as I feel that it will be more robust in the long run than
my local home-made string_vector (even if the code that I wrote was
pretty simple to start with). Overall, it doesn't looks like much
less code, but that's mostly because of the copy that I have to make,
I think.

While working on this change, it made me wonder about a few things:

  1. Would it make sense to have standard VECs predefined somewhere?
     For instance, I wonder how often a VEC of strings will be useful
     in GDB...

  2. Would it make sense to expand the VEC API to have a method
     that will return a copy of the underlying array? After some
     thoughts, and a look at the current usage, it seemed to me
     that it didn't make much sense. I didn't see any other places
     where we need to make that copy, and making that copy is
     pretty simple.

What do you think of the patch? In hindsight, I think it has more
merit than I thought. So I think I'll commit it unless you see some
flaws in it.

2007-02-05  Joel Brobecker  <brobecker@adacore.com>

        * ada-lang.c: #include "vec.h".
        (struct string_vector, new_string_vector, string_vector_append):
        Delete.
        (char_ptr): New typedef.
        (DEF_VEC_P (char_ptr)): New VEC type.
        (symbol_completion_add): Update profile to take the new VEC type
        instead of the old string_vector structure. Update code accordingly.
        (ada_make_symbol_completion_list): Use the new VEC type instead of
        the old string_vector structure, and update the code accordingly.
        * Makefile.in (ada-lang.o): Add dependency on vec.h.

Tested on x86-linux. No regression.

-- 
Joel


[-- Attachment #2: complete-VEC.diff --]
[-- Type: text/plain, Size: 7512 bytes --]

Index: ada-lang.c
===================================================================
--- ada-lang.c	(revision 165)
+++ ada-lang.c	(revision 166)
@@ -55,6 +55,7 @@
 #include "valprint.h"
 #include "source.h"
 #include "observer.h"
+#include "vec.h"
 
 /* Define whether or not the C operator '/' truncates towards zero for
    differently signed operands (truncation direction is undefined in C). 
@@ -64,25 +65,6 @@
 #define TRUNCATION_TOWARDS_ZERO ((-5 / 2) == -2)
 #endif
 
-/* A structure that contains a vector of strings.
-   The main purpose of this type is to group the vector and its
-   associated parameters in one structure.  This makes it easier
-   to handle and pass around.
-   
-   brobecker/2008-02-04:  GDB does provide a generic VEC which should be
-   preferable.  But we are using the string_vector structure in the context
-   of symbol completion, and the current infrastructure is such that it's
-   more convenient to use the string vector for now.  It would become
-   advantageous to switch to VECs if the rest of the completion-related
-   code switches to VECs as well.  */
-
-struct string_vector
-{
-  char **array; /* The vector itself.  */
-  int index;    /* Index of the next available element in the array.  */
-  size_t size;  /* The number of entries allocated in the array.  */
-};
-
 static void extract_string (CORE_ADDR addr, char *buf);
 
 static void modify_general_field (char *, LONGEST, int, int);
@@ -347,34 +329,6 @@ static struct obstack symbol_list_obstac
 
                         /* Utilities */
 
-/* Create a new empty string_vector struct with an initial size of
-   INITIAL_SIZE.  */
-
-static struct string_vector
-new_string_vector (int initial_size)
-{
-  struct string_vector result;
-
-  result.array = (char **) xmalloc ((initial_size + 1) * sizeof (char *));
-  result.index = 0;
-  result.size = initial_size;
-
-  return result;
-}
-
-/* Add STR at the end of the given string vector SV.  If SV is already
-   full, its size is automatically increased (doubled).  */
-
-static void
-string_vector_append (struct string_vector *sv, char *str)
-{
-  if (sv->index >= sv->size)
-    GROW_VECT (sv->array, sv->size, sv->size * 2);
-
-  sv->array[sv->index] = str;
-  sv->index++;
-}
-
 /* Given DECODED_NAME a string holding a symbol name in its
    decoded form (ie using the Ada dotted notation), returns
    its unqualified name.  */
@@ -5525,6 +5479,9 @@ symbol_completion_match (const char *sym
   return sym_name;
 }
 
+typedef char *char_ptr;
+DEF_VEC_P (char_ptr);
+
 /* A companion function to ada_make_symbol_completion_list().
    Check if SYM_NAME represents a symbol which name would be suitable
    to complete TEXT (TEXT_LEN is the length of TEXT), in which case
@@ -5541,7 +5498,7 @@ symbol_completion_match (const char *sym
    encoded).  */
 
 static void
-symbol_completion_add (struct string_vector *sv,
+symbol_completion_add (VEC(char_ptr) *sv,
                        const char *sym_name,
                        const char *text, int text_len,
                        const char *orig_text, const char *word,
@@ -5577,7 +5534,7 @@ symbol_completion_add (struct string_vec
       strcat (completion, match);
     }
 
-  string_vector_append (sv, completion);
+  VEC_safe_push (char_ptr, sv, completion);
 }
 
 /* Return a list of possible symbol names completing TEXT0.  The list
@@ -5591,7 +5548,7 @@ ada_make_symbol_completion_list (char *t
   int text_len;
   int wild_match;
   int encoded;
-  struct string_vector result = new_string_vector (128);
+  VEC(char_ptr) *completions = VEC_alloc (char_ptr, 128);
   struct symbol *sym;
   struct symtab *s;
   struct partial_symtab *ps;
@@ -5640,7 +5597,7 @@ ada_make_symbol_completion_list (char *t
                  + ps->n_global_syms); psym++)
       {
         QUIT;
-        symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (*psym),
+        symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (*psym),
                                text, text_len, text0, word,
                                wild_match, encoded);
       }
@@ -5650,7 +5607,7 @@ ada_make_symbol_completion_list (char *t
                  + ps->n_static_syms); psym++)
       {
         QUIT;
-        symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (*psym),
+        symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (*psym),
                                text, text_len, text0, word,
                                wild_match, encoded);
       }
@@ -5664,7 +5621,7 @@ ada_make_symbol_completion_list (char *t
   ALL_MSYMBOLS (objfile, msymbol)
   {
     QUIT;
-    symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (msymbol),
+    symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (msymbol),
                            text, text_len, text0, word, wild_match, encoded);
   }
 
@@ -5678,7 +5635,7 @@ ada_make_symbol_completion_list (char *t
 
       ALL_BLOCK_SYMBOLS (b, iter, sym)
       {
-        symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (sym),
+        symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (sym),
                                text, text_len, text0, word,
                                wild_match, encoded);
       }
@@ -5693,7 +5650,7 @@ ada_make_symbol_completion_list (char *t
     b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (sym),
+      symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (sym),
                              text, text_len, text0, word,
                              wild_match, encoded);
     }
@@ -5708,16 +5665,30 @@ ada_make_symbol_completion_list (char *t
       continue;
     ALL_BLOCK_SYMBOLS (b, iter, sym)
     {
-      symbol_completion_add (&result, SYMBOL_LINKAGE_NAME (sym),
+      symbol_completion_add (completions, SYMBOL_LINKAGE_NAME (sym),
                              text, text_len, text0, word,
                              wild_match, encoded);
     }
   }
 
   /* Append the closing NULL entry.  */
-  string_vector_append (&result, NULL);
+  VEC_safe_push (char_ptr, completions, NULL);
 
-  return (result.array);
+  /* Make a copy of the COMPLETIONS VEC before we free it, and then
+     return the copy.  It's unfortunate that we have to make a copy
+     of an array that we're about to destroy, but there is nothing much
+     we can do about it.  Fortunately, it's typically not a very large
+     array.  */
+  {
+    const size_t completions_size = 
+      VEC_length (char_ptr, completions) * sizeof (char *);
+    char **result = malloc (completions_size);
+    
+    memcpy (result, VEC_address (char_ptr, completions), completions_size);
+
+    VEC_free (char_ptr, completions);
+    return result;
+  }
 }
 
                                 /* Breakpoint-related */
Index: Makefile.in
===================================================================
--- Makefile.in	(revision 165)
+++ Makefile.in	(revision 166)
@@ -1797,7 +1797,7 @@ ada-lang.o: ada-lang.c $(defs_h) $(gdb_s
 	$(gdbcore_h) $(hashtab_h) $(gdb_obstack_h) $(ada_lang_h) \
 	$(completer_h) $(gdb_stat_h) $(ui_out_h) $(block_h) $(infcall_h) \
 	$(dictionary_h) $(exceptions_h) $(annotate_h) $(valprint_h) \
-	$(source_h) $(observer_h)
+	$(source_h) $(observer_h) $(vec_h)
 ada-typeprint.o: ada-typeprint.c $(defs_h) $(gdb_obstack_h) $(bfd_h) \
 	$(symtab_h) $(gdbtypes_h) $(expression_h) $(value_h) $(gdbcore_h) \
 	$(target_h) $(command_h) $(gdbcmd_h) $(language_h) $(demangle_h) \

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFA/commit/Ada] Replace home-made string_vector struct with  VEC
  2008-02-06  2:40         ` [RFA/commit/Ada] Replace home-made string_vector struct with VEC Joel Brobecker
@ 2008-02-06  2:48           ` Daniel Jacobowitz
  2008-02-07 19:14           ` Joel Brobecker
  1 sibling, 0 replies; 10+ messages in thread
From: Daniel Jacobowitz @ 2008-02-06  2:48 UTC (permalink / raw)
  To: Joel Brobecker; +Cc: gdb-patches

On Tue, Feb 05, 2008 at 05:57:03PM -0800, Joel Brobecker wrote:
>   1. Would it make sense to have standard VECs predefined somewhere?
>      For instance, I wonder how often a VEC of strings will be useful
>      in GDB...

Sounds reasonable to me.

>   2. Would it make sense to expand the VEC API to have a method
>      that will return a copy of the underlying array? After some
>      thoughts, and a look at the current usage, it seemed to me
>      that it didn't make much sense. I didn't see any other places
>      where we need to make that copy, and making that copy is
>      pretty simple.

I agree.  The memory allocation doesn't work out to make this
advantageous.

If we want to save the copy, allowing some other way to register a
destructor for the array might do more good.

Thanks!

-- 
Daniel Jacobowitz
CodeSourcery


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [RFA/commit/Ada] Replace home-made string_vector struct with VEC
  2008-02-06  2:40         ` [RFA/commit/Ada] Replace home-made string_vector struct with VEC Joel Brobecker
  2008-02-06  2:48           ` Daniel Jacobowitz
@ 2008-02-07 19:14           ` Joel Brobecker
  1 sibling, 0 replies; 10+ messages in thread
From: Joel Brobecker @ 2008-02-07 19:14 UTC (permalink / raw)
  To: gdb-patches

> 2007-02-05  Joel Brobecker  <brobecker@adacore.com>
> 
>         * ada-lang.c: #include "vec.h".
>         (struct string_vector, new_string_vector, string_vector_append):
>         Delete.
>         (char_ptr): New typedef.
>         (DEF_VEC_P (char_ptr)): New VEC type.
>         (symbol_completion_add): Update profile to take the new VEC type
>         instead of the old string_vector structure. Update code accordingly.
>         (ada_make_symbol_completion_list): Use the new VEC type instead of
>         the old string_vector structure, and update the code accordingly.
>         * Makefile.in (ada-lang.o): Add dependency on vec.h.

I just checked this one in.

> >   1. Would it make sense to have standard VECs predefined somewhere?
> >      For instance, I wonder how often a VEC of strings will be useful
> >      in GDB...
> 
> Sounds reasonable to me.

OK. Let's reserve the idea for now until we notice the same VECs
being used in several places...

-- 
Joel


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2008-02-07 19:14 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-28 13:07 [RFA] Make symbol completion language-specific Joel Brobecker
2008-01-29 17:35 ` Daniel Jacobowitz
2008-01-30 20:43   ` Joel Brobecker
2008-01-30 21:25     ` Daniel Jacobowitz
2008-02-04 23:15   ` Joel Brobecker
2008-02-04 23:30     ` Daniel Jacobowitz
2008-02-05 22:32       ` Joel Brobecker
2008-02-06  2:40         ` [RFA/commit/Ada] Replace home-made string_vector struct with VEC Joel Brobecker
2008-02-06  2:48           ` Daniel Jacobowitz
2008-02-07 19:14           ` Joel Brobecker

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox