>>>>> "Bruno" == Bruno Larsen via Gdb-patches <gdb-patches@sourceware.org> writes:
Bruno> When running gdb.cp/nsusing.cc and stopping at line 17, we can ask GDB
Bruno> to print x and get a compiler-dependent answer. Using gcc 12.2.1, GDB
Bruno> will print M::x, and using clang 16.0.0 prints N::x. Not only is this
Bruno> behavior confusing to users, it is also not consistent with compiler
Bruno> behaviors, which would warn that using x is ambiguous at this point.
Thanks for the patch.
Bruno> + error (_("%s"), error_str.c_str ());
Bruno> + }
Bruno> + else
Bruno> + return found_symbols;
Bruno> +
Bruno> + /* This is needed to silence a -Werror=return-type warning, because
Bruno> + the above if case doesn't have a return statement. */
Bruno> + gdb_assert_not_reached ();
Bruno> }
This is surprising, because error is marked as noreturn.
Anyway perhaps a better workaround would be to just remove the 'else'.
Bruno> + /* Despite getting a map, it should have at most one element, otherwise
Bruno> + cp_lookup_symbol_via_import will have already reported the ambiguity. */
Bruno> + std::map<std::string, struct block_symbol> result
Bruno> + = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 1, 1);
I think it would be better to have some kind of wrapper, where
cp_lookup_symbol_via_imports takes a set as a parameter (to avoid having
to merge sets), and then the wrapper extracts the result.
Maybe the error handling could be stuck in the wrapper as well. I
didn't look into that.
Tom
@@ -32,6 +32,7 @@
#include "buildsym.h"
#include "language.h"
#include "namespace.h"
+#include <map>
#include <string>
static struct block_symbol
@@ -370,9 +371,13 @@ cp_lookup_symbol_in_namespace (const char *the_namespace, const char *name,
only the import of Y is considered.
SEARCH_SCOPE_FIRST is an internal implementation detail: Callers must
- pass 0 for it. Internally we pass 1 when recursing. */
+ pass 0 for it. Internally we pass 1 when recursing.
-static struct block_symbol
+ When recursing, the function may return a map with multiple elements,
+ corresponding to the symbols that were found in the lower scopes. The
+ top level call will never return a map with more than one element. */
+
+static std::map<std::string, struct block_symbol>
cp_lookup_symbol_via_imports (const char *scope,
const char *name,
const struct block *block,
@@ -386,13 +391,20 @@ cp_lookup_symbol_via_imports (const char *scope,
int len;
int directive_match;
+ /* All the symbols we found will be kept in this relational map between
+ the mangled name and the block_symbol found. We do this so that GDB
+ won't incorrectly report an ambiguous symbol for finding the same
+ thing twice. */
+ std::map<std::string, struct block_symbol> found_symbols;
+
/* First, try to find the symbol in the given namespace if requested. */
if (search_scope_first)
- sym = cp_lookup_symbol_in_namespace (scope, name,
- block, domain, 1);
-
- if (sym.symbol != NULL)
- return sym;
+ {
+ sym = cp_lookup_symbol_in_namespace (scope, name,
+ block, domain, 1);
+ if (sym.symbol != nullptr)
+ found_symbols[sym.symbol->m_name] = sym;
+ }
/* Due to a GCC bug, we need to know the boundaries of the current block
to know if a certain using directive is valid. */
@@ -446,7 +458,7 @@ cp_lookup_symbol_via_imports (const char *scope,
if (declaration_only || sym.symbol != NULL || current->declaration)
{
if (sym.symbol != NULL)
- return sym;
+ found_symbols[sym.symbol->m_name] = sym;
continue;
}
@@ -467,23 +479,45 @@ cp_lookup_symbol_via_imports (const char *scope,
sym = cp_lookup_symbol_in_namespace (scope,
current->import_src,
block, domain, 1);
+ found_symbols[sym.symbol->m_name] = sym;
}
else if (current->alias == NULL)
{
/* If this import statement creates no alias, pass
current->inner as NAMESPACE to direct the search
towards the imported namespace. */
- sym = cp_lookup_symbol_via_imports (current->import_src,
+ std::map<std::string, struct block_symbol> sym_map
+ = cp_lookup_symbol_via_imports (current->import_src,
name, block,
domain, 1, 0, 0);
+ found_symbols.merge(sym_map);
}
- if (sym.symbol != NULL)
- return sym;
}
}
- return {};
+ /* We only want to trigger this error on the top level call, otherwise
+ the error may only have a partial list of the possible symbols. */
+ if (search_scope_first == 0 && found_symbols.size () > 1)
+ {
+ auto itr = found_symbols.cbegin ();
+ std::string error_str = "Reference to \"";
+ error_str += name;
+ error_str += "\" is ambiguous, possibilities are: ";
+ error_str += itr->second.symbol->print_name ();
+ for (itr++; itr != found_symbols.end (); itr++)
+ {
+ error_str += " and ";
+ error_str += itr->second.symbol->print_name ();
+ }
+ error (_("%s"), error_str.c_str ());
+ }
+ else
+ return found_symbols;
+
+ /* This is needed to silence a -Werror=return-type warning, because
+ the above if case doesn't have a return statement. */
+ gdb_assert_not_reached ();
}
/* Helper function that searches an array of symbols for one named NAME. */
@@ -503,9 +537,10 @@ search_symbol_list (const char *name, int num,
return NULL;
}
-/* Like cp_lookup_symbol_via_imports, but if BLOCK is a function, it
- searches through the template parameters of the function and the
- function's type. */
+/* Search for symbols whose name match NAME in the given SCOPE.
+ if BLOCK is a function, we'll search first through the template
+ parameters and function type. Afterwards (or if BLOCK is not a function)
+ search through imported directives using cp_lookup_symbol_via_imports. */
struct block_symbol
cp_lookup_symbol_imports_or_template (const char *scope,
@@ -514,7 +549,6 @@ cp_lookup_symbol_imports_or_template (const char *scope,
const domain_enum domain)
{
struct symbol *function = block->function ();
- struct block_symbol result;
if (symbol_lookup_debug)
{
@@ -596,15 +630,21 @@ cp_lookup_symbol_imports_or_template (const char *scope,
}
}
- result = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 1, 1);
+ /* Despite getting a map, it should have at most one element, otherwise
+ cp_lookup_symbol_via_import will have already reported the ambiguity. */
+ std::map<std::string, struct block_symbol> result
+ = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 1, 1);
if (symbol_lookup_debug)
{
gdb_printf (gdb_stdlog,
"cp_lookup_symbol_imports_or_template (...) = %s\n",
- result.symbol != NULL
- ? host_address_to_string (result.symbol) : "NULL");
+ result.size() == 1
+ ? host_address_to_string (result[0].symbol) : "NULL");
}
- return result;
+ if (result.empty ())
+ return {};
+ else
+ return result.begin ()->second;
}
/* Search for NAME by applying relevant import statements belonging to BLOCK
@@ -616,13 +656,13 @@ cp_lookup_symbol_via_all_imports (const char *scope, const char *name,
const struct block *block,
const domain_enum domain)
{
- struct block_symbol sym;
+ std::map<std::string, struct block_symbol> sym;
while (block != NULL)
{
sym = cp_lookup_symbol_via_imports (scope, name, block, domain, 0, 0, 1);
- if (sym.symbol)
- return sym;
+ if (sym.size() == 1)
+ return sym.begin ()->second;
block = block->superblock ();
}
@@ -127,11 +127,20 @@ gdb_test_multiple "print x" "print x, before using statement" {
-re -wrap "No symbol .x. in current context.*" {
pass $gdb_test_name
}
- -re -wrap "= 911.*" {
+ -re -wrap "Reference to .x. is ambiguous.*" {
# GCC doesn't properly set the decl_line for namespaces, so GDB believes
# that the "using namespace M" line has already passed at this point.
xfail $gdb_test_name
}
}
gdb_test "next" ".*" "using namespace M"
-gdb_test "print x" "= 911" "print x, only using M"
+gdb_test_multiple "print x" "print x, only using M" {
+ -re -wrap "= 911.*" {
+ pass $gdb_test_name
+ }
+ -re -wrap "Reference to .x. is ambiguous.*" {
+ xfail $gdb_test_name
+ }
+}
+gdb_test "next" ".*" "using namespace N"
+gdb_test "print x" "Reference to .x. is ambiguous.*" "print x, with M and N"