Re: [patch 1/2] Perform a namespace lookup at every block level

[Sami Wagiaalla <swagiaal at redhat dot com>]

> Sorry about the delay on this.

Thanks for the review.

I fixed the mentioned formatting issues. I also got rid of the static 
prototypes in the patch and rearranged the functions as needed.

> Sami> @@ -1304,13 +1304,14 @@ lookup_symbol_aux (const char *name, const char *linkage_name,
> [...]
> Sami> +      /* C++ language specific lookup requires the lowest block level.  */
> Sami> +      if (language != language_cplus)
> Sami> +        block = function_block;
>
> I like to avoid special cases for a specific language in the generic
> code.  These make gdb more brittle over time; also they make it harder
> to understand.

This is actually not needed. As I mentioned up-thread I put it there to 
preserve the status quo for languages other than c++. I looked further 
into it. All languages with the exception of c++ and ADA use 
basic_lookup_symbol_nonlocal, cp_lookup_symbol_nonlocal, and 
ada_lookup_symbol_nonlocal. cp_lookup_symbol_nonlocal is the only 
function which makes an assumption about which block is passed to it. 
The other functions retrieve the desired block explicitly via 
block_static_block (block) or block_global_block (block). So it is gone!

> Sami> diff --git a/gdb/testsuite/gdb.cp/namespace-using.cc b/gdb/testsuite/gdb.cp/namespace-using.cc
>
> I think this needs more tests.  At the very least it should have a test
> case for the bug Daniel pointed out upthread, but I think it would be
> good to have tests for other confounding cases.

I have added shadowing.exp to the patch which belongs to a future patch 
but has some good tests, and it tests for the problem pointed out by 
Daniel. I also added namespace-multiple-imports.exp. Excuse the kfails;
they will be fixed by a future patch.


   2009-10-20  Sami Wagiaalla  <swagiaal@redhat.com>
   
       * dwarf2read.c (read_lexical_block_scope): Create blocks for scopes
       which contain using directives even if they contain no declarations.
       * symtab.c (lookup_symbol_aux): Pass lowest level
       block to la_lookup_symbol_nonlocal.
       * cp-namespace.c (cp_lookup_symbol_nonlocal): call
       cp_lookup_symbol_namespace.
       (cp_lookup_symbol_namespace): Perform an import lookup at every block
       level.
       (cp_lookup_symbol_imports): New function.
       (cp_lookup_symbol_in_namespace): New function.
   
   2009-10-20  Sami Wagiaalla  <swagiaal@redhat.com>
   
       * gdb.cp/namespace-using.exp: Add test for printing of namespaces
       imported into file scope.
       Marked test as xfail.
       * gdb.cp/namespace-using.cc (marker5): New function.
       * gdb.cp/shadowing.exp: New test.
       * gdb.cp/shadowing.cc: New test program.

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index d2d8f2e..06a5486 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -265,8 +265,115 @@ cp_lookup_symbol_nonlocal (const char *name,
			   const struct block *block,
			   const domain_enum domain)
{
-  return lookup_namespace_scope (name, linkage_name, block, domain,
-				 block_scope (block), 0);
+  struct symbol *sym;
+  const char *scope = block_scope (block);
+
+  sym = lookup_namespace_scope (name, linkage_name, block, domain, scope, 0);
+  if (sym != NULL)
+    return sym;
+
+  return cp_lookup_symbol_namespace (scope, name, linkage_name, block, domain);
+}
+
+/* Look up NAME in the C++ namespace NAMESPACE. Other arguments are as in
+   cp_lookup_symbol_nonlocal.  */
+
+static struct symbol *
+cp_lookup_symbol_in_namespace (const char *namespace,
+                   const char *name,
+                   const char *linkage_name,
+                   const struct block *block,
+                   const domain_enum domain)
+{
+
+  if (namespace[0] == '\0')
+    {
+      return lookup_symbol_file (name, linkage_name, block,
+                                 domain, 0);
+    }
+  else
+    {
+      char *concatenated_name = alloca (strlen (namespace) + 2 +
+                                        strlen (name+ 1));
+      strcpy (concatenated_name, namespace);
+      strcat (concatenated_name, "::");
+      strcat (concatenated_name, name);
+      return lookup_symbol_file (concatenated_name, linkage_name,
+                                 block, domain,cp_is_anonymous (namespace));
+    }
+}
+
+/* Search for NAME by applying all import statements belonging
+   to BLOCK which are applicable in SCOPE.  */
+
+static struct symbol *
+cp_lookup_symbol_imports (const char *scope,
+                          const char *name,
+                          const char *linkage_name,
+                          const struct block *block,
+                          const domain_enum domain)
+{
+  const struct using_direct *current;
+  struct symbol *sym;
+
+  /* First, try to find the symbol in the given namespace.  */
+  sym = cp_lookup_symbol_in_namespace (scope, name, linkage_name, block,
+                                       domain);
+  if (sym != NULL)
+    return sym;
+
+  /* Go through the using directives.  If any of them add new
+     names to the namespace we're searching in, see if we can find a
+     match by applying them.  */
+
+  for (current = block_using (block);
+       current != NULL;
+       current = current->next)
+    {
+
+      /* If the import destination is the current scope or one of its ancestors then
+         it is applicable.  */
+      if (strncmp (scope, current->import_dest, strlen (current->import_dest)) == 0)
+	{
+	  sym = cp_lookup_symbol_in_namespace (current->import_src,
+	                                       name,
+	                                       linkage_name,
+	                                       block,
+	                                       domain);
+	  if (sym != NULL)
+	    return sym;
+	}
+    }
+
+  return NULL;
+}
+
+
+ /* Searches for NAME in the current namespace, and by applying relevant import
+    statements belonging to BLOCK and its parents. SCOPE is the namespace scope
+    of the context in which the search is being evaluated.  */
+
+struct symbol*
+cp_lookup_symbol_namespace (const char *scope,
+                            const char *name,
+                            const char *linkage_name,
+                            const struct block *block,
+                            const domain_enum domain)
+{
+  struct symbol *sym;
+
+  /* Search for name in namespaces imported to this and parent blocks.  */
+  while (block != NULL)
+    {
+      sym = cp_lookup_symbol_imports (scope,name, linkage_name, block, domain);
+
+      if (sym)
+	return sym;
+
+      block = BLOCK_SUPERBLOCK (block);
+    }
+
+  return NULL;
}

/* Lookup NAME at namespace scope (or, in C terms, in static and
@@ -320,65 +427,8 @@ lookup_namespace_scope (const char *name,
  namespace = alloca (scope_len + 1);
  strncpy (namespace, scope, scope_len);
  namespace[scope_len] = '\0';
-  return cp_lookup_symbol_namespace (namespace, name, linkage_name,
-				     block, domain);
-}
-
-/* Look up NAME in the C++ namespace NAMESPACE, applying the using
-   directives that are active in BLOCK.  Other arguments are as in
-   cp_lookup_symbol_nonlocal.  */
-
-struct symbol *
-cp_lookup_symbol_namespace (const char *namespace,
-			    const char *name,
-			    const char *linkage_name,
-			    const struct block *block,
-			    const domain_enum domain)
-{
-  const struct using_direct *current;
-  struct symbol *sym;
-
-  /* First, go through the using directives.  If any of them add new
-     names to the namespace we're searching in, see if we can find a
-     match by applying them.  */
-
-  for (current = block_using (block);
-       current != NULL;
-       current = current->next)
-    {
-      if (strcmp (namespace, current->import_dest) == 0)
-	{
-	  sym = cp_lookup_symbol_namespace (current->import_src,
-					    name,
-					    linkage_name,
-					    block,
-					    domain);
-	  if (sym != NULL)
-	    return sym;
-	}
-    }
-
-  /* We didn't find anything by applying any of the using directives
-     that are still applicable; so let's see if we've got a match
-     using the current namespace.  */
-  
-  if (namespace[0] == '\0')
-    {
-      return lookup_symbol_file (name, linkage_name, block,
-				 domain, 0);
-    }
-  else
-    {
-      char *concatenated_name
-	= alloca (strlen (namespace) + 2 + strlen (name) + 1);
-      strcpy (concatenated_name, namespace);
-      strcat (concatenated_name, "::");
-      strcat (concatenated_name, name);
-      sym = lookup_symbol_file (concatenated_name, linkage_name,
-				block, domain, 
-				cp_is_anonymous (namespace));
-      return sym;
-    }
+  return cp_lookup_symbol_in_namespace (namespace, name, linkage_name,
+                                        block, domain);
}

/* Look up NAME in BLOCK's static block and in global blocks.  If
@@ -461,11 +511,11 @@ cp_lookup_nested_type (struct type *parent_type,
	   lookup_symbol_namespace works when looking them up.  */

	const char *parent_name = TYPE_TAG_NAME (parent_type);
-	struct symbol *sym = cp_lookup_symbol_namespace (parent_name,
-							 nested_name,
-							 NULL,
-							 block,
-							 VAR_DOMAIN);
+	struct symbol *sym = cp_lookup_symbol_in_namespace (parent_name,
+	                                                    nested_name,
+	                                                    NULL,
+	                                                    block,
+	                                                    VAR_DOMAIN);
	if (sym == NULL || SYMBOL_CLASS (sym) != LOC_TYPEDEF)
	  return NULL;
	else
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 69a7242..c2b2e3c 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -3920,7 +3920,7 @@ read_lexical_block_scope (struct die_info *die, struct dwarf2_cu *cu)
    }
  new = pop_context ();

