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

[sami wagiaalla <swagiaal at redhat dot com>]

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: search-cleanup.patch
Description: Text document