Re: [PATCH 2/2] Test and support all cpp operator types

sami wagiaalla
2010-05-18T20:49:24Z

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

On 05/12/2010 06:21 PM, Tom Tromey wrote:
>>>>>> "Sami" == sami wagiaalla<swagiaal@redhat.com> writes:
>
> Sami> This patch adds testing and support for the following types of operator
> Sami> lookup:
> Sami> - non-member operators.
> Sami> - imported operators (using directive, anonymous namespaces).
> Sami> - ADL operators.
>
> I like the general approach of this patch.
>
> I have a few comments.
>
> Sami> @@ -800,21 +800,27 @@ make_symbol_overload_list_using (const char *func_name,
> Sami> const char *namespace)
> [...]
> Sami> - for (current = block_using (get_selected_block (0));
> Sami> - current != NULL;
> Sami> - current = current->next)
> Sami> - {
> Sami> - if (strcmp (namespace, current->import_dest) == 0)
> Sami> - {
> Sami> - make_symbol_overload_list_using (func_name,
> Sami> - current->import_src);
> Sami> - }
> Sami> - }
> Sami> + for (block = get_selected_block (0);
> Sami> + block != NULL;
> Sami> + block = BLOCK_SUPERBLOCK(block))
> Sami> + for (current = block_using (block);
> Sami> + current != NULL;
> Sami> + current = current->next)
> Sami> + {
> [...]
>
> This part seems a little weird to me.
> make_symbol_overload_list_using calls make_symbol_overload_list_namespace,
> which calls make_symbol_overload_list_qualified, which itself
> starts with get_selected_block and iterates over the superblocks.
>
> It seems to me that only one such iteration should be needed.
> I don't have a test case but it seems like this could cause incorrect
> results in some corner case.
>

This patch fixes the above. It assumes that namespace names will belong
to the static and global blocks. This enables us to separate the
non-namespace iterative search from the namespace search.

[-- Attachment #2: search-cleanup.patch --]
[-- Type: text/plain, Size: 4386 bytes --]

2010-05-18 Sami Wagiaalla <swagiaal@redhat.com>

* cp-support.c (make_symbol_overload_list_namespace): Only search
static and global blocks.
(make_symbol_overload_list_block): New function.
(make_symbol_overload_list): Separate namespace search from block
search.
(make_symbol_overload_list_qualified): Use
make_symbol_overload_list_block.

diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 0a0f777..e7ab6c5 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -693,6 +693,7 @@ make_symbol_overload_list (const char *func_name,
const char *namespace)
{
struct cleanup *old_cleanups;
+ const char *name;

sym_return_val_size = 100;
sym_return_val_index = 0;
@@ -704,20 +705,54 @@ make_symbol_overload_list (const char *func_name,

make_symbol_overload_list_using (func_name, namespace);

+ if (namespace[0] == '\0')
+ name = func_name;
+ else
+ {
+ char *concatenated_name
+ = alloca (strlen (namespace) + 2 + strlen (func_name) + 1);
+ strcpy (concatenated_name, namespace);
+ strcat (concatenated_name, "::");
+ strcat (concatenated_name, func_name);
+ name = concatenated_name;
+ }
+
+ make_symbol_overload_list_qualified (name);
+
discard_cleanups (old_cleanups);

return sym_return_val;
}

+/* Add all symbols with a name matching NAME in BLOCK to the overload
+ list. */
+
+static void
+make_symbol_overload_list_block (const char *name,
+ const struct block *block)
+{
+ struct dict_iterator iter;
+ struct symbol *sym;
+
+ const struct dictionary *dict = BLOCK_DICT (block);
+
+ for (sym = dict_iter_name_first (dict, name, &iter);
+ sym != NULL;
+ sym = dict_iter_name_next (name, &iter))
+ overload_list_add_symbol (sym, name);
+}
+
/* Adds the function FUNC_NAME from NAMESPACE to the overload set. */

static void
make_symbol_overload_list_namespace (const char *func_name,
const char *namespace)
{
+ const char *name;
+ const struct block *block = NULL;

if (namespace[0] == '\0')
- make_symbol_overload_list_qualified (func_name);
+ name = func_name;
else
{
char *concatenated_name
@@ -725,8 +760,17 @@ make_symbol_overload_list_namespace (const char *func_name,
strcpy (concatenated_name, namespace);
strcat (concatenated_name, "::");
strcat (concatenated_name, func_name);
- make_symbol_overload_list_qualified (concatenated_name);
+ name = concatenated_name;
}
+
+ /* look in the static block. */
+ block = block_static_block (get_selected_block (0));
+ make_symbol_overload_list_block (name, block);
+
+ /* look in the global block. */
+ block = block_global_block (block);
+ make_symbol_overload_list_block (name, block);
+
}

/* Search the namespace of the given type and namespace of and public base
@@ -852,16 +896,7 @@ make_symbol_overload_list_qualified (const char *func_name)
complete on local vars. */

for (b = get_selected_block (0); b != NULL; b = BLOCK_SUPERBLOCK (b))
- {
- dict = BLOCK_DICT (b);
-
- for (sym = dict_iter_name_first (dict, func_name, &iter);
- sym;
- sym = dict_iter_name_next (func_name, &iter))
- {
- overload_list_add_symbol (sym, func_name);
- }
- }
+ make_symbol_overload_list_block (func_name, b);

surrounding_static_block = block_static_block (get_selected_block (0));

@@ -872,14 +907,7 @@ make_symbol_overload_list_qualified (const char *func_name)
{
QUIT;
b = BLOCKVECTOR_BLOCK (BLOCKVECTOR (s), GLOBAL_BLOCK);
- dict = BLOCK_DICT (b);
-
- for (sym = dict_iter_name_first (dict, func_name, &iter);
- sym;
- sym = dict_iter_name_next (func_name, &iter))
- {
- overload_list_add_symbol (sym, func_name);
- }
+ make_symbol_overload_list_block (func_name, b);
}

ALL_PRIMARY_SYMTABS (objfile, s)
@@ -889,14 +917,7 @@ make_symbol_overload_list_qualified (const char *func_name)
/* Don't do this block twice. */
if (b == surrounding_static_block)
continue;
- dict = BLOCK_DICT (b);
-
- for (sym = dict_iter_name_first (dict, func_name, &iter);
- sym;
- sym = dict_iter_name_next (func_name, &iter))
- {
- overload_list_add_symbol (sym, func_name);
- }
+ make_symbol_overload_list_block (func_name, b);
}
}