-  if (local_symbols != NULL)
+  if (local_symbols != NULL || using_directives != NULL)
    {
      struct block *block
        = finish_block (0, &local_symbols, new->old_blocks, new->start_addr,
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 8d9d72c..dfa8f48 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -1304,13 +1304,14 @@ lookup_symbol_aux (const char *name, const char *linkage_name,
      && block != NULL)
    {
      struct symbol *sym = NULL;
+      const struct block *function_block = block;
      /* 'this' is only defined in the function's block, so find the
	 enclosing function block.  */
-      for (; block && !BLOCK_FUNCTION (block);
-	   block = BLOCK_SUPERBLOCK (block));
+      for (; function_block && !BLOCK_FUNCTION (function_block);
+	   function_block = BLOCK_SUPERBLOCK (function_block));

-      if (block && !dict_empty (BLOCK_DICT (block)))
-	sym = lookup_block_symbol (block, langdef->la_name_of_this,
+      if (function_block && !dict_empty (BLOCK_DICT (function_block)))
+	sym = lookup_block_symbol (function_block, langdef->la_name_of_this,
				   NULL, VAR_DOMAIN);
      if (sym)
	{
diff --git a/gdb/testsuite/gdb.cp/namespace-using.cc b/gdb/testsuite/gdb.cp/namespace-using.cc
index 4786fd5..762a508 100644
--- a/gdb/testsuite/gdb.cp/namespace-using.cc
+++ b/gdb/testsuite/gdb.cp/namespace-using.cc
@@ -1,3 +1,16 @@
+namespace C
+{
+  int cc = 3;
+}
+
+using namespace C;
+int marker5 ()
+{
+  cc;
+  return 0;
+}
+
+
namespace A
{
  int _a = 1;
@@ -6,7 +19,7 @@ namespace A

int marker4(){
	using A::x;
-	return 0;
+	return marker5 ();
}

int marker3(){
diff --git a/gdb/testsuite/gdb.cp/namespace-using.cc.orig b/gdb/testsuite/gdb.cp/namespace-using.cc.orig
new file mode 100644
index 0000000..4786fd5
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/namespace-using.cc.orig
@@ -0,0 +1,45 @@
+namespace A
+{
+  int _a = 1;
+  int x = 2;
+}
+
+int marker4(){
+	using A::x;
+	return 0;
+}
+
+int marker3(){
+	return marker4();
+}
+
+int marker2()
+{
+  namespace B = A;
+  B::_a;
+  return marker3();
+}
+
+int marker1()
+{
+  int total = 0;
+  {
+    int b = 1;
+    {
+      using namespace A;
+      int c = 2;
+      {
+        int d = 3;
+        total = _a + b + c + d + marker2(); // marker1 stop
+      }
+    }
+  }
+  return total;
+}
+
+int main()
+{
+  using namespace A;
+  _a;
+  return marker1();
+}
diff --git a/gdb/testsuite/gdb.cp/namespace-using.exp b/gdb/testsuite/gdb.cp/namespace-using.exp
index f24973f..bd1016b 100644
--- a/gdb/testsuite/gdb.cp/namespace-using.exp
+++ b/gdb/testsuite/gdb.cp/namespace-using.exp
@@ -28,6 +28,11 @@ if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {deb
    return -1
}

+if [get_compiler_info ${binfile}] {
+    return -1;
+}
+
+
# Get things started.

gdb_exit
@@ -73,7 +78,13 @@ gdb_test "print B::a" "= 1"
gdb_breakpoint "marker3"
gdb_continue_to_breakpoint "marker3"

-gdb_test "print _a" "No symbol \"_a\" in current context." "Print a without import"
+# gcc-4-3 puts import statements for aliases in
+# the global scope instead of the corresponding
+# function scope. These wrong import statements throw
+# this test off. This is fixed in gcc-4-4.
+if [test_compiler_info gcc-4-3-*] then { setup_xfail *-*-* }
+
+gdb_test "print _a" "No symbol \"_a\" in current context." "Print _a without import"

############################################
# Test printing of individually imported elements
@@ -85,3 +96,14 @@ if ![runto marker4] then {
}

gdb_test "print x" "= 2"
+
+############################################
+# test printing of namespace imported into
+# file scope.
+
+if ![runto marker5] then {
+    perror "couldn't run to marker5"
+    continue
+}
+
+gdb_test "print cc" "= 3"
diff --git a/gdb/testsuite/gdb.cp/shadowing.cc b/gdb/testsuite/gdb.cp/shadowing.cc
new file mode 100644
index 0000000..1651510
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/shadowing.cc
@@ -0,0 +1,45 @@
+namespace A
+{
+  int x = 11;
+}
+
+int x = 22;
+int y = 0;
+
+class B
+{
+public:
+  int x;
+
+  int
+  func()
+  {
+    x = 33;
+    y++; // marker1
+
+      {
+        int x = 44;
+        y++; // marker2
+
+          {
+            int x = 55;
+            y++; // marker3
+
+              {
+                using namespace A;
+                y++; // marker4
+
+                using A::x;
+                y++; // marker5
+              }
+          }
+      }
+  }
+};
+
+int
+main()
+{
+  B theB;
+  return theB.func();
+}
diff --git a/gdb/testsuite/gdb.cp/shadowing.exp b/gdb/testsuite/gdb.cp/shadowing.exp
new file mode 100644
index 0000000..443881e
--- /dev/null
+++ b/gdb/testsuite/gdb.cp/shadowing.exp
@@ -0,0 +1,86 @@
+# Copyright 2008 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
+}
+
+set prms_id 0
+set bug_id 0
+
+set testfile shadowing
+set srcfile ${testfile}.cc
+set binfile ${objdir}/${subdir}/${testfile}
+if  { [gdb_compile "${srcdir}/${subdir}/${srcfile}" "${binfile}" executable {debug c++}] != "" } {
+    untested "Couldn't compile test program"
+    return -1
+}
+
+# Get things started.
+
+gdb_exit
+gdb_start
+gdb_reinitialize_dir $srcdir/$subdir
+gdb_load ${binfile}
+
+if ![runto_main] then {
+    perror "couldn't run to breakpoint main"
+    continue
+}
+
+############################################
+# Test printing of class variable is not shadowed
+# by global variable
+
+gdb_breakpoint [gdb_get_line_number "marker1"]
+gdb_continue_to_breakpoint "marker1"
+
+gdb_test "print x" "= 33" "Print class x shadowing global x"
+
+
+############################################
+# Test printing local variable is not shadowed
+# by class variable
+
+gdb_breakpoint [gdb_get_line_number "marker2"]
+gdb_continue_to_breakpoint "marker2"
+
+gdb_test "print x" "= 44" "Print local x shadowing class x"
+
+############################################
+# Test inner scope x is printed not outer scope
+
+gdb_breakpoint [gdb_get_line_number "marker3"]
+gdb_continue_to_breakpoint "marker3"
+
+gdb_test "print x" "= 55" "Print inner scope x"
+
+############################################
+# Test printing local variable is not shadowed
+# by namespace variable
+
+gdb_breakpoint [gdb_get_line_number "marker4"]
+gdb_continue_to_breakpoint "marker4"
+
+gdb_test "print x" "= 55" "Print local x not namespace x"
+
+############################################
+# Test imported namespace element is printed
+
+gdb_breakpoint [gdb_get_line_number "marker5"]
+gdb_continue_to_breakpoint "marker5"
+
+setup_kfail "gdb/7936" "*-*-*"
+gdb_test "print x" "= 11" "Print imported namespace x